Fix flaky test #1158
Fix flaky test #1158
Conversation
|
I've fixed #1154 as well. Also here, the issues might have been caused by timing issues. Now the fake server returns a 504 error, which will trigger a retry internally. |
| function waitSniffEnd (callback) { | ||
| if (client.transport._isSniffing) { | ||
| setTimeout(waitSniffEnd, 500, callback) | ||
| } else { | ||
| callback() | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Didn't know about client.transport._isSniffing Seems like a nice trick. I was dealing with the same, so what I did, I just ignored the last sniff. Because even when expecting it to happen 3-times (2 nodes killed) or 2-times (1 node killed), it had sometimes a chance to be called one more time.
500 seems like enough of waiting time
There was a problem hiding this comment.
500 seems like enough of waiting time
500 is just a random number, it could also be 50, the point here is that I'm not waiting for 500 and then doing something, but I'm checking a value that represents what I need every 500ms before starting the next operation. In this way the test code is more reliable :)
| const node = nodes[id] | ||
| delete nodes[id] | ||
| delete sniffResult.nodes[id] | ||
| node.server.stop(callback) |
There was a problem hiding this comment.
I realized later that sniff result endpoint is mocked for the tests. So if I waited or didn't wait, I think I was getting the same results. But I think in case of resurrect it is needed to wait actually.
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-5.x 5.x
# Navigate to the new working tree
cd .worktrees/backport-5.x
# Create a new branch
git switch --create backport-1158-to-5.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick fd8f02b9df9371bb2eae204881abcf9940eac31e
# Push it to GitHub
git push --set-upstream origin backport-1158-to-5.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.xThen, create a pull request where the |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-6.x 6.x
# Navigate to the new working tree
cd .worktrees/backport-6.x
# Create a new branch
git switch --create backport-1158-to-6.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick fd8f02b9df9371bb2eae204881abcf9940eac31e
# Push it to GitHub
git push --set-upstream origin backport-1158-to-6.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-6.xThen, create a pull request where the |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-7.x 7.x
# Navigate to the new working tree
cd .worktrees/backport-7.x
# Create a new branch
git switch --create backport-1158-to-7.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick fd8f02b9df9371bb2eae204881abcf9940eac31e
# Push it to GitHub
git push --set-upstream origin backport-1158-to-7.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-7.xThen, create a pull request where the |
As titled.
The root cause of the issue was that we were testing with timers, and timers are not reliable between different platforms because they can cause timing issues like in this test. I've refactored the code to not depend on timers, but mock the
Dateconstructor instead and use the internal state of the client to figure out if a sniff operation is still running or not.A big thanks to @penge for his initial research.
Closes: #1108 #1142 #1154