-
Notifications
You must be signed in to change notification settings - Fork 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
[69-74] KeepGoingStageSpec
#6617
Conversation
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.
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.
Use modern async testkit APIs instead of work-arounds
@@ -231,14 +230,14 @@ public void A_stage_with_keep_going_must_still_be_alive_after_all_ports_have_bee | |||
// PostStop should not be concurrent with the event handler. This event here tests this. | |||
ExpectMsg<EndOfEventHandler>(); |
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.
Change to ExpectMsgAsync
@@ -269,15 +268,14 @@ public void A_stage_with_keep_going_must_still_be_alive_after_all_ports_have_bee | |||
// PostStop should not be concurrent with the event handler. This event here tests this. | |||
ExpectMsg<EndOfEventHandler>(); |
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.
Change to ExpectMsgAsync
@@ -337,6 +334,7 @@ public void A_stage_with_keep_going_must_close_down_earls_if_keepAlive_is_not_re | |||
maybePromise.TrySetResult(0); | |||
ExpectMsg<UpstreamCompleted>(); |
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.
Change to ExpectMsgAsync
Head branch was pushed to by a user without write access
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.
LGTM
Changes
AssertAllStagesStopped
toAssertAllStagesStoppedAsync