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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

agilgur5
Copy link
Member

@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 <agilgur5@gmail.com>
@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
Member 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/artifacts S3/GCP/OSS/Git/HDFS etc area/build Build or GithubAction/CI issues type/tech-debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants