Skip to content

test: update tests to pnpm v9+#2412

Merged
jbedard merged 1 commit intoaspect-build:mainfrom
jbedard:rm-old-npm-in-tests
Nov 2, 2025
Merged

test: update tests to pnpm v9+#2412
jbedard merged 1 commit intoaspect-build:mainfrom
jbedard:rm-old-npm-in-tests

Conversation

@jbedard
Copy link
Copy Markdown
Member

@jbedard jbedard commented Nov 1, 2025 •
edited
Loading

Upgrade tests to use pnpm v9+ in preparation for dropping <v9 in the next major release.

Changes are visible to end-users: no

Test plan

  • Covered by existing test cases
  • New test cases added

@aspect-workflows
Copy link
Copy Markdown

aspect-workflows Bot commented Nov 1, 2025 •
edited
Loading

Test

All tests were cache hits

293 tests (100.0%) were fully cached saving 48s.

Test

e2e/bzlmod

All tests were cache hits

5 tests (100.0%) were fully cached saving 640ms.

Test

e2e/git_dep_metadata

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/gyp_no_install_script

All tests were cache hits

2 tests (100.0%) were fully cached saving 173ms.

Test

e2e/js_image_oci

All tests were cache hits

1 test (100.0%) was fully cached saving 7s.

Test

e2e/npm_link_package

All tests were cache hits

3 tests (100.0%) were fully cached saving 407ms.

Test

e2e/npm_link_package-esm

All tests were cache hits

3 tests (100.0%) were fully cached saving 484ms.

Test

e2e/npm_link_package-rerooted

All tests were cache hits

3 tests (100.0%) were fully cached saving 536ms.

Test

e2e/npm_translate_lock

All tests were cache hits

3 tests (100.0%) were fully cached saving 919ms.

Test

e2e/npm_translate_lock_disable_hooks

All tests were cache hits

3 tests (100.0%) were fully cached saving 247ms.

Test

e2e/npm_translate_lock_empty

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/npm_translate_lock_exclude_package_contents

All tests were cache hits

1 test (100.0%) was fully cached saving 34ms.

Test

e2e/npm_translate_lock_multi

All tests were cache hits

2 tests (100.0%) were fully cached saving 164ms.

Test

e2e/npm_translate_lock_partial_clone

All tests were cache hits

1 test (100.0%) was fully cached saving 61ms.

Test

e2e/npm_translate_lock_replace_packages

All tests were cache hits

4 tests (100.0%) were fully cached saving 384ms.

Test

e2e/npm_translate_lock_subdir_patch

All tests were cache hits

1 test (100.0%) was fully cached saving 95ms.

Test

e2e/npm_translate_package_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/npm_translate_yarn_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/package_json_module

All tests were cache hits

1 test (100.0%) was fully cached saving 348ms.

Test

e2e/patch_from_repo

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/pnpm_lockfiles

All tests were cache hits

67 tests (100.0%) were fully cached saving 9s.

Test

e2e/pnpm_repo_install

All tests were cache hits

1 test (100.0%) was fully cached saving 950ms.

Test

e2e/pnpm_workspace

All tests were cache hits

15 tests (100.0%) were fully cached saving 3s.

Test

e2e/pnpm_workspace_deps

All tests were cache hits

3 tests (100.0%) were fully cached saving 901ms.

Test

e2e/pnpm_workspace_rerooted

All tests were cache hits

15 tests (100.0%) were fully cached saving 5s.

Test

e2e/repo_mapping

All tests were cache hits

3 tests (100.0%) were fully cached saving 454ms.

Test

e2e/rules_foo

All tests were cache hits

2 tests (100.0%) were fully cached saving 258ms.

Test

e2e/runfiles

All tests were cache hits

1 test (100.0%) was fully cached saving 118ms.

Test

e2e/stamped_package_json

All tests were cache hits

1 test (100.0%) was fully cached saving 44ms.

Test

e2e/vendored_node

All tests were cache hits

1 test (100.0%) was fully cached saving 78ms.

Test

e2e/vendored_tarfile

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/verify_patches

All tests were cache hits

2 tests (100.0%) were fully cached saving 109ms.

