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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unreliable tests #1123

Merged
merged 1 commit into from Apr 8, 2019

Conversation

Projects
None yet
2 participants
@nathansobo
Copy link
Contributor

commented Apr 8, 2019

This PR attempts intermittent failures of tests that interact with snippets. I think these failures are caused by the following race condition:

  • The tests activate the snippets package first.
  • The snippets package loads bundled snippets, then reads all loaded packages in an async function.
  • The tests then activate the language-test package asynchronously in a waitsFor block.
  • Most of the time, the waitsFor block runs before the async function that reads loaded packages in the snippets package, but this isn't guaranteed.
  • If the waitsFor block runs after the async code in the snippets package, snippets from the language-test package are never loaded and the tests fail.

I think this what is happening. Hopefully I am right and CI will pass now. 馃

@nathansobo nathansobo force-pushed the fix-timing-issues branch from 990d85e to 1d0a98a Apr 8, 2019

Load language-test package before activating snippets in all tests
When the snippets package activates, it reads snippets from all of the 
loaded packages in an async code block after loading bundled snippets. 

Previously, tests were loading the `language-test` package after 
activating snippets, which created a race between the async code block 
that read the loaded packages and the loading of this package. Most of 
the time, the jasmine waitsFor block would load the package before the 
async code in the snippets package scanned the loaded package. 
Sometimes, however, it would not. Hopefully this addresses this 
unreliability by always activating the language package before the 
snippets package to avoid the race.

@nathansobo nathansobo force-pushed the fix-timing-issues branch from 1d0a98a to 2bbbd44 Apr 8, 2019

@nathansobo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

Force-pushed and updated the PR body with a new hypothesis.

@nathansobo nathansobo merged commit 33d48d4 into master Apr 8, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nathansobo nathansobo deleted the fix-timing-issues branch Apr 8, 2019

@nathansobo nathansobo changed the title Attempt to fix unreliable tests Fix unreliable tests Apr 8, 2019

@nathansobo nathansobo self-assigned this Apr 8, 2019

@50Wliu

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

Thank you!! I've been wrangling with this so much. Should close #973, right?

@nathansobo

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2019

Yep. In the future you should always feel free to just close a linked issue that I miss on your own. Thanks for the heads up though! Closes #973.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.