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

[tests] rewrite doctor tests to dictate the current node version #2681

Merged
merged 11 commits into from
Feb 12, 2021

Conversation

ljharb
Copy link
Contributor

@ljharb ljharb commented Feb 12, 2021

Extracted out from #2654.

isaacs and others added 10 commits February 11, 2021 10:42
If an env script is already defined, run that
instead of the default.

PR-URL: npm#2655
Credit: @isaacs
Close: npm#2655
Reviewed-by: @ljharb
Emphasizing them like we do everywhere else to fix encoding errors and
for consistency.

Also removed the `<String>` from the type definition as we don't do that
anywhere else.

PR-URL: npm#2662
Credit: @ethomson
Close: npm#2662
Reviewed-by: @wraithgar
This brought in @npmcli/installed-package-contents@1.0.7 from
commit e1822cf
This pulls in, installs, and de-dupes our subdependencies.
Notable updates are promise-retry and @npmcli/move-file which
had new versions but we had no way to update and/or dedupe

We also manually removed uuid from our package.json which was
only added in the past to try to get around this same deduping
issue
@ljharb ljharb requested a review from a team as a code owner February 12, 2021 17:07
This decouples our tests from depending on the node version being right, and
allows us to run in all of the environments we need to during CI.

PR-URL: npm#2681
Credit: @ljharb
Close: npm#2681
Reviewed-by: @wraithgar
@ljharb
Copy link
Contributor Author

ljharb commented Feb 12, 2021

did you mean to update this PR so it includes 10 unrelated commits?

@wraithgar
Copy link
Member

No, the pull script did weird things to this PR.

In the release branch though it's ok

~/D/n/cli (release/v7.5.4|✔) $ git diff 3a159d27e976933098ec18fa9c3e474c85b5b332 554d91cdf82e9c92c2ac3752ed91e7081c2271e5 --stat
 test/lib/doctor.js | 1640 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------
 1 file changed, 807 insertions(+), 833 deletions(-)

This PR itself is just gonna look weird.

@wraithgar
Copy link
Member

3a159d2

@wraithgar wraithgar 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 Feb 12, 2021
@wraithgar wraithgar merged commit 3a159d2 into npm:latest Feb 12, 2021
@ljharb ljharb deleted the doctor-tests branch February 12, 2021 19:29
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.

4 participants