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

chore(test): fix flaky test cases #3314

Merged
merged 3 commits into from
May 23, 2019

Conversation

devoto13
Copy link
Collaborator

See separate commits for details on fixed issues.

karma-runner@cc2eff2 introduced different behavior depending on whether socket reconnected or browser reconnected after reload. This lead to the reconnecting test being flaky as depending on the timing it will trigger second test run and result in unexpected output.
@devoto13 devoto13 marked this pull request as ready for review May 21, 2019 21:44
@devoto13 devoto13 closed this May 21, 2019
@devoto13 devoto13 reopened this May 21, 2019
@devoto13 devoto13 changed the title chore(test): fix flaky test cases WIP: chore(test): fix flaky test cases May 21, 2019
devoto13 added 2 commits May 22, 2019 22:16
The issue is that it resulted in the following sequence of actions:
- run karma start
- run karma run
- and then instantly kill karma start command

This resulted in flaky tests because preliminary killing resulted in
incomplete output. In fact test was unnecessary complicated as it was
enough to use runOut, which asserts output of the karma run command.
'data' event handler may be called multiple times, which will result in
multiple calls to karma run. To take it even further its execution is
delayed by setTimeout, which means that subsequent executions will run
while next test scenario is in progress and may mess it up. To prevent
this make sure that karma run is executed only once independently of how
many time 'data' event is fired.
@devoto13 devoto13 changed the title WIP: chore(test): fix flaky test cases chore(test): fix flaky test cases May 22, 2019
@devoto13
Copy link
Collaborator Author

@johnjbarton This is ready for review. Please check separate commits to make review easier. It doesn't really make sense to split commits into separate PRs as they target same issue and have a combined effect.

Hopefully this will make tests more stable and unblock my other PRs.

Copy link
Contributor

@johnjbarton johnjbarton left a comment

Choose a reason for hiding this comment

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

Excellent, thanks!

@johnjbarton johnjbarton merged commit 1205bce into karma-runner:master May 23, 2019
@devoto13 devoto13 deleted the fix-flaky-tests branch May 23, 2019 10:58
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