xo should run after other test scripts

[stevemao]

You might want to just quickly test but you don't care about stylings for now. For example, you add a console.log or you add some scripts with no indentation, and then you wanna quickly run the test scripts.

[sindresorhus]

I kinda agree. I've been annoyed at having to fix style issues during development too.

@joakimbeng @Qix- @markogresak @dthree Thoughts?

[Qix-]

This is what I was talking about a while back; I wish npm run test bypassed everything else.

EDIT: Meh; you're using && which I personally don't even see as portable. It works on most systems but I don't see it as guaranteed. @sindresorhus what were the arguments for not using pretest or posttest again?

[sindresorhus]

Meh; you're using && which I personally don't even see as portable.

How is it not portable? Even Windows supports it.

what were the arguments for not using pretest or posttest again?

I didn't have any strong arguments. It's just less bloated using && in on run script instead of multiple.

[Qix-]

How is it not portable? Even Windows supports it.

Sure it does, but being the purist I am, I wouldn't rely on it personally. Using pretest/posttest is using the package manager as it was designed. Having xo in one of them signifies Hey, your code should be conformant to our standard before testing or Hey, you can test, but I'm going to check your style after you do.

I understand they're the same, but separating them out seems less hackish. Also, I believe logging helps too (however minutely) because NPM separates them out into logical steps.

Using pretest/posttest is "more correct" in my opinion - it's 100% guaranteed to work on anything. && might have an edge case somewhere where the underlying shell doesn't support && syntax.

[sindresorhus]

&& might have an edge case somewhere where the underlying shell doesn't support && syntax.

I highly doubt that, but I also don't feel strongly about using &&. It just looks nicer. Let's see what the other people say. I'm fine with either.

// @arthurvr @floatdrop @vdemedes

[Qix-]

One last note: By using pretest/posttest, the chances of you modifying a command lessen. Another thing you can't guarantee is that tacking on && xo to the end will still be syntactically/semantically correct on the underlying shell.

Which kind of makes me wish it supported arrays of commands, like Travis does.

[stevemao]

I agree with @Qix-

I just honestly don't see anyone using pretest/posttest

[joakimbeng]

I personally prefer using pretest / posttest as I have mentioned before.

I was hoping one could use the --ignore-scripts flag for this to not run any pre or post scripts, but having this:

{
  "scripts": {
    "lint": "xo",
    "pretest": "npm run -s lint",
    "test": "ava"
  }
}

and running npm test --ignore-scripts doesn't run anything at all...

[Qix-]

I'd normally jump to suggest someone creating a ticket over at NPM but alas I think this has been discussed before. A ticket is welcome and I would support it.

[stevemao]

--ignore-scripts -1 on this coz the npm team have other priorities and it really defeats the purpose of pretest / posttest. If you don't wanna run them just name them differently.

[Qix-]

@stevemao Though the point of the flag would be to, for a single run, skip pre/post tests.

[stevemao]

I know. As I said I think pre/post shouldn't be skipped.

[vadimdemedes]

Personally, I am used to &&, I see nothing wrong with it. And btw, if you want tests to run, even if xo failed:

xo; tape test.js | tap-dot

Use ; instead of &&.

[Qix-]

Use ; instead of &&.

Is that Windows compatible?

[dthree]

Yes, fixing styling issues mid dev was driving me crazy.

vdemedes' suggestion looked promising, however it doesn't seem to be working on Windows.

[Qix-]

I'm 50% sure ; doesn't and 90% sure | definitely doesn't. :/

[joakimbeng]

if you want tests to run, even if xo failed

But this is nothing one wants. As stated in XO's readme:

Never discuss code style on a pull request again!

So therefore is ; not an option even if it works on Windows.

[markogresak]

I'm 50% sure ; doesn't and 90% sure | definitely doesn't. :/

Pipe worked even in Windows XP, source.

But you are correct about ;, Windows equivalent is &. Although that is only correct for CMD, PowerShell interprets it the same as *nix shells.

And regarding the previous discussion, the && in Windows CMD (can't confirm for before XP) works the same as in *nix shells, source. But it does not work in PowerShell.

I've also tested all of the above in my Windows 10 VB and it's still the same.

• Adding && xo to test script will introduce issues with (at least) PowerShell.
• Replacing && with ; would mean the linter will run regardless of test success and the thing will not even work in Windows CMD.
• Another edge case would be a script ending with ; or &, appending && to this would cause an error when shell is parsing the script.

---

I like the idea of adding linter to test step, in my projects I'm running the linter before other tests, sometimes it can prevent failed tests because of something stupid in my code. I don't see the point of using the lint tool after, if the tests pass there are probably no serious errors, it serves just as a "Is the code pretty?" check.

The suggested posttest or pretest could make it easier if there is no existing script with that name, otherwise the main problem is not avoided.
I have tried with multiple pretest scripts, but as expected, only the last defined script (key) is used, the same as with any JS objects.

[Qix-]

So it sounds like we should open a ticket for multiple scripts on NPM.

[sindresorhus]

Adding && xo to test script will introduce issues with (at least) PowerShell.

No, even though the user is on PowerShell, npm scripts are run using the Node.js child_process module which uses cmd.exe on Windows.

[markogresak]

So it sounds like we should open a ticket for multiple scripts on NPM.

This would mean rewriting the whole json parser. I wouldn't want to be near the issues that this will lead to. As said, it's how JS engines works, you can't have multiple keys with same name.

Example:

{
  "test": "...1",
  "test": "...2"
}

Will be parsed (JSON.parse, require(....json)) to:

{ test: "...2" }

Does this make the issue I was trying to point out more clear?

Adding && xo to test script will introduce issues with (at least) PowerShell.

No, even though the user is on PowerShell, npm scripts are run using the Node.js child_process module which uses cmd.exe on Windows.

I didn't know that, when I said I've tested it, I used the actual shell, not a script ran via npm. If that is the case, then using the && will not be a problem.

[sindresorhus]

@markogresak I think @Qix- meant:

{
  "test": [
    "...1",
    "...2"
  ]
}

[markogresak]

npm/npm#6943

[Qix-]

Haha yes, that's indeed what I meant. I left a comment on that issue pointing back to this.

[markogresak]

@Qix- That makes more sense, I haven't even thought of it before @sindresorhus pointed it out. It would be great if they figure out something and not just for this project, but for many workflows out there.

[stevemao]

Any conclusion?

[stevemao]

I found some other useful information. If there is a typo-as-programmer-error, it should be caught by linter (as it's probably faster than running your whole tests, or for whatever reason the test doesn't point you to the right code). In this case, its probably better to run the linter first.

I think they should both always execute no matter what order they are, they could run together asynchronously even one fails.

The tricky part would be how you print the reports. I think a simple bash script may not be ideal then.

[sindresorhus]

Decision time: #8 (comment)

Thanks all for participating in the discussion :)