lookup: re-add npm

[Trott]

trafficstars

Checklist

• [x] npm test passes
• [x] contribution guidelines followed here

[codecov-commenter]

Codecov Report

Patch coverage has no change and project coverage change: -1.13% :warning:

Comparison is base (f15c9a2) 96.44% compared to head (98eccdf) 95.32%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #979      +/-   ##
==========================================
- Coverage   96.44%   95.32%   -1.13%     
==========================================
  Files          28       28              
  Lines        2139     2139              
==========================================
- Hits         2063     2039      -24     
- Misses         76      100      +24     

see 3 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[Trott]

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3261/

[Trott]

The tests that set LC_ALL to 'sk' all fail because (I think) that locale is not on our Jenkins servers.

 sh: warning: setlocale: LC_ALL: cannot change locale (sk): No such file or directory
 sh: warning: setlocale: LC_ALL: cannot change locale (sk): No such file or directory
 sh: warning: setlocale: LC_ALL: cannot change locale (sk): No such file or directory
 sh: warning: setlocale: LC_ALL: cannot change locale (sk): No such file or directory
 sh: warning: setlocale: LC_ALL: cannot change locale (sk): No such file or directory
 sh: warning: setlocale: LC_ALL: cannot change locale (sk): No such file or directory
 sh: warning: setlocale: LC_ALL: cannot change locale (sk): No such file or directory

@nodejs/npm Is there an easy way to skip those tests or somehow accommodate another locale?

@nodejs/build Is there an easy way for us to add that locale?

[lukekarrys]

Originally this locale was used in the test environment to ensure the correct sorting behavior when no locale was set in the CLI code: https://github.com/npm/cli/commit/1d092144eaaabff63ac8424b40b2286822be7677.

It's possible that the tests will still pass without it and we can remove it for citgm but keep it in npm/cli to stop any regressions. I will confirm whether this is true or not.

If it is, then there is a package.json config that could be unset before the tests run. I regret that I haven't looked closer yet into npm's failures in citgm, so I'm not sure if this is possible. Is it possible to run a setup command prior to the tests in citgm only? npm pkg delete tap.test-env should do the trick.

[Trott]

I regret that I haven't looked closer yet into npm's failures in citgm, so I'm not sure if this is possible.

I'm not sure, but it might have only just started happening. We've made more than few changes to CITGM.

Is it possible to run a setup command prior to the tests in citgm only? npm pkg delete tap.test-env should do the trick.

I think we'd just need to put it into the "scripts" setting for npm in the CITGM package.json. EDIT: I thought it was already built into CITGM to do that sort of thing, but I think I was wrong.

[Trott]

For comparison, running CITGM on npm using CITGM@8. If this passes, then we'll bisect and figure out what changed in CITGM to cause the failures: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3262/

[Trott]

For comparison, running CITGM on npm using CITGM@8. If this passes, then we'll bisect and figure out what changed in CITGM to cause the failures: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3262/

CITGM@8 fails too, so the problem isn't something that CITGM@9 newly introduces.

[richardlau]

@Trott FYI For a long time npm has been failing in citgm https://github.com/nodejs/citgm/issues/897

[ruyadorno]

Is it possible to run a setup command prior to the tests in citgm only? npm pkg delete tap.test-env should do the trick.

@lukekarrys in order to tweak a value for the generic citgm run, it looks like there are a few possibilities, looking at the current lib/lookup.json I see there are ways to define either a custom install command, or a scripts that can run or a envVar to set an environment variable. Hopefully using one of these can help you get tests passing again on citgm so that we can readd npm.