-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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): ensure workflow has completed before check its phase. Fixes #12906 #12927
Conversation
Signed-off-by: AlbeeSo <suyashi1321@163.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verification
Are you sure this fixes the tests? Did you test the E2Es extensively with no failures?
In particular, this change is similar to the default, see in-line comment below
@@ -141,7 +141,7 @@ func (s *SignalsSuite) TestSignaled() { | |||
Workflow("@testdata/signaled-workflow.yaml"). | |||
When(). | |||
SubmitWorkflow(). | |||
WaitForWorkflow(). | |||
WaitForWorkflow(fixtures.ToBeCompleted). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the default is ToBeDone
, which is a subset of ToBeCompleted
which should be equivalent in this case, so I wouldn't think this would change anything
This PR has been automatically marked as stale because it has not had recent activity and needs further changes. It will be closed if no further activity occurs. |
This PR has been closed due to inactivity and lack of changes. If you would like to still work on this PR, please address the review comments and re-open. |
Fixes #12906
Motivation
TestSignaled
andTestSignaledContainerSet
failed sometimes as check phase of a workflow before it is completed.Modifications
Use
fixtures.ToBeCompleted
to ensure workflow has completed.Verification