Change the runtime mocks to support batches and rebasing #16460
Conversation
github-actions
bot
added
area: dds
andre4i
changed the title
| * If flush mode is set to FlushMode.TurnBased, it will send all messages queued since the last time | ||
| * this method was called. Otherwise, calling the method does nothing. | ||
| */ | ||
| public flush?() { |
There was a problem hiding this comment.
The reason for these methods to be optional is because of back-compat. Making them required will break forward compatibility and we'd have to be explicit about it in package.json, which is not something we like to do in main. After merging, a corresponding PR will be made to next which will have these methods appropriately declared and also, we'll get rid of all the other asserts in the code related to this.
There was a problem hiding this comment.
I understand that when it comes to interface definition. But I'm not sure what "?" helps with here. It basically says that MockContainerRuntime and derived classes could overwrite this property to be empty. Is this what we want?
There was a problem hiding this comment.
Not sure I follow, @vladsud. Which interface? This class is exported as-is in the public API. Clients are taking a dependency is on the class, take a look at index.ts. If this method is not optional, we'll technically break forward-compat and we'd need the
"ClassDeclaration_MockContainerRuntimeForReconnection": {"forwardCompat": false}
"ClassDeclaration_MockContainerRuntime": {"forwardCompat": false}
in package.json. Not a big deal but also, it's something we try not to do in main if we can help it.
Do you have an alternative in mind?
Description
Adding a mock to support the new states added with #16176. This will be further integrated into the test pipeline via the fuzz testing harness.
Another (arguably better) approach to #16163