Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

//internal:test is broken with Bazel's Windows native test wrapper #452

Closed
laszlocsomor opened this issue May 29, 2019 · 8 comments
Closed

Comments

@laszlocsomor
Copy link
Contributor

🐞 bug report

Affected Rule

The issue is caused by the rule:

//internal:test, which is a jasmine_node_test from @npm_bazel_jasmine//:index.bzl

Is this a regression?

Not really, I think it's always been there.

Description

A clear and concise description of the problem...

Bazel 0.27 will enable a feature by default: to use a new test execution mechanism on Windows. Currently (in Bazel 0.26) this features is disabled. The new feature allows faster test execution, and doesn't use Bash. See bazelbuild/bazel#6622 for details.

The bad news is, rules_typescript breaks with this planned change 😢 I fixed it in the past (e50c806), but it's broken again 😿

Currently, I'm stuck: can't figure out where I have to update what.
I think I know the problem. It looks similar to that fixed by bazel-contrib/rules_nodejs@76609c7

And I believe I have to update @npm_bazel_jasmine//src/jasmine_node_test.bzl to use nodejs_test_macro instead of nodejs_test.

But I can't find where to update this file, or how it's pulled in as a dependency :(

🔬 Minimal Reproduction

With Bazel 0.26.0, check out rules_typescript c160db9, and run:

bazel test --incompatible_windows_native_test_wrapper --test_output=errors //internal:test

It fails because the test rule tries to execute a .sh file, which is not an executable file on Windows.

🔥 Exception or Error





🌍 Your Environment

Operating System:

Windows 10

Output of bazel version:

0.26 release

Rules version (SHA):

  

  

Anything else relevant?

@laszlocsomor
Copy link
Contributor Author

Bazel 0.27 will enable a feature by default

Looks like this will NOT happen :(

@alexeagle
Copy link
Contributor

Next time you can just ping me to unwind the silly sync sandwich between this repo and rules_nodejs

Sounds like the right thing is for us to pair a bit to remove the need for the wrapper macro on nodejs_test and nodejs_binary?

@laszlocsomor
Copy link
Contributor Author

Sure, and thanks.
I'll ping you again about this when you're not on vacation.

@laszlocsomor
Copy link
Contributor Author

Ping -- sorry for the long silence, I'm just now picking this up.

The test still fails:

  • Windows 10

  • Bazel 0.28.1

  • PATH=c:\git\mingw64\bin;c:\msys64\usr\bin;c:\src\bazel-releases\current;c:\windows\system32

  • Pass: C:\src\tw-migrations\rules_typescript>bazel --ignore_all_rc_files --output_user_root=c:/b test --noincompatible_windows_native_test_wrapper --experimental_allow_incremental_repository_updates //internal:test

  • Fail: C:\src\tw-migrations\rules_typescript>bazel --ignore_all_rc_files --output_user_root=c:/b test --incompatible_windows_native_test_wrapper --experimental_allow_incremental_repository_updates //internal:test

    INFO: From Testing //internal:test:
    ==================== Test output for //internal:test:
    ERROR(tools/test/windows/tw.cc:1248) ERROR: src/main/native/windows/process.cc(184): CreateProcessW("C:\b\thor5vol\execroot\build_bazel_rules_typescript\bazel-out\x64_windows-fastbuild\bin\internal\test.sh"): %1 is not a valid Win32 application.
    
    ERROR(tools/test/windows/tw.cc:1405) Failed to start test process (arg: C:\b\thor5vol\execroot\build_bazel_rules_typescript\bazel-out\x64_windows-fastbuild\bin\internal\test.sh)
    ================================================================================
    

@laszlocsomor
Copy link
Contributor Author

Linking to bazelbuild/bazel#6622, which is blocked by this

@laszlocsomor
Copy link
Contributor Author

ping

@alexeagle
Copy link
Contributor

dupe of bazel-contrib/rules_nodejs#947, let's fix them together

@laszlocsomor
Copy link
Contributor Author

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants