Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(test): add a sleep to TestLoadToStream #13037

Closed

Conversation

agilgur5
Copy link
Contributor

@agilgur5 agilgur5 commented May 12, 2024

Motivation

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

Verification

Test passes more often

- 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 for a while, but just experienced it again, so nope

- also fix incorrect comment in `when.go` -- [`defaultTimeout` defaults to 60s](https://github.com/argoproj/argo-workflows/blob/0c1398ea6fa66cc77156f15a6d69c260c6b524da/test/e2e/fixtures/e2e_suite.go#L48), not 30s

Signed-off-by: Anton Gilgur <[email protected]>
@agilgur5 agilgur5 added area/build Build or GithubAction/CI issues area/artifacts S3/GCP/OSS/Git/HDFS etc type/tech-debt labels May 12, 2024
@Joibel Joibel self-requested a review June 7, 2024 13:23
@Joibel Joibel self-assigned this Jun 7, 2024
@Joibel
Copy link
Member

Joibel commented Jun 7, 2024

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

@agilgur5
Copy link
Contributor Author

agilgur5 commented Jun 7, 2024

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

@agilgur5 agilgur5 marked this pull request as draft June 22, 2024 17:44
@Joibel
Copy link
Member

Joibel commented Jul 19, 2024

Replaced by #13366

@Joibel Joibel closed this Jul 19, 2024
@agilgur5 agilgur5 added the solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) label Jul 19, 2024
@agilgur5 agilgur5 deleted the fix-test-load-to-stream-fs-race branch July 19, 2024 12:04
@argoproj argoproj locked as resolved and limited conversation to collaborators Sep 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/artifacts S3/GCP/OSS/Git/HDFS etc area/build Build or GithubAction/CI issues solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) type/tech-debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants