Ava: Exit with error if --watch is used in CI

Created on 16 Jan 2017  Â·  14Comments  Â·  Source: avajs/ava

(Rather than ignoring --watch we've decided to exit with an error instead: https://github.com/avajs/ava/issues/1186#issuecomment-274246060 — @novemberborn)


Description

This is a feature request. Skip '--watch' arg if isCi

Since we have known our test running on CI server. Ava should more clever to ignore --watch arg.

Config

Copy the relevant section from package.json:

{
  "scripts": {
    "test": "ava -w"
  }
}

Now I write my test command like this:

{
  "scripts": {
    "test": "[ -z $CI ] && (ava -w); [ $CI ] && (ava)"
  }
}
enhancement good for beginner help wanted

Most helpful comment

To summarize the discussion, the solution would be to raise an error (exiting with 1 to allow stop the process in CI) with the following explanatory message :
AVA will not run with the --watch (-w) option in CI, because CI processes should terminate, and with the --watch option, AVA will never terminate.

Is that the right solution?

I can do the PR if you want (Code, test and explanation in docs)

All 14 comments

I'm not sure I fully understand the code logic of lib/cli.js

Only change this line? https://github.com/avajs/ava/blob/master/lib/cli.js#L148

    if (cli.flags.watch && !isCi) {

This line https://github.com/avajs/ava/blob/master/lib/cli.js#L97 set conf.watch and conf.tag. Then where it going?

This doesn't mean that "test": "ava -w" should be encouraged. Seems like an anti-pattern to me. $npm test should mean the same thing on development and CI, except for inconsequential things, right?

I agree with @mightyiam. Though it might be useful to exit with a warning if you use --watch inside CI.

Yes. "Skipping" watching, with a warning, seems reasonable. But, if anyone is running with -w in CI it most likely means that his test script has ava -w. Shouldn't we "educate" by not letting that pass, and instead provide a kind message?

"Error: AVA will not run with the --watch (-w) option in CI, because CI processes should terminate, and with the --watch option, AVA will never terminate."

@mightyiam I understand what you say. But why not make ava even better?

🚀 Futuristic JavaScript test runner

I'm :+1: better. I just didn't see it as better. Are you using "test": "ava --watch" in packages.json?

I think that it is better to display a super helpful error message when ava is used wrongly, then to allow it to be used wrongly.

But I'm not sure whether it should be a warning or an error in this case.

I also agree with @mightyiam and @novemberborn. We shouldn't silently ignore CLI flag and instead display a help message instead. Plus, in that specific case, we're encouraging the incorrect use of -w.

Much like when --tap and --watch are combined we should print a warning when --watch is used in a CI environment, and then exit.

Warning or error? Warning could allow exiting with 0, while error should mean exiting with 1. I find this distinction important.

ava disable tap reporter in watch mode. And switch to verbose reporter in CI. Why?
Because ava is smart enough to correct that.
Use tap reporter in watch mode is unsuitable or maybe "wrongly".
We're not encouraging the incorrect use of tap also. So we adjust it.

AVA refuses to start when --tap and --watch are combined. It doesn't correct it because that would be misleading. Given that in watch mode the process never exits, I'd find it misleading for that behavior to change in a CI environment.

To summarize the discussion, the solution would be to raise an error (exiting with 1 to allow stop the process in CI) with the following explanatory message :
AVA will not run with the --watch (-w) option in CI, because CI processes should terminate, and with the --watch option, AVA will never terminate.

Is that the right solution?

I can do the PR if you want (Code, test and explanation in docs)

@forresst yes that's my preferred solution. Sorry for not going your way @cncolder, but thank you for raising this ambiguity.

Alright. I'm convinced too. What @forresst proposed above sounds good to me.

Was this page helpful?
0 / 5 - 0 ratings