Skip to content
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.

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

Regression in 1.6.0 autogenerated test rules #1823

Closed
purkhusid opened this issue Apr 12, 2020 · 6 comments · Fixed by #1854
Closed

Regression in 1.6.0 autogenerated test rules #1823

purkhusid opened this issue Apr 12, 2020 · 6 comments · Fixed by #1854
Labels

Comments

@purkhusid
Copy link
Contributor

Affected Rule

Not so sure if it's only autogenerated rules, but the repro I have is failing in a auto generated test rule.

Is this a regression?

Yes, this works in 1.5.0.

Description

Auto generated rules fails to load resolve module from a relative file during a test run.

🔬 Minimal Reproduction

https://github.com/purkhusid/ts-jest-repro

🔥 Exception or Error


exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //:test
-----------------------------------------------------------------------------
[link_node_modules.js] manifest file /private/var/tmp/_bazel_danielpurkhus/6514ebbf801be819708572b35f1a35af/sandbox/darwin-sandbox/5/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/_test.module_mappings.json
[link_node_modules.js] manifest contents {
  "workspace": "jest_repro",
  "bin": "bazel-out/darwin-fastbuild/bin",
  "root": "npm/node_modules",
  "modules": {
    "jest_repro": [
      "execroot",
      "bazel-out/darwin-fastbuild/bin"
    ]
  }
}
[link_node_modules.js] startCwd /private/var/tmp/_bazel_danielpurkhus/6514ebbf801be819708572b35f1a35af/sandbox/darwin-sandbox/5/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro
[link_node_modules.js] isExecroot false
[link_node_modules.js] resolved node_modules root npm/node_modules to /private/var/tmp/_bazel_danielpurkhus/6514ebbf801be819708572b35f1a35af/sandbox/darwin-sandbox/5/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/npm/node_modules
[link_node_modules.js] cwd /private/var/tmp/_bazel_danielpurkhus/6514ebbf801be819708572b35f1a35af/sandbox/darwin-sandbox/5/execroot/jest_repro
[link_node_modules.js] symlink( node_modules -> /private/var/tmp/_bazel_danielpurkhus/6514ebbf801be819708572b35f1a35af/sandbox/darwin-sandbox/5/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/npm/node_modules )
[link_node_modules.js] mapping hierarchy [{"name":"jest_repro","link":["execroot","bazel-out/darwin-fastbuild/bin"]}]
[link_node_modules.js] symlink( jest_repro -> /private/var/tmp/_bazel_danielpurkhus/6514ebbf801be819708572b35f1a35af/sandbox/darwin-sandbox/5/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/bazel-out/darwin-fastbuild/bin )
bazel node patches enabled. root: /private/var/tmp/_bazel_danielpurkhus/6514ebbf801be819708572b35f1a35af/sandbox/darwin-sandbox/5/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles symlinks in this directory will not escape
FAIL ./lib.test.js
  ● Test suite failed to run

    Cannot find module 'jest_repro/lib' from 'lib.test.js'

      at Resolver.resolveModule (../../../../../node_modules/jest-resolve/build/index.js:259:17)
      at ../../../lib.test.ts:1:1
      at lib.test.js:3:13
      at Object. (lib.test.js:8:3)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        1.085s
Ran all test suites within paths "./lib.test.js".

🌍 Your Environment

Operating System:

  
MacOS 1.14.6
  

Output of bazel version:

  
Build label: 3.0.0
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Mon Apr 6 12:55:48 2020 (1586177748)
Build timestamp: 1586177748
Build timestamp as int: 1586177748
  

Rules_nodejs version:

  
1.6.0
  
@alexeagle
Copy link
Collaborator

Must have been introduced in 1.5.0...1.6.0
let's bisect...

for each commit, say ff0369c

  • git checkout ff0369c
  • bazel build :release
    then in the repro project change the urls for rules_nodejs
    "file:///Users/alx/Projects/rules_nodejs/dist/bin/release.tar.gz"

note, bazel can't know that the repository rule needs to be re-run so I do a bazel clean --expunge between trials

I found the culprit causing the issue was introduced in 5c2f6c1
/cc @gregmagolan

alexeagle added a commit to alexeagle/rules_nodejs that referenced this issue Apr 26, 2020
@alexeagle
Copy link
Collaborator

Imported your repro so we see the breakage here in our CI
#1850

@purkhusid
Copy link
Contributor Author

Thanks for including the bisect steps. I couldn't figure out how to importted tar.gz in my WORKSPACE file . The file://<url> is super handy!

@gregmagolan
Copy link
Collaborator

On my list for this week. Will get a fix into a 1.6.1 release.

gregmagolan added a commit to gregmagolan/rules_nodejs that referenced this issue Apr 29, 2020
…when under runfiles

Resolves bazel-contrib#1823 which had a repro https://github.com/purkhusid/ts-jest-repro.

NB: The repo uses ts_library devmode UMD output so the relative require in the source code will be transformed to an absolute require which will include the workspace name.

In 1.5.0:

```
.../sandbox/darwin-sandbox/23/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/lib.test.js

const lib_1 = require("jest_repro/lib");
```

works as it finds

```
.../sandbox/darwin-sandbox/23/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/node_modules/jest_repro/lib.js
```

where `test.sh.runfiles/jest_repro/node_modules/jest_repro` is a symlink to the root of the output tree setup by the linker:

```
lrwxr-xr-x  1 greg  wheel  165 29 Apr 05:56 .../sandbox/darwin-sandbox/6/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/node_modules/jest_repro -> .../sandbox/darwin-sandbox/6/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro
```

In 1.6.0:

