Fix racing tests. #2290
Merged
Fix racing tests. #2290
Conversation
This file contains hidden or 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
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.
Fixes two tests which flapped incessantly, causing endless CI headaches.
@finos/perspective-cli's test suite must start and stop a Perspective server, which causes some log spam as the browser and server are consecutively shutdown. These errors are innocuous in this case as they occur after the server is in a shutdown-prepare state, but the logging itself is important as the conditionView method cancelledcan occur in other contexts unexpectedly. Moving this initialization out of the test prevents it from failing.https://github.com/finos/perspective/actions/runs/5427137814/jobs/9870282667
perspective-python's recent multi-threading support added tests to check for concurrency race conditions, and uncovered one in CI. Debugging these failing tests uncovered thatupdate()calls were being conflated correctly, but the internal state which "marked" aTableas having pending updates that needed to be applied was subject to race condition, which left the engine in a state where some updates at the end of a long stream of subsequent updates may become "stuck" in this state, pending application to the table but without the flag set to trigger the engine to apply it. In practice, the nextupdate()call will flush the pending ones, but it still means an update may not resolve in the correct order (or for a long time and/or appear stuck). As the tests don't ever interact with the engine again after the test setup is applied, this test failed infrequently, making it difficult to detect until a full build was run.https://github.com/finos/perspective/actions/runs/5448608452/jobs/9912906432