fix: restartOnFileChange option not restarting the test run #3727
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.
The problem was caused by a race condition in the client JS implementation. When the
restartOnFileChange
option is enabled, the server would send two commands to the browser:stop
andexecute
. Both of these commands navigate the context.stop
navigates to the blank page, whileexecute
navigate to thecontext.html
thus triggering a test run. Theexecute
command would trigger the navigation synchronously, but thestop
command would trigger it on the next event loop tick. As a result, if both commands arrive almost at the same time, their order is effectively reversed: first schedule new execution and then immediately stop it. The bug is resolved by handling both commands synchronously thus ensuring correct order.The setTimeout() call in question was introduced in 15d80f4 to solve #27 and was retained over time. It's not completely clear why the timeout was added, but it does not seem to be relevant.
With
clearContext: true
, karma will reset the context and thus cancel any scheduled navigations as soon as the test framework has reported thecomplete
event. As navigations are canceled they don't affect karma and thus karma does not care about them. It's true that the user may have a test with race conditions (e.g. missingdone
callback), but it is up to them to investigate and fix it.With
clearContext: false
, the same logic applies, that karma does not care about navigations aftercomplete
event as long as they don't break karma itself. Here it gets a bit tricky:context.html
or interact with it at all for that matter until the page reload (see this SO answer). This will break the watch mode, but IMO we can let it be given that the case is quite uncommon, the user will see the browser window displaying something unexpected and the error in the CLI output. The changes in this commit don't make this case any worse and it does not seem to be possible to prevent such problematic navigations with the current state of browsers per https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload:Fixes #3724