[no-issue]: Reduce flakiness in sync queue related tests #4170
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Code is still a bit unreliable, but now should properly wait for changes to be committed before continuing
IgorNadj
reviewed
Sep 15, 2022
| @@ -74,6 +75,10 @@ describe('GET /changes/count', async () => { | |||
|
|
|||
| // Add some more questions | |||
| await oneSecondSleep(); | |||
There was a problem hiding this comment.
Can we get rid of oneSecondSleep now? :)
There was a problem hiding this comment.
I don't think we can 😞 The ChangeHandler won't schedule an execution until it receives a notification from the database that a change has occurred. If for some reason it takes the database longer to send a notification than it does for node to get to the next line, then calling waitForAllChangeHandlers() will complete instantly as no ChangeHandlers have been scheduled.
There's multiple levels of async stuff going on here 😅 It might just work though... I'm not really sure... but I don't think it'd be reliable...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since the change to making the ChangeHandlers run in a transaction, these sync queue related tests have all become a bit more flaky (eg. here, and here). They have previously been relying purely on timing at points, which isn't really all that safe. I've added a bunch of
await database.waitForAllChangeHandlers();calls in places where they seemed necessary, which should halt the test execution until the change transaction is complete.