Skip to content

#2261 Fix npm_deps tests on Windows, add a fix for Bazel 9.0.0 #2700

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.

Sign up for GitHub

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

Jump to bottom
Open
Mivr wants to merge 3 commits into
base: main
Choose a base branch
from
Open

#2261 Fix npm_deps tests on Windows, add a fix for Bazel 9.0.0 #2700

Mivr wants to merge 3 commits into from
+114 −50

Conversation

@Mivr
Copy link
Collaborator

@Mivr Mivr commented Jan 27, 2026 •
edited
Loading

Bazel 8.x is not compatible with Windows at the moment due to: bazelbuild/bazel#26701.
This branch fixes some common Windows issues so that the bug formentioned can be observed at all.
Then it introduces a fix for Bazel 9.0.0+ where the needed attribute is present.

Bazel 8.x will not work with rules_js until the fix is backported.

For now this seems unlikely: bazelbuild/bazel#27607

Fixes: #2261

Changes are visible to end-users: no

Test plan

  • Covered by existing test cases

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Mivr Mivr mentioned this pull request Jan 27, 2026
Open
@aspect-workflows
Copy link

aspect-workflows bot commented Jan 27, 2026 •
edited
Loading

Bazel 7 (Test)

⚠️ Buildkite build #11988 failed.

Failed tests (8) 
//js/private/test/image:assert_custom_layers_nomatch_test_package_store_1p_test [k8-fastbuild] 🔗
//js/private/test/image:assert_custom_owner_test_app_test [k8-fastbuild]                       🔗
//js/private/test/image:assert_custom_owner_test_package_store_1p_test [k8-fastbuild]          🔗
//js/private/test/image:assert_default_test_app_test [k8-fastbuild]                            🔗
//js/private/test/image:assert_default_test_package_store_1p_test [k8-fastbuild]               🔗
//js/private/test/image:assert_regex_edge_cases_test_app_test [k8-fastbuild]                   🔗
//js/private/test/image:assert_regex_edge_cases_test_package_store_1p_test [k8-fastbuild]      🔗
//js/private/test/image:checksum_test_test_test [k8-fastbuild]                                 🔗

💡 To reproduce the test failures, run

bazel test //js/private/test/image:assert_custom_layers_nomatch_test_package_store_1p_test //js/private/test/image:assert_custom_owner_test_package_store_1p_test //js/private/test/image:assert_regex_edge_cases_test_app_test //js/private/test/image:assert_default_test_app_test //js/private/test/image:assert_regex_edge_cases_test_package_store_1p_test //js/private/test/image:assert_default_test_package_store_1p_test //js/private/test/image:assert_custom_owner_test_app_test //js/private/test/image:checksum_test_test_test

Bazel 8 (Test)

All tests were cache hits

278 tests (100.0%) were fully cached saving 37s.

Bazel 9 (Test)

All tests were cache hits

278 tests (100.0%) were fully cached saving 35s.

Bazel 7 (Test)

e2e/bzlmod

All tests were cache hits

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

Bazel 7 (Test)

e2e/git_dep_metadata

All tests were cache hits

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

Bazel 7 (Test)

e2e/gyp_no_install_script

All tests were cache hits

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

Bazel 7 (Test)

e2e/js_binary_workspace

All tests were cache hits

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

Bazel 7 (Test)

e2e/js_image_oci

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_link_package

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_link_package-esm

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_link_package-rerooted

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_lock

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_lock_disable_hooks

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_lock_empty

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_lock_exclude_package_contents

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_lock_multi

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_lock_partial_clone

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_lock_replace_packages

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_lock_subdir_patch

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_package_lock

All tests were cache hits

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

Bazel 7 (Test)

e2e/npm_translate_yarn_lock

All tests were cache hits

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

Bazel 7 (Test)

e2e/package_json_module

All tests were cache hits

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

Bazel 7 (Test)

e2e/patch_from_repo

All tests were cache hits

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

Bazel 7 (Test)

e2e/pnpm_lockfiles

All tests were cache hits

37 tests (100.0%) were fully cached saving 4s.

Bazel 7 (Test)

e2e/pnpm_repo_install

All tests were cache hits

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

Bazel 7 (Test)

e2e/pnpm_version

All tests were cache hits

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

Bazel 7 (Test)

e2e/pnpm_workspace

All tests were cache hits

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

Bazel 7 (Test)

e2e/pnpm_workspace_deps

All tests were cache hits

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

Bazel 7 (Test)

e2e/pnpm_workspace_rerooted

All tests were cache hits

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

Bazel 7 (Test)

e2e/repo_mapping

All tests were cache hits

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

Bazel 7 (Test)

e2e/runfiles

All tests were cache hits

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

Bazel 7 (Test)

e2e/stamped_package_json

All tests were cache hits

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

Bazel 7 (Test)

e2e/vendored_node

All tests were cache hits

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

Bazel 7 (Test)

e2e/vendored_tarfile

All tests were cache hits

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

Bazel 7 (Test)

e2e/verify_patches

All tests were cache hits

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

Bazel 7 (Test)

e2e/worker

All tests were cache hits

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

Buildifier      Format

jbedard
jbedard reviewed Jan 27, 2026
View reviewed changes
examples/npm_package/packages/pkg_a/index.js Outdated

function sandboxAssert() {
if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
Copy link
Member

@jbedard jbedard Jan 27, 2026

Choose a reason for hiding this comment

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

If these tests are broken on Windows let's exclude them on Windows, not break the test

Copy link
Collaborator Author

@Mivr Mivr Jan 28, 2026

Choose a reason for hiding this comment

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

Sorry, added an if statement that uses the proper regex per OS

cameron-martin
cameron-martin reviewed Jan 27, 2026
View reviewed changes
npm/private/utils.bzl
# Seems like it won't be cherry picked: https://github.com/bazelbuild/bazel/issues/27607
# Added in commit https://github.com/bazelbuild/bazel/commit/b9bbda939cddab807e34559cb7ee798febfa3861
# If a backport happens this version just needs to be lowered to the 8.x version that has the backport
_SUPPORTS_SYMLINK_TARGET_TYPE = ge("9.0.0")
Copy link

@cameron-martin cameron-martin Jan 27, 2026

Choose a reason for hiding this comment

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

It would be good to add this to bazel_features instead of depending on internals of it.

Copy link
Collaborator Author

@Mivr Mivr Jan 28, 2026

Choose a reason for hiding this comment

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

opened a PR for the addition: bazel-contrib/bazel_features#124

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jbedard jbedard jbedard left review comments

+1 more reviewer

@cameron-martin cameron-martin cameron-martin left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: bazel test //examples/npm_deps:test10 fails on windows with bazel 8+

4 participants

@Mivr @CLAassistant @jbedard @cameron-martin