Add parallletest linter and t.Parallell (almost) everywhere

[ntnn]

Summary

What Type of PR Is This?

/kind chore

Related Issue(s)

Fixes #

Release Notes

NONE

[SimonTheLeg]

overall looks good to me, let's see what CI will say

[Elbehery]

Thanks for adding this, it is really important to have the tests running in parallel 👍🏽

I would suggest to split this into multiple PRs, one per package.

This will help detecting issues early and easily :)

I believe some tests won't be able to run in parallel with other tests, as the cluster state ( due to running tests ) would violate the invariants required by some other tests

please correct me if I am wrong 🙏🏽

[xrstf]

I must be the only one who's not the biggest fan of running everything in parallel. Mainly for two reasons:

• It eats up a lot of CPU, starving other apps while the tests are running. And some of us only have 32GB in their machines..
• The test log is not available in realtime and can be a jumbled mess.

For small unit tests I can see the appeal, but my laptop melts when it tries to run every single kcp e2e test at the same time.

Thankfully I can setup env variables to my liking, so I don't have to veto this PR :D

[ntnn]

@Elbehery
I would suggest to split this into multiple PRs, one per package.

That'd be over a hundred PRs - I'd rather have one commit that adds t.Parallel where needed and everything but that in its own commits and PRs.

This will help detecting issues early and easily :)

Yes, and a few I already found :D

I believe some tests won't be able to run in parallel with other tests, as the cluster state ( due to running tests ) would violate the invariants required by some other tests

Correct, but for those we skip parallel requirement or they are using the private kcp instance instead of the shared one.

@xrstf
* It eats up a lot of CPU, starving other apps while the tests are running. And some of us only have 32GB in their machines..

That's what GOMAXPROCS is for - I e.g. set it to ~3/4th of the available cores so the machine stays functional without starving other apps of resources.

* The test log is not available in realtime and can be a jumbled mess.

I don't think I follow there, generally when a specific test fails I run that test specifically instead of all tests.

For small unit tests I can see the appeal, but my laptop melts when it tries to run every single kcp e2e test at the same time.

This change is largely targeting the unit tests :D None of those had the parallel marker so far.

Thankfully I can setup env variables to my liking, so I don't have to veto this PR :D

:D

[Elbehery]

@ntnn thanks for your follow up.

Looks like folks here are more agile regarding merging rules :)

LGTM

[xrstf]

I don't think I follow there, generally when a specific test fails I run that test specifically instead of all tests.

When tests run in sequence, I can just do go test -v and get a nice, realtime log and can debug the test in realtime as it is happening. Once parallelism is involved, Go does not print the logs anymore, and if your application code uses its own logging/printing, you will see all output from all parallel tests at the same time, making the logs pretty much worthless. That alone makes it sometimes needlessly hard to even figure out which test I would need to run in isolation to see a consistent log.

But again, back in the day I had direnv to work around the IGNORE_GO_VERSION thing (until we thankfully removed that speed bump :D), and I already use it to reduce parallelism on most other kcp subproject e2e tests. ^^

[xrstf]

/retest

CI nodes were rotated, so Athens restarted.

[ntnn]

/test pull-kcp-test-e2e-sharded

[xrstf]

/approve
/lgtm

[kcp-ci-bot]

LGTM label has been added.

Details

Git tree hash: 32fe88623232ff9097b82381de4fdd81797ecb66

[kcp-ci-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xrstf

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [xrstf]
• config/crds/OWNERS [xrstf]
• pkg/admission/OWNERS [xrstf]
• staging/src/github.com/kcp-dev/sdk/apis/OWNERS [xrstf]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[kcp-ci-bot]

New changes are detected. LGTM label has been removed.

[ntnn]

@xrstf retag pls, because we merged something inbetween I had to update the t.Parallel commit :D

Linting root module...
pkg/proxy/metrics/metrics_test.go:32:1: Function TestCategorizeTLSError missing the call to method parallel (paralleltest)
func TestCategorizeTLSError(t *testing.T) {
^
pkg/proxy/metrics/metrics_test.go:85:2: Range statement for test TestCategorizeTLSError missing the call to method parallel in test Run (paralleltest)
	for _, tt := range tests {
	^
pkg/proxy/metrics/metrics_test.go:95:1: Function TestIsTLSError missing the call to method parallel (paralleltest)
func TestIsTLSError(t *testing.T) {
^
pkg/proxy/metrics/metrics_test.go:138:2: Range statement for test TestIsTLSError missing the call to method parallel in test Run (paralleltest)
	for _, tt := range tests {
	^
pkg/proxy/metrics/metrics_test.go:148:1: Function TestCountingReader missing the call to method parallel (paralleltest)
func TestCountingReader(t *testing.T) {
^
pkg/proxy/metrics/metrics_test.go:175:2: Range statement for test TestCountingReader missing the call to method parallel in test Run (paralleltest)
	for _, tt := range tests {
	^
pkg/proxy/metrics/metrics_test.go:199:1: Function TestResponseWriterDelegator missing the call to method parallel (paralleltest)
func TestResponseWriterDelegator(t *testing.T) {
^
pkg/proxy/metrics/metrics_test.go:222:2: Range statement for test TestResponseWriterDelegator missing the call to method parallel in test Run (paralleltest)
	for _, tt := range tests {
	^
pkg/proxy/metrics/metrics_test.go:246:1: Function TestResponseWriterDelegator_Unwrap missing the call to method parallel (paralleltest)
func TestResponseWriterDelegator_Unwrap(t *testing.T) {
^
pkg/proxy/metrics/metrics_test.go:255:1: Function TestWithBodyTracking missing the call to method parallel (paralleltest)
func TestWithBodyTracking(t *testing.T) {
^
pkg/proxy/metrics/metrics_test.go:283:2: Range statement for test TestWithBodyTracking missing the call to method parallel in test Run (paralleltest)
	for _, tt := range tests {
	^
pkg/proxy/metrics/metrics_test.go:316:1: Function TestNewProxyErrorHandler missing the call to method parallel (paralleltest)
func TestNewProxyErrorHandler(t *testing.T) {
^
pkg/proxy/metrics/metrics_test.go:334:2: Range statement for test TestNewProxyErrorHandler missing the call to method parallel in test Run (paralleltest)
	for _, tt := range tests {
	^
13 issues:
* paralleltest: 13 

[ntnn]

/retest

infra failure

     assertions.go:1994: Waiting for condition, but got: Get "https://127.0.0.1:44789/clusters/root:e2e-workspace-nknxh/api/v1/namespaces": stream error: stream ID 903; INTERNAL_ERROR; received from peer
    assertions.go:1994: Waiting for condition, but got: Get "https://127.0.0.1:44789/clusters/root:e2e-workspace-nknxh/api/v1/namespaces": stream error: stream ID 909; INTERNAL_ERROR; received from peer
    assertions.go:1994: Waiting for condition, but got: the server was unable to return a response in the time allotted, but may still be processing the request (get namespaces)
    assertions.go:1994: Waiting for condition, but got: Get "https://127.0.0.1:44789/clusters/root:e2e-workspace-nknxh/api/v1/namespaces": stream error: stream ID 975; INTERNAL_ERROR; received from peer
    assertions.go:1994: Waiting for condition, but got: Get "https://127.0.0.1:44789/clusters/root:e2e-workspace-nknxh/api/v1/namespaces": stream error: stream ID 981; INTERNAL_ERROR; received from peer
    assertions.go:1994: Waiting for condition, but got: Get "https://127.0.0.1:44789/clusters/root:e2e-workspace-nknxh/api/v1/namespaces": stream error: stream ID 987; INTERNAL_ERROR; received from peer
    assertions.go:1994: Waiting for condition, but got: the server was unable to return a response in the time allotted, but may still be processing the request (get namespaces)
    assertions.go:1994: Waiting for condition, but got: Get "https://127.0.0.1:44789/clusters/root:e2e-workspace-nknxh/api/v1/namespaces": stream error: stream ID 1035; INTERNAL_ERROR; received from peer
    assertions.go:1994: Waiting for condition, but got: the server was unable to return a response in the time allotted, but may still be processing the request (get namespaces)
    assertions.go:1994: Waiting for condition, but got: Get "https://127.0.0.1:44789/clusters/root:e2e-workspace-nknxh/api/v1/namespaces": stream error: stream ID 1043; INTERNAL_ERROR; received from peer
    assertions.go:1994: Waiting for condition, but got: Get "https://127.0.0.1:44789/clusters/root:e2e-workspace-nknxh/api/v1/namespaces": stream error: stream ID 1049; INTERNAL_ERROR; received from peer