-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Investigate instantiating new API and reporter in --watch #533
Comments
@jamestalmage given your refactoring work what's your take on this? |
I actually was thinking we could move watcher support into the API itself. That was kind of the thinking behind the I'm thinking of adapting the run method a little: // new testRun is returned immediately, it may not start receiving events until the current run finishes.
var testRun = api.run(files, {
runOnlyExclusive: boolean,
stopCurrentRun: boolean
});
testRun.on('test', ...)
testRun.result.then(function (result) {
}); |
I like that idea. Ideally the CLI should just be a tiny wrapper around the API, with CLI only stuff. |
Agreed. Our CLI is pretty huge. A lot of it is options processing (parsing the command line and merging with |
The watcher is fairly big though. I wonder if we could make the watcher instantiate its Api. We couldn't before due to how the loggers were coupled. |
I didn't mean "move all of |
@jamestalmage that sound cool. |
The lack of this refactor hasn't been an issue. Closing this, but I expect we'll do something like this eventually as needs dictate. |
#502 resets the API and logger/reporter between runs. Creating fresh instances may be preferable. This would require some sort of code sharing between
cli.js
andwatcher.js
though, since the former sets up the instances and has access to the required CLI flags.One option would be to pass a callback to
watcher.start()
allowing the watcher to create new API and logger/reporter instances, but leaving the definition incli.js
.The text was updated successfully, but these errors were encountered: