fix(test): add a `sleep` to `TestLoadToStream`

agilgur5 commented 1 month ago

Motivation

it very infrequently has a test flake in it
- have had this in my git stash since at least September, and only hit this a few times in those ~9 months
- I thought it had been fixed by some other changes (#11753 in particular) for a while, but just experienced it again (and twice more), so nope

Error log:

``` --- FAIL: TestLoadToStream (0.01s) --- FAIL: TestLoadToStream/Success (0.01s) load_to_stream_test.go:99: Error Trace: /home/runner/work/argo-workflows/argo-workflows/workflow/artifacts/common/load_to_stream_test.go:99 Error: Not equal: expected: 25 actual : 26 Test: TestLoadToStream/Success ```

Modifications

also fix incorrect comment in when.go -- defaultTimeout defaults to 60s, not 30s

Verification

Test passes more often

Joibel commented 1 week ago

@agilgur5 - this is still failing in the test it's supposed to fix.

agilgur5 commented 1 week ago

Yes it actually fails even worse. So there is definitely a race in this test, but I hadn't had time to figure out where.

Feel free to take over the PR

argoproj / argo-workflows