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.

Sign up for GitHub

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

Jump to bottom

fix: Flakey test about lifecycle hooks #11534

Merged
merged 12 commits into from
Aug 7, 2023
Merged

fix: Flakey test about lifecycle hooks #11534

merged 12 commits into from
Aug 7, 2023

Conversation

toyamagu-2021
Copy link
Member

@toyamagu-2021 toyamagu-2021 commented Aug 6, 2023 •
edited
Loading

Signed-off-by: toyamagu-2021 toyamagu2021@gmail.com

Relates #10807

Motivation

  • Fix flaky tests about lifecycle hooks

Modifications

Verification

Succeed 100% in 4x2+1=9 times

@toyamagu-2021 toyamagu-2021 force-pushed the fix-flaky-test-lifecycle-hook branch 3 times, most recently from 90d5cbd to 4076420 Compare August 6, 2023 11:34
@toyamagu-2021
Copy link
Member Author

toyamagu-2021 commented Aug 6, 2023 •
edited
Loading

Docs are failing by 500 error in Node server, so I will retry later.

@toyamagu-2021 toyamagu-2021 force-pushed the fix-flaky-test-lifecycle-hook branch 2 times, most recently from 0a989c2 to 284bffb Compare August 7, 2023 11:47
@toyamagu-2021
fix: Flaky test about lifecycle hooks
46c1a21 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
chore: empty commit
3e99181 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
chore: empty commit
2924e2e 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
chore: empty commit
c2ba618 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
chore: empty commit
bf47e8f 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
chore: empty commit
f07a3cb 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
chore: empty commit
bb18394 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
chore: empty commit
34a1466 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
chore: empty commit
c10ad0c 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
chore: commit empty
14f069a 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
fix: wait after create cm
1ec76f7 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021
fix: increase sleep time
020f6ca 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@toyamagu-2021 toyamagu-2021 marked this pull request as ready for review August 7, 2023 13:59
agilgur5
agilgur5 reviewed Aug 7, 2023
View reviewed changes
Copy link
Member

@agilgur5 agilgur5 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question, otherwise LGTM. Thanks for fixing some of the flaky tests, that's very helpful!!!

test/e2e/hooks_test.go
container:
image: alpine:latest
image: argoproj/argosay:v2
Copy link
Member

@agilgur5 agilgur5 Aug 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was the purpose of changing some of these from Alpine to argosay? didn't see a mention of that in the PR description

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Not essential, just the same as the others.
https://github.com/argoproj/argo-workflows/blob/20d0923611f1df6b7147c3547aeeff6b6bfecf18/test/e2e/hooks_test.go#L89C1-L99

Copy link
Member

@agilgur5 agilgur5 Aug 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah consistency with this rest of this file, gotcha.
I searched the rest of the codebase and found that alpine:latest is used with some frequency.
So didn't think that this one particularly stood out, but in the context of the file, it appears to have been inconsistent. Thanks for the clarification!

@terrytangyuan terrytangyuan merged commit 4a3cb0e into argoproj:master Aug 7, 2023
29 checks passed
@toyamagu-2021 toyamagu-2021 deleted the fix-flaky-test-lifecycle-hook branch August 7, 2023 23:20
@terrytangyuan terrytangyuan mentioned this pull request Aug 9, 2023
Closed
45 tasks
terrytangyuan pushed a commit that referenced this pull request Aug 11, 2023
@toyamagu-2021 @terrytangyuan
fix: Flaky test about lifecycle hooks (#11534)
83756dc 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
@agilgur5 agilgur5 added area/build Build or GithubAction/CI issues area/hooks labels Oct 18, 2023
@agilgur5 agilgur5 changed the title fix: Flaky test about lifecycle hooks fix: Flakey test about lifecycle hooks Mar 24, 2024
@toyamagu-2021 toyamagu-2021 mentioned this pull request Mar 30, 2024
Merged
dpadhiar pushed a commit to dpadhiar/argo-workflows that referenced this pull request May 9, 2024
@toyamagu-2021 @dpadhiar
fix: Flaky test about lifecycle hooks (argoproj#11534)
e659de9 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agilgur5 agilgur5 agilgur5 left review comments

@terrytangyuan terrytangyuan terrytangyuan approved these changes

Assignees
No one assigned
Labels
area/build Build or GithubAction/CI issues area/hooks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@toyamagu-2021 @terrytangyuan @agilgur5