-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Type "rs\n" to restart tests, fixes #871 #3979
Conversation
Although the `--watch` feature is somewhat controversial (see #1780), is there any reason not to make it a little easier to use in the meantime? This adds nodemon's shortcut (`rs\n`) for manually restarting.
@broofa I am searching for ways to improve my heavily disk-laden test executions due to multiple import/require statements, so I thought I would give your branch a test run. This code does not appear to work for me. In a multi-file system where my test requires another file, if I modify the required file and type "rs\n" I see the files re-run, but the required file does not appear to have been reloaded from disk. |
@createthis Can you put up an SSCCE? I tried to repro your issue with the following (very simple) test, but wasn't able to. Modifying ====
Command line:
Looking at the source code, I think this PR is doing the right thing - it calls |
@broofa You right. This was user error. I was editing a |
@boneskull @craigtaub Any comment on the likelihood of this PR being accepted? 'Just wondering if I should bother figuring out how to resolve the conflicts (not quite as trivial a task as I thought initially.) |
thanks!! yes, this would be a good add if you want to work on it. otherwise, maybe someone will pick up where you left off. |
Cool. I'm busy with work for the next month or so, but I'll try to find some time to patch this up. If someone wants to help out with it that'd be cool, too. |
@boneskull Can you take another look at this? I've resolved conflicts and added a short description in "index.md". (Can't seem to build the site, though because node-gyp isn't installing properly for me at the moment. Also, I'm not sure why README.md is showing as changed - that seems to be an artifact of merging master. :-/) |
Small comment, but really it LGTM |
@boneskull @craigtaub I think this ready to go. I'll be out of the country for the next couple of weeks. It'd be nice to see this merged before more conflicts develop. Thanks! |
Ping: "It'd be nice to see this merged before more conflicts develop. Thanks!" |
Tested locally. Seems good. |
Description of the Change
Add a keyboard shortcut (
rs\n
) for manually restarting tests when running the CLI in watch (--watch
) mode.Although the--watch
feature is somewhat controversial (see #1780), hopefully it's okay to make minor improvements while that debate is being hashed out?To test this feature:
mocha -w
to run CLI in watch modeAlternate Designs
This feature mimics the behavior of the popular
nodemon
module, so should be familiar to many developers. (And, in fact, the code is cribbed pretty much straight from nodemon)Why should this be in core?
This is a minor tweak to behavior already in core (
--watch
). Although the watch feature is somewhat controversial (see #1780), hopefully there's no reason not to make minor improvements as long as it's supported.Benefits
Provides behavior that many developers are familiar with in similar tool(s) - specifically
nodemon
.Possible Drawbacks
This may make using stdin in the CLI (watch mode) problematic in the future, however that seems like a pretty abstract concern, and mocha does not appear to do anything with stdin at this time
Applicable issues
None that I'm aware of.
This is an enhancement (semver "minor" release)