Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fix npm run test #2225

Closed
wants to merge 1 commit into from
Closed

Conversation

ruyadorno
Copy link
Contributor

Fixes two issues that enable running tests downstream in nodejs:

  • Running tests using npm run-script test instead of the npm test alias will result in a failure since test/lib/npm.js have an assertion for "test" being the command only. This changes it so that this particular test also accepts "run-script" as the command.
  • Explicitly enable colors for tap output so that running the test suite in a non-tty environment will result in output that matches coloured snapshots.

Fixes two issues that enable running tests downstream in nodejs:
- Running tests using `npm run-script test` instead of the `npm test`
alias will result in a failure since `test/lib/npm.js` have an assertion
for "test" being the command only. This changes it so that this
particular test also accepts "run-script" as the command.
- Explicitly enable colors for tap output so that running the test suite
in a non-tty environment will result in output that matches coloured
snapshots.
@ruyadorno ruyadorno requested a review from a team as a code owner November 23, 2020 22:24
@ruyadorno ruyadorno added Release 7.x work is associated with a specific npm 7 release release: next These items should be addressed in the next release labels Nov 23, 2020
@ruyadorno ruyadorno self-assigned this Nov 24, 2020
@ruyadorno ruyadorno added this to the OSS - Sprint 20 milestone Nov 24, 2020
@ruyadorno ruyadorno closed this in 14c3f6f Nov 27, 2020
wraithgar added a commit that referenced this pull request Apr 6, 2022
The original PR #2225 indicated that this
would prevent snapshot failures in CI with no color, but I hard set this
to "0" and ran tests and they passed.  This will make debugging tests in
CI environments like the Node.js CITGM much easier.
wraithgar added a commit that referenced this pull request Apr 6, 2022
The original PR #2225 indicated that this
would prevent snapshot failures in CI with no color, but I hard set this
to "0" and ran tests and they passed.  This will make debugging tests in
CI environments like the Node.js CITGM much easier.

The reason for the failures is because even if we hard set environment
variables in tests to force color one way or another, chalk has already
made up its mind so changing those in the same process won't affect
anything.  The fix is to hard set two values in the actual `npm test`
script to make `chalk` and `supports-color` do what we need.

The `sudo` test commands were also removed because we do not want to
encourage running npm w/ sudo.
wraithgar added a commit that referenced this pull request May 5, 2022
The original PR #2225 indicated that this
would prevent snapshot failures in CI with no color, but I hard set this
to "0" and ran tests and they passed.  This will make debugging tests in
CI environments like the Node.js CITGM much easier.

The reason for the failures is because even if we hard set environment
variables in tests to force color one way or another, chalk has already
made up its mind so changing those in the same process won't affect
anything.  The fix is to hard set two values in the actual `npm test`
script to make `chalk` and `supports-color` do what we need.

The `sudo` test commands were also removed because we do not want to
encourage running npm w/ sudo.
wraithgar added a commit that referenced this pull request May 5, 2022
The original PR #2225 indicated that this
would prevent snapshot failures in CI with no color, but I hard set this
to "0" and ran tests and they passed.  This will make debugging tests in
CI environments like the Node.js CITGM much easier.

The reason for the failures is because even if we hard set environment
variables in tests to force color one way or another, chalk has already
made up its mind so changing those in the same process won't affect
anything.  The fix is to hard set two values in the actual `npm test`
script to make `chalk` and `supports-color` do what we need.

The `sudo` test commands were also removed because we do not want to
encourage running npm w/ sudo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release: next These items should be addressed in the next release Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants