-
Notifications
You must be signed in to change notification settings - Fork 79
Singleton: address feedback in #311 #347
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
Conversation
#311 (comment) Maybe #346
| // No leader so singleton is not available, messages sent should be stashed | ||
| ref2.tell(.greet(name: "Charlie-2", _replyTo: replyProbe2.ref)) | ||
| ref3.tell(.greet(name: "Charlie-3", _replyTo: replyProbe2.ref)) | ||
| ref3.tell(.greet(name: "Charlie-3", _replyTo: replyProbe3.ref)) |
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.
bug 🙀
I don't know how the test was passing locally for me (with ActorTestKit(system), it started failing always when I changed to self.testKit(system))...
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.
whoa! typo indeed... 😄
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.
huh yeah unexpected that this had worked 🤔
Good catch!
| if leaderNode != expectedNode { | ||
| throw testKit.error("Expected \(reflecting: expectedNode) to be leader node on \(reflecting: system.cluster.node) but was [\(reflecting: leaderNode)]") | ||
| } | ||
| } |
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.
👍 nice addition
|
|
||
| override func tearDown() { | ||
| self.logCaptureHandler.printIfFailed(self.testRun) | ||
| } |
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.
👍
ktoso
left a comment
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.
Good improvements thanks!
Technically "tiny race" remains I believe -- the cluster may know already that we're leaderless but the singleton may still receive the message we send it before it gets the info that it is leaderless. This definitely improves robustness of this test though - Thanks!
Address review comments in #311.