Bug introduced in bazel-contrib#1850 (thanks for tracking down the culprit @alexeagle) where the root symlink is not created properly when under runfiles (such as a jest_test context).

Fix is:

```
          let runfilesPath = modulePath;
          if (runfilesPath.startsWith(`${bin}/`)) {
            runfilesPath = runfilesPath.slice(bin.length + 1);
+          } else if (runfilesPath === bin) {
+            runfilesPath = '';
          }
```

in `link_node_modules.ts`.

NB: This root workspace symlink will be removed in 2.0 as it causes other unrelated issues. PR for that change is in progress bazel-contrib#1797. In 2.0, we recommend either using the prodmode esm output of ts_library for jest_test or switching to ts_project where the output flavor is controlled by your tsconfig. An example of using ts_library esm output of ts_library (output_group 'es6_sources') is `under /internal/linker/test/issue_1823_use_ts_library_esm`. It require babel-jest to transform the esm to commonjs. Alternately, you can control the 'es5_sources' module format of ts_library with the `devmode_module` but this can lead to other issues such as the output no longer being compatible with ts_devserver and karma_web_test which require named-UMD or named-AMD.
gregmagolan added a commit that referenced this issue Apr 29, 2020
…when under runfiles

Resolves #1823 which had a repro https://github.com/purkhusid/ts-jest-repro.

NB: The repo uses ts_library devmode UMD output so the relative require in the source code will be transformed to an absolute require which will include the workspace name.

In 1.5.0:

```
.../sandbox/darwin-sandbox/23/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/lib.test.js

const lib_1 = require("jest_repro/lib");
```

works as it finds

```
.../sandbox/darwin-sandbox/23/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/node_modules/jest_repro/lib.js
```

where `test.sh.runfiles/jest_repro/node_modules/jest_repro` is a symlink to the root of the output tree setup by the linker:

```
lrwxr-xr-x  1 greg  wheel  165 29 Apr 05:56 .../sandbox/darwin-sandbox/6/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/node_modules/jest_repro -> .../sandbox/darwin-sandbox/6/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro
```

In 1.6.0:

Bug introduced in #1850 (thanks for tracking down the culprit @alexeagle) where the root symlink is not created properly when under runfiles (such as a jest_test context).

Fix is:

```
          let runfilesPath = modulePath;
          if (runfilesPath.startsWith(`${bin}/`)) {
            runfilesPath = runfilesPath.slice(bin.length + 1);
+          } else if (runfilesPath === bin) {
+            runfilesPath = '';
          }
```

in `link_node_modules.ts`.

NB: This root workspace symlink will be removed in 2.0 as it causes other unrelated issues. PR for that change is in progress #1797. In 2.0, we recommend either using the prodmode esm output of ts_library for jest_test or switching to ts_project where the output flavor is controlled by your tsconfig. An example of using ts_library esm output of ts_library (output_group 'es6_sources') is `under /internal/linker/test/issue_1823_use_ts_library_esm`. It require babel-jest to transform the esm to commonjs. Alternately, you can control the 'es5_sources' module format of ts_library with the `devmode_module` but this can lead to other issues such as the output no longer being compatible with ts_devserver and karma_web_test which require named-UMD or named-AMD.
gregmagolan added a commit to gregmagolan/rules_nodejs that referenced this issue Apr 30, 2020
…when under runfiles

Resolves bazel-contrib#1823 which had a repro https://github.com/purkhusid/ts-jest-repro.

NB: The repo uses ts_library devmode UMD output so the relative require in the source code will be transformed to an absolute require which will include the workspace name.

In 1.5.0:

```
.../sandbox/darwin-sandbox/23/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/lib.test.js

const lib_1 = require("jest_repro/lib");
```

works as it finds

```
.../sandbox/darwin-sandbox/23/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/node_modules/jest_repro/lib.js
```

where `test.sh.runfiles/jest_repro/node_modules/jest_repro` is a symlink to the root of the output tree setup by the linker:

```
lrwxr-xr-x  1 greg  wheel  165 29 Apr 05:56 .../sandbox/darwin-sandbox/6/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro/node_modules/jest_repro -> .../sandbox/darwin-sandbox/6/execroot/jest_repro/bazel-out/darwin-fastbuild/bin/test.sh.runfiles/jest_repro
```

In 1.6.0:

Bug introduced in bazel-contrib#1850 (thanks for tracking down the culprit @alexeagle) where the root symlink is not created properly when under runfiles (such as a jest_test context).

Fix is:

```
          let runfilesPath = modulePath;
          if (runfilesPath.startsWith(`${bin}/`)) {
            runfilesPath = runfilesPath.slice(bin.length + 1);
+          } else if (runfilesPath === bin) {
+            runfilesPath = '';
          }
```

in `link_node_modules.ts`.

NB: This root workspace symlink will be removed in 2.0 as it causes other unrelated issues. PR for that change is in progress bazel-contrib#1797. In 2.0, we recommend either using the prodmode esm output of ts_library for jest_test or switching to ts_project where the output flavor is controlled by your tsconfig. An example of using ts_library esm output of ts_library (output_group 'es6_sources') is `under /internal/linker/test/issue_1823_use_ts_library_esm`. It require babel-jest to transform the esm to commonjs. Alternately, you can control the 'es5_sources' module format of ts_library with the `devmode_module` but this can lead to other issues such as the output no longer being compatible with ts_devserver and karma_web_test which require named-UMD or named-AMD.
@flolu
Copy link
Contributor

flolu commented Apr 30, 2020

Thanks for fixing it that quickly!

@WesleyYue
Copy link

It looks like this regressed again in 2.0.0. I created a new issue. #2008

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants