fix(examples): fix jest example on windows #2178

lukasholzer · 2020-09-08T14:07:04Z

Fixes #1454

PR Checklist

Please check if your PR fulfills the following requirements:

Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature (please, look at the "Scope of the project" section in the README.md file)
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

Yes
No

Other information

alexeagle · 2020-09-08T17:21:41Z

Failing on CI


INFO: From Testing //:test:
--
  | ==================== Test output for //:test:
  | Error: Can't find a root directory while resolving a config file path.
  | Provided path to resolve: ./jest.config.js
  | cwd: C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\bazel-out\x64_windows-fastbuild\bin\test.bat.runfiles\examples_jest
  | at _default (C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-config\build\resolveConfigPath.js:69:11)
  | at readConfig (C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-config\build\index.js:152:49)
  | at readConfigs (C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-config\build\index.js:373:26)
  | at C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-cli\node_modules\@jest\core\build\cli\index.js:155:58
  | at Generator.next (<anonymous>)
  | at asyncGeneratorStep (C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-cli\node_modules\@jest\core\build\cli\index.js:108:24)

do you know how to fix?

alexeagle · 2020-09-08T17:25:51Z

examples/jest/.bazelrc

@@ -1 +1,6 @@
 import %workspace%/../../common.bazelrc
+
+# Do not build runfile forests by default. If an execution strategy relies on runfile


Interesting, thanks for the links! It seems like this isn't particular to the jest example, right? Maybe we should have this in the /common.bazelrc so it gets picked up by all our tests, as well as new workspaces created with @bazel/create ?

yea! that is a good Idea this will also help normal run files (nodejs_binary) to work on windows.

done. Moved it into the common bazelrc

alexeagle · 2020-09-08T17:26:00Z

examples/jest/BUILD.bazel

-        # Need to set the pwd to avoid jest needing a runfiles helper
-        # Windows users with permissions can use --enable_runfiles
-        # to make this test work
-        "no-bazelci-windows",


nice thanks for removing one of these

lukasholzer · 2020-09-09T07:05:57Z

Failing on CI


INFO: From Testing //:test:
--
  | ==================== Test output for //:test:
  | Error: Can't find a root directory while resolving a config file path.
  | Provided path to resolve: ./jest.config.js
  | cwd: C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\bazel-out\x64_windows-fastbuild\bin\test.bat.runfiles\examples_jest
  | at _default (C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-config\build\resolveConfigPath.js:69:11)
  | at readConfig (C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-config\build\index.js:152:49)
  | at readConfigs (C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-config\build\index.js:373:26)
  | at C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-cli\node_modules\@jest\core\build\cli\index.js:155:58
  | at Generator.next (<anonymous>)
  | at asyncGeneratorStep (C:\users\b\_bazel_b\o3fgohdy\execroot\examples_jest\node_modules\jest-cli\node_modules\@jest\core\build\cli\index.js:108:24)

do you know how to fix?

I'm currently on it – seems that the runfilesHelper has a bug on windows as well (need to debug the wall of rc flags that are used on the ci :D).

lukasholzer · 2020-09-17T10:34:49Z

@alexeagle The updated regex in the launcher.sh fixes all issues where this error message occured: ERROR: Manifest file c:\Users\ADMINI~1\_BAZEL~1\rgyky3dz\execroot\EXAMPL~1\bazel-out\x64_windows-fastbuild\bin\test.bat.runfiles\examples_jest\test.bat.runfiles_manifest does not exist.

The problem is that the msys2 package sometimes writes paths with a lower letter: c:\tmp\qwm2tovj\execroot\dynatr~1\bazel.... This results in going into the if and appending the current working directory so the BAZEL_NODE_PATCH_REQUIRE had always a wrong path on windows. Wich results in that some nodejs_tests where crashing.

lukasholzer · 2020-09-17T10:43:34Z

@alexeagle can you help me with the failing ci – suddenly the whole ci seems to fail. No clue why. Locally it is working well:

alexeagle · 2020-10-16T18:33:58Z

sorry I haven't had time to repro, maybe someone in Slack #javascript can help?

I was finding the run and test commands on a nodejs binary on Windows gave an error message similar to the one mentioned in the following post when --enable_runfiles is passed in on the command line: bazelbuild#2178 (comment) Running with --noenable_runfiles, the tests worked correctly, but other packages in my repo require runfiles. This patch changes the resolution to use the MANIFEST file inside the runfiles folder when runfiles are enabled, which seems to fix the issue.

I was finding the run and test commands on a nodejs binary on Windows gave an error message similar to the one mentioned in the following post when --enable_runfiles is passed in on the command line: bazelbuild#2178 (comment) Running with --noenable_runfiles, the tests worked correctly, but other packages in my repo require runfiles. This patch changes the resolution to use the MANIFEST file inside the runfiles folder when runfiles are enabled, which seems to fix the issue for me.

alexeagle · 2020-11-10T19:40:29Z

hey @lukasholzer I tried to green this up, but still failing on windows:


>>>> FAIL: The node binary 'nodejs_linux_amd64/bin/nodejs/bin/node' not found in runfiles.
--
  | This node toolchain was chosen based on your uname ' x86_64'.
  | Please file an issue to https://github.com/bazelbuild/rules_nodejs/issues if
  | you would like to add your platform to the supported rules_nodejs node platforms. <<<<

could you TAL?

lukasholzer · 2020-11-23T16:29:22Z

@alexeagle I get now a different error locally when running: bazel run //examples:examples_jest

Fixes bazelbuild#1454

alexeagle

thanks!

lukasholzer requested review from alexeagle, gregmagolan and mattem as code owners September 8, 2020 14:07

googlebot added the cla: yes label Sep 8, 2020

lukasholzer mentioned this pull request Sep 8, 2020

Fixes an issue where bazel could not create runfiles on windows. dynatrace-oss/barista#1579

Merged

4 tasks

alexeagle reviewed Sep 8, 2020

View reviewed changes

lukasholzer force-pushed the fix-jest-windows branch from 0589311 to a090e56 Compare September 17, 2020 10:30

lukasholzer requested a review from soldair as a code owner September 17, 2020 10:30

lukasholzer requested a review from alexeagle September 17, 2020 10:39

dae mentioned this pull request Oct 28, 2020

fix: support running with --enable_runfiles on Windows #2258

Closed

12 tasks

alexeagle force-pushed the fix-jest-windows branch from a090e56 to 2463136 Compare November 10, 2020 18:50

lukasholzer requested a review from mrmeku as a code owner November 10, 2020 18:50

alexeagle approved these changes Nov 10, 2020

View reviewed changes

alexeagle force-pushed the fix-jest-windows branch 2 times, most recently from cf80b0b to 1e785bc Compare November 10, 2020 19:14

alexeagle mentioned this pull request Nov 10, 2020

chore: only create runfiles trees when needed #2279

Closed

alexeagle added the need: investigation Requires some digging to determine if action is needed label Nov 10, 2020

lukasholzer force-pushed the fix-jest-windows branch from 1e785bc to 979137d Compare November 23, 2020 16:25

lukasholzer force-pushed the fix-jest-windows branch 2 times, most recently from 23cc9bd to 73aa64e Compare November 26, 2020 10:49

lukasholzer force-pushed the fix-jest-windows branch from 73aa64e to c70ed1d Compare November 26, 2020 16:23

alexeagle force-pushed the fix-jest-windows branch from c70ed1d to daa884d Compare November 27, 2020 18:38

alexeagle removed the need: investigation Requires some digging to determine if action is needed label Nov 27, 2020

lukasholzer force-pushed the fix-jest-windows branch from daa884d to b3c012a Compare November 27, 2020 20:04

lukasholzer mentioned this pull request Nov 27, 2020

Fixes an issue with the runfiles generation under windows for nodejs_test #2314

Closed

12 tasks

fix(examples): fix jest example on windows

20c259a

Fixes bazelbuild#1454

lukasholzer force-pushed the fix-jest-windows branch from b3c012a to 20c259a Compare November 27, 2020 21:04

alexeagle approved these changes Nov 27, 2020

View reviewed changes

alexeagle merged commit cc04f6c into bazelbuild:stable Nov 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(examples): fix jest example on windows #2178

fix(examples): fix jest example on windows #2178

lukasholzer commented Sep 8, 2020 •

edited

alexeagle commented Sep 8, 2020

alexeagle Sep 8, 2020

lukasholzer Sep 9, 2020

lukasholzer Sep 17, 2020

alexeagle Sep 8, 2020

lukasholzer commented Sep 9, 2020

lukasholzer commented Sep 17, 2020

lukasholzer commented Sep 17, 2020 •

edited

alexeagle commented Oct 16, 2020

alexeagle commented Nov 10, 2020

lukasholzer commented Nov 23, 2020

alexeagle left a comment

		@@ -1 +1,6 @@
		import %workspace%/../../common.bazelrc

		# Do not build runfile forests by default. If an execution strategy relies on runfile

fix(examples): fix jest example on windows #2178

fix(examples): fix jest example on windows #2178

Conversation

lukasholzer commented Sep 8, 2020 • edited

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

alexeagle commented Sep 8, 2020

alexeagle Sep 8, 2020

Choose a reason for hiding this comment

lukasholzer Sep 9, 2020

Choose a reason for hiding this comment

lukasholzer Sep 17, 2020

Choose a reason for hiding this comment

alexeagle Sep 8, 2020

Choose a reason for hiding this comment

lukasholzer commented Sep 9, 2020

lukasholzer commented Sep 17, 2020

lukasholzer commented Sep 17, 2020 • edited

alexeagle commented Oct 16, 2020

alexeagle commented Nov 10, 2020

lukasholzer commented Nov 23, 2020

alexeagle left a comment

Choose a reason for hiding this comment

lukasholzer commented Sep 8, 2020 •

edited

lukasholzer commented Sep 17, 2020 •

edited