CLI: Add file watching option #1123
Conversation
trentmwillis
force-pushed
the
cli-watch
branch
17 times, most recently
from
735bbae
to
89f151f
Compare
trentmwillis
changed the title
|
|
||
| run.watch = function watch() { | ||
| const chokidar = require( "chokidar" ); | ||
| const args = Array.prototype.slice.call( arguments ); |
There was a problem hiding this comment.
We could use a rest parameter instead of slicing the arguments object here.
| console.log( "Restarting..." ); | ||
| } | ||
|
|
||
| run.abort( () => run.apply( null, args ) ); |
There was a problem hiding this comment.
Could use the spread operator here.
There was a problem hiding this comment.
Spread/rest are not supported in Node 4 unfortunately 😞 https://kangax.github.io/compat-table/es6/#test-rest_parameters
bin/run.js
Outdated
| } | ||
| } ); | ||
| } else { | ||
| delete global.QUnit; |
There was a problem hiding this comment.
Any point in extracting this logic to a closure, so that this code isn't duplicated with the above case?
There was a problem hiding this comment.
Good suggestion, I meant to do that but forgot.
| const sig = signal || "SIGINT"; | ||
|
|
||
| // Linux increments the pid by 1 due to creating a new shell instance | ||
| if ( process.platform === "linux" ) { |
There was a problem hiding this comment.
What about OSX or MacOS? Can this be detected any better? Can we do a kill-with-children sort of thing on the process so we don't even need to worry about whether a grandchild process was even spawned?
There was a problem hiding this comment.
We could add more logic and special case per-platform, but I'd rather keep this simple to start and evolve it as needed over time.
The kill-with-child thing isn't really applicable, because we don't want to kill the parent process (which is the test suite running).
|
LGTM, but left some nitpicks just in case you want to address them. |
|
Thanks for the review! |
This PR introduces a
--watchoption to the CLI so that tests will re-run when a file is added, changed, or removed from the working directory. It also ensures that if one of the above occurs while tests are executing, then the current test finishes and does any cleanup (viaafter/afterEachhooks) before restarting.This depends on #1121 and #1124. I recommend not reviewing until both of those PRs are merged, because this PR will be much simpler afterward.
Todo: