chore: fix corepack wiring in integration tests #10044
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
setup_package_manager.sh exports `PATH`, so we need to source it for the isolated corepack to actually get enabled. Otherwise certain npm integration tests can fail, depending on the global npm version and whether corepack's npm shims have been explicitly enabled globally. This hasn't been an issue under GitHub Actions, since actions/setup-node is installing the same version of npm (10.5.0) that the tests expect, but could otherwise become problematic down the road. Fixing this uncovers two new problems around testing under Windows, which this PR also addresses: - the corepack path needs to be POSIX-ified, since `C:\Users\RUNNER~1\...` contains backslashes and the name separator (:) - corepack uses `PATHEXT` to determine the casing of its shims (e.g. npm.cmd vs npm.CMD), whereas node always uses lowercase ... while we could update all the tests to accept both styles, the easiest thing to do is just to downcase `PATHEXT`.
turbo-orchestrator
bot
added
the
needs: triage
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@jenseng is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
chris-olszewski
approved these changes
Feb 27, 2025
chris-olszewski
added a commit
that referenced
this pull request
Feb 27, 2025
This reverts commit 670146e.
chris-olszewski
added a commit
that referenced
this pull request
Feb 27, 2025
Reverts #10044 This seems to be causing failures https://github.com/vercel/turborepo/actions/runs/13571078355/job/37935992392?pr=9995#step:9:1167 Failures seem to be flakey: [attempt 1](https://github.com/vercel/turborepo/actions/runs/13570184039/attempts/1) [attempt 2](https://github.com/vercel/turborepo/actions/runs/13570184039/attempts/2)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Spun out from #10023 (comment)
setup_package_manager.sh exports
PATH, so we need to source it for the isolated corepack shims to actually get used in tests. Otherwise certain npm integration tests can fail, depending on the global npm version and if corepack's npm shims haven't been explicitly enabled globally. This can lead to false positives and wasted time debugging integration tests locally.This hasn't been an issue under GitHub Actions, since actions/setup-node is installing the same version of npm (10.5.0) that the tests expect, but could otherwise become problematic down the road.
Fixing this uncovers two new problems around testing under Windows, which this PR also addresses:
C:\Users\RUNNER~1\...contains backslashes and the name separator (:)PATHEXT(via cmd-extension) to determine the casing of its shims' file extension (e.g. npm.cmd vs npm.CMD), whereas node's bundled npm.cmd is always lowercase ... while we could update all the tests to accept both styles, the easiest thing to do is just to downcasePATHEXT.Testing Instructions
Integration tests 🥳