Use testing/synctest to eliminate waits in tests #528
Open
HadrienPatte wants to merge 2 commits intooracle:masterfrom
Open
Use testing/synctest to eliminate waits in tests #528HadrienPatte wants to merge 2 commits intooracle:masterfrom
testing/synctest to eliminate waits in tests #528HadrienPatte wants to merge 2 commits intooracle:masterfrom
Conversation
Update go to 1.25, the currently used go 1.24 has stopped being supported when go 1.26 got released in February, see [go release policy](https://go.dev/doc/devel/release#policy). Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
`pkg/csi/driver` tests currently take more than 4 minutes to run and cause timeouts in CI (exit code 143, SIGTERM from runner). This is caused by 14 "timeout" sub-tests across `bv_controller_test.go` and `fss_controller_test.go` each blocking for a full testTimeout (15s) of real wall-clock time (~210s total). These tests verify that operations correctly propagate context cancellation when a volume or attachment gets stuck. The mocks simulate an infinite page stream using a non-blocking select with a default branch, causing the caller's pagination loop to spin-wait on the CPU until the context deadline elapses in real time. Since [go 1.25](https://go.dev/doc/go1.25#new-testingsynctest-package), go supports using fake time to reliabily test those scenarios without having to wait for the actual timeout when running the tests. See [Testing concurrent code with testing/synctest](https://go.dev/blog/synctest). This PR updates those tests to use `testing/synctest`: * Update go from 1.24 to 1.25 to have `testing/synctest` support. * Wrap each affected `t.Run` body in `synctest.Test()`, so `context.WithTimeout` and all mock calls run under a fake clock. * Change the five non-blocking `select { default: }` branches in `mock_oci_clients.go` to `select { case <-time.After(mockPollInterval): }`, making goroutines durably block so the fake clock can advance. * Move the shared `context.WithTimeout` in `fss_controller_test.go` inside each `t.Run` (it was incorrectly shared across sub-tests in a loop). Before: ``` ok github.com/oracle/oci-cloud-controller-manager/pkg/csi/driver 231.711s ``` After: ``` ok github.com/oracle/oci-cloud-controller-manager/pkg/csi/driver 1.933s ``` Result: `pkg/csi/driver` tests are 100x faster Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
oracle-contributor-agreement
bot
added
the
OCA Verified
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pkg/csi/drivertests currently take more than 4 minutes to run and cause timeouts in CI (exit code 143, SIGTERM from runner). This is caused by 14 "timeout" sub-tests acrossbv_controller_test.goandfss_controller_test.goeach blocking for a full testTimeout (15s) of real wall-clock time (~210s total).These tests verify that operations correctly propagate context cancellation when a volume or attachment gets stuck. The mocks simulate an infinite page stream using a non-blocking select with a default branch, causing the caller's pagination loop to spin-wait on the CPU until the context deadline elapses in real time.
Since go 1.25, go supports using fake time to reliabily test those scenarios without having to wait for the actual timeout when running the tests. See Testing concurrent code with testing/synctest.
This PR updates those tests to use
testing/synctest:testing/synctestsupport.t.Runbody insynctest.Test(), socontext.WithTimeoutand all mock calls run under a fake clock.select { default: }branches inmock_oci_clients.gotoselect { case <-time.After(mockPollInterval): }, making goroutines durably block so the fake clock can advance.context.WithTimeoutinfss_controller_test.goinside eacht.Run(it was incorrectly shared across sub-tests in a loop).Before:
After:
Result:
pkg/csi/drivertests are 100x faster