-
Notifications
You must be signed in to change notification settings - Fork 273
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
Add suspend/resume tests #3028
Add suspend/resume tests #3028
Conversation
Thread.Sleep(1000); | ||
|
||
var orchestrationDetails = await DurableHelpers.GetRunningOrchestrationDetailsAsync(statusQueryGetUri); | ||
Assert.Equal("Running", orchestrationDetails.RuntimeStatus); |
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.
Rather than sleeping and hoping that the status will change after the sleep interval, we can make these checks a bit more reliable and potentially faster if we use a polling pattern to check for the state change. This would address the very common flakey test problem where engineers need to increase sleep durations because things ran slower than we expected. Consider creating a helper method that can wait for an orchestration to transition from one state to another (Pending --> Running, Running --> Suspended, Suspended --> Running, etc.)
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.
Done. Feel free to critique my implementation
try | ||
{ | ||
using HttpResponseMessage suspendResponse = await HttpHelpers.InvokeHttpTrigger("SuspendInstance", $"?instanceId={instanceId}"); | ||
await AssertRequestFailsAsync(suspendResponse); |
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.
A couple things:
- The method
AssertRequestFailsAsync
assumes that the request is a termination request, but you're sending a suspend request. - What's the expected behavior for in-proc? Do we throw if you try to suspend a suspended orchestration? I was hoping we wouldn't, but if we are, then checking for a failure message is fine.
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 behavior is the same, but I've updated the parameters and variable names to reflect this.
- Yes, we throw in the same way for in-proc (except with a more verbose exception message, as described in the bug linked in the comments)
Adds testing for suspend/resume in Durable
Pull request checklist
pending_docs.md
release_notes.md
/src/Worker.Extensions.DurableTask/AssemblyInfo.cs
dev
andmain
branches and will not be merged into thev2.x
branch.