Conversation
tillrohrmann
left a comment
There was a problem hiding this comment.
Thanks for creating an e2e test for the time travel functionality. The changes look good to me. I left a few clarifying questions for my own understanding. +1 for merging once the feature lands.
| .containsExactlyInAnyOrder( | ||
| "Command: Input", | ||
| "Command: Call", | ||
| "Notification: CallInvocationId", |
There was a problem hiding this comment.
For my own understanding: When are we returning from val firstMethodResponse = TimeTravelTestCalleeServiceClient.fromContext(ctx).firstMethod()? Before or after the invocation id was decided?
There was a problem hiding this comment.
before, getting the invocation id is an async operation
| "Notification: Run", | ||
| "Command: Call", |
There was a problem hiding this comment.
For my own understanding: The notification is allowed to move past Command: Call, right? Is it generated once the run closure completes and persists the value?
There was a problem hiding this comment.
yes, that's why containsExactlyInAnyOrder
| // Set shouldFail to false so the handler will succeed after time travel | ||
| TimeObject.shouldFail.set(false) | ||
|
|
||
| // Use the time travel API to trim entry index 2 |
There was a problem hiding this comment.
We seem to trim trimIndex and not a fixed index 2.
| "Command: SetState", | ||
| "Command: Run", | ||
| "Notification: Run", | ||
| "Command: Call", |
There was a problem hiding this comment.
For my own understanding: Will the SDK send the commands in creation order (as the awaitables are created in the code)? I would assume so because otherwise it's probably hard to match them on replay with existing journal entries.
There was a problem hiding this comment.
Will the SDK send the commands in creation order
yes but the notification order is established by the runtime
No description provided.