Skip to content
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

test: fix cross-platform persist test issues #788

Merged

Conversation

no-stack-dub-sack
Copy link
Collaborator

@no-stack-dub-sack no-stack-dub-sack commented Oct 30, 2022

Attempt to fix cross-platform issues for persist test "multiple changes don't cause concurrent persist operations". I noticed that this test passed on my mac, but produced highly inconsistent results on my PC. This commit redistributes wait times by making updates to the store in more quick succession and waits longer periods to allow the 80ms to elapse before asserting on the store's state. Also make some asserts a bit more specific, asserting that the counter is never intermediate change values, as on Windows after 40-50ms from the last change the counter updates to 5 inconsistently and unexpectedly early. This should prove that the counter never has an intermediate value but eliminate failures due to this inconsistency. We still validate the final value of the counter as 5.

* redistribute wait times and more specifically assert that
counter is never intermediate change values to account for
cross-platform wishy-washiness
@vercel
Copy link

vercel bot commented Oct 30, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
easy-peasy ✅ Ready (Inspect) Visit Preview Oct 30, 2022 at 9:01PM (UTC)

Copy link
Collaborator

@jmyrland jmyrland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@no-stack-dub-sack
Copy link
Collaborator Author

@jmyrland did you run this on your machine and confirm it works for you too?

@jmyrland
Copy link
Collaborator

@jmyrland did you run this on your machine and confirm it works for you too?

I can verify that it works on my machine as well 👍

@jmyrland jmyrland merged commit 7a88686 into ctrlplusb:master Nov 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants