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

[BUG] test/lib/publish.js fails if not ran from a git repo #3842

Closed
1 task done
wraithgar opened this issue Oct 6, 2021 · 3 comments
Closed
1 task done

[BUG] test/lib/publish.js fails if not ran from a git repo #3842

wraithgar opened this issue Oct 6, 2021 · 3 comments
Assignees
Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release

Comments

@wraithgar
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

https://github.com/nodejs/node/pull/40319 cigtm failed cause of a quirk in the way our tests work. Because the snapshots are inside the cli repo itself, when we normally run them there is a gitHead. nodejs tests run the code in a state where there is no .git folder to find, so that attribute is missing and the tests fail.

These tests should be robust enough to handle either case, either deleting the attribute itself or hard setting it without the if statement.

Expected Behavior

No response

Steps To Reproduce

No response

Environment

  • Node: 16.11.0 release candidate
  • npm: 7.24.2
@wraithgar wraithgar added Release 7.x work is associated with a specific npm 7 release Bug thing that needs fixing Needs Triage needs review for next steps labels Oct 6, 2021
@wraithgar wraithgar self-assigned this Oct 6, 2021
@targos
Copy link
Contributor

targos commented Oct 6, 2021

It's also possible to configure CITGM to use a git checkout for npm (the feature exists already)

@wraithgar
Copy link
Member Author

wraithgar commented Oct 6, 2021

It's also possible to configure CITGM to use a git checkout for npm (the feature exists already)

That would get your citgm unblocked but this really should be a test that is robust enough to work in this case. The publish tests aren't the place where we are even checking if the gitHead is set properly when reading a package, they should be discarded imo.

npm8 is the only thing going out this week but I'll get a pr ready and this should be remediated by next week's npm release.

@ruyadorno ruyadorno removed the Needs Triage needs review for next steps label Jan 25, 2022
@ruyadorno
Copy link
Contributor

it looks like this was fixed, thanks @wraithgar 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

No branches or pull requests

3 participants