search

nodejs / citgm

Canary in the Gold Mine
https://www.npmjs.com/package/citgm
Other
562 stars 147 forks source link

npm test targets for win and unix #246

Open gibfahn opened 7 years ago

gibfahn commented 7 years ago

So as part of the process of running modules on Windows, we're running into some issues as the npm test command on many modules is Unix specific.

I can see three possible solutions to this problem:

  1. Work out what commands tests should be using, by finding something that works with Windows and Unix. Raise PRs against the modules to get them to update their test targets. If anyone is using bash-specific test scripts, encourage them to use Node scripts instead.
  2. Work out a different command and npm target (e.g. npm run-script test-win) for windows, raise PRs against the modules adding this command.
  3. Give up and use Git bash (to the extent that this actually resolves the issues).

I'd say 3 is the least work (and it's what we're doing at the moment), but Git bash isn't officially supported by Node for windows, so it's probably not the best way to go.

1 would be the ideal solution, the question is whether there is a standard npm test command that can be used. For npm itself, @iarna suggested this (option 1), which works on windows, but then later suggested this (option 2), which is a separate windows command.

Would love to hear views from someone who's had to deal with this before on how to proceed with this.

bzoz commented 7 years ago

At one point I proposed adding system for overriding default npm test command (https://github.com/nodejs/citgm/commit/081e0398b9d9e0112ecb6c8f7846bd8dd5978e91) and used it to make CITGM run almost all modules on Windows (https://github.com/nodejs/citgm/commit/f1296d52a7591a542ae7395da2790f11c9e3a259). Usual problems are:

I think we will be able to easily overcome those problems.

So, IMHO option 1 would be the best, with option 2 for all those cases when one standard command won’t work. We could start with CITGM https://github.com/nodejs/citgm/issues/276 😉 . Anyhow, landing all those PR will be problematic. As an example, there is an PR for qunit-cli (widely used by modules) to add support for Windows that has been open for almost a year now: https://github.com/devongovett/qunit-cli/pull/12.

As for 3 – we get node.js errors related to the console subsystem from time to time. I’m not sure if CITGM would catch those, but still it could affect the tests.

gibfahn commented 7 years ago

For 2. we already have the script option, we'd just need to extend it (as discussed in https://github.com/nodejs/citgm/pull/278).