Test

e2e/worker

All tests were cache hits

1 test (100.0%) was fully cached saving 35ms.

Test

e2e/workspace

All tests were cache hits

1 test (100.0%) was fully cached saving 35ms.

Buildifier      Format

@jbedard jbedard force-pushed the rm-old-npm-in-tests branch 9 times, most recently from fbed34e to ca82851 Compare November 2, 2025 20:13
jbedard
jbedard commented Nov 2, 2025
View reviewed changes
Comment thread examples/nextjs/package.json
"lint": "next lint"
},
"dependencies": {
"react": "^19.0.0",
Copy link
Copy Markdown
Member Author

@jbedard jbedard Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the versions in the lockfile align better between pnpm 8+9. It seems pnpm v9 changed the ^ to only incremental the last semver specified (which sounds right to me 🤔). Without this some transitive deps change and actually caused this nextjs example to fail.

jbedard
jbedard commented Nov 2, 2025
View reviewed changes
Comment thread package.json
"@babel/cli": "^7.23.0",
"@babel/core": "^7.23.0",
"@babel/plugin-transform-modules-commonjs": "^7.23.0",
"@babel/cli": "^7.28.3",
Copy link
Copy Markdown
Member Author

@jbedard jbedard Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also required to get the versions in the lockfile to align better between pnpm 8+9

@jbedard jbedard force-pushed the rm-old-npm-in-tests branch 6 times, most recently from 38bdc6d to e07e48f Compare November 2, 2025 20:47
@jbedard jbedard requested a review from dzbarsky November 2, 2025 20:49
jbedard
jbedard commented Nov 2, 2025
View reviewed changes
Comment thread examples/linked_consumer/test_pkg_deps_linked.js
}
if (e == null) {
throw new Error('devDependency should NOT be available')
// TODO: see https://github.com/aspect-build/rules_js/issues/2013 and similar issues.
Copy link
Copy Markdown
Member Author

@jbedard jbedard Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<v9 the lockfile dev flag was used (incorrectly) for this feature, >=v9 that flag no longer exists so the (buggy) behaviour is gone but we have nothing replacing it yet...

@jbedard jbedard force-pushed the rm-old-npm-in-tests branch 2 times, most recently from d5ff4e7 to 2b815fc Compare November 2, 2025 21:27
@jbedard jbedard force-pushed the rm-old-npm-in-tests branch from 2b815fc to 4e8d300 Compare November 2, 2025 21:38
jbedard
jbedard commented Nov 2, 2025
View reviewed changes
Comment thread package.json
"pnpm": {
"onlyBuiltDependencies": [
"@aspect-test/c",
"@figma/nodegit",
Copy link
Copy Markdown
Member Author

@jbedard jbedard Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dzbarsky I added a few things here since you approved the PR.

Adding fsevents reduced the diff on the snapshot for that file. Adding @kubernetes/client-node (which I think I did before you approved) is what caused the @kubernetes/client-node packageExtensions entry down below due to pnpm9 deciding transitive deps differently and causing a tsc error with a newer version of @types/node.

@jbedard jbedard marked this pull request as ready for review November 2, 2025 21:40
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Nov 2, 2025
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread js/private/coverage/coverage.js
@jbedard jbedard force-pushed the rm-old-npm-in-tests branch 2 times, most recently from 37bedf3 to 61dd751 Compare November 2, 2025 22:02
@jbedard jbedard force-pushed the rm-old-npm-in-tests branch from 61dd751 to 099f105 Compare November 2, 2025 22:02
@jbedard jbedard mentioned this pull request Nov 2, 2025
Merged
@jbedard jbedard merged commit cbc33b5 into aspect-build:main Nov 2, 2025
199 of 201 checks passed
@jbedard jbedard deleted the rm-old-npm-in-tests branch November 2, 2025 22:23
jbedard added a commit that referenced this pull request Nov 4, 2025
@jbedard
chore: upgrade e2e tests to pnpm v9+ (#2413)
fce2001 
Same as #2412 for all the
e2e tests

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

@dzbarsky dzbarsky dzbarsky approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jbedard @dzbarsky