lookup: add npm

[addaleax]

This was, surprisingly, not in this list. Also, I am not sure, but I guess this shouldn’t be merged as long as npm is broken on node master?

Checklist

• npm test passes
• tests are included
• documentation is changed or added
• contribution guidelines followed here

[MylesBorins]

If I recall correctly npm test suite wasn’t able to run in our CI… hopefully I’m incorrect :D

[addaleax]

If I remember correctly, the main (only?) reason we’re not currently running it is that it makes a lot of network calls, but then again citgm isn’t exactly shy of doing that anyway…

[gibfahn]

I assume we normally want to test the vendored copy of npm rather than latest, which is what gibfahn-test-npm and gibfahn-test-npm-win do (see also nodejs/build#317 and nodejs/node#11540). I was hoping for review from the npm team, but barring that I'll just land it.

My issue with this is that the npm tests have never passed for me on any platform other than macOS, and seem to be flaky there. See the latest runs of the CI jobs for an example. If we're going to have to mark it as flaky everywhere there's not much point in having it.

[BridgeAR]

@gibfahn I guess it would still be useful to have npm run in case the tests pass on OS-X. So we could just skip them everywhere else.

[gibfahn]

@gibfahn I guess it would still be useful to have npm run in case the tests pass on OS-X. So we could just skip them everywhere else.

They're also flaky on macOS...

If someone would like to raise a PR adding npm we can run citgm on it to test. I'm not strongly against adding npm, just skeptical of the benefits.

[BridgeAR]

@gibfahn I am not sure what PR you would want besides this one?

[gibfahn]

@gibfahn I am not sure what PR you would want besides this one?

Sorry, got my wires crossed.

Smoker run: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-nobuild/239/

[BridgeAR]

If we can run the npm tests at least on one system, I still think we should do that.

New smoker run: 10. https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-nobuild/284/ 8. https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-nobuild/285/ Wrong runs... I should get some sleep.

[targos]

on Linux:

error:                       | 4200 passing (7m)                                                                                                                                
error:                       | 11 pending                                                                                                                                       
error:                       | 28 failing          

[codecov-io]

Codecov Report

Merging #489 (97416b5) into main (d0185d9) will decrease coverage by 0.79%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #489      +/-   ##
==========================================
- Coverage   96.12%   95.33%   -0.80%     
==========================================
  Files          31       26       -5     
  Lines         929      900      -29     
==========================================
- Hits          893      858      -35     
- Misses         36       42       +6     

Impacted Files	Coverage Δ
lib/spawn.js	69.23% <0.00%> (-30.77%)	⬇️
lib/match-conditions.js	91.48% <0.00%> (-2.13%)	⬇️
bin/citgm.js	98.00% <0.00%> (-2.00%)	⬇️
bin/citgm-all.js	91.00% <0.00%> (-1.00%)	⬇️
test/fixtures/omg-i-timeout/test.js
test/fixtures/omg-i-pass-with-scripts/build.js
test/fixtures/omg-i-pass-with-scripts/test.js
test/fixtures/omg-i-write-to-tmpdir/test.js
...ixtures/omg-i-pass-with-install-param/test_args.js

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0185d9...97416b5. Read the comment docs.

[targos]

Launched https://github.com/targos/citgm/actions/runs/627105955 to check the status.

[targos]

It's green! Let's try with more platforms: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-pipeline/137/

[richardlau]

The failure on LinuxONE (s390x) is tapjs/tapjs#691, the underlying cause being emscripten currently lacking support for big endian systems (emscripten-core/emscripten#12387).

[targos]

Rebased and skipped s390.

[targos]

New CI: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-pipeline/142/console