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

No code coverage when testing with jest #153

Closed
GV94 opened this issue Jun 3, 2022 · 16 comments · Fixed by #238
Closed

No code coverage when testing with jest #153

GV94 opened this issue Jun 3, 2022 · 16 comments · Fixed by #238
Assignees

Comments

@GV94
Copy link

GV94 commented Jun 3, 2022

I am trying to add code coverage reports to opened PRs in my projects CI pipelines (using rules_js), but I am just getting empty reports.

I stumbled across this example, which is working for me. However, it is using the old ts_project rule from bazel. I forked and ported it to use rules_js (as that is what I am using), and the resulting setup is again getting empty code coverage reports. Is this a problem with rules_js, am I doing something wrong, or is it something else entirely?

Here is a version where code coverage is working (using old ts_project from bazel): https://github.com/GV94/bazel_jest_test_coverage.
Here is the same code ported to use rules_js (empty code coverage reports): GV94/bazel_jest_test_coverage#1.

@bazaglia
Copy link
Contributor

bazaglia commented Jun 3, 2022

I also can't find a way to get code coverage. I tried with my own jest rule and with the one by Aspect. Repro to the issue is possible by running the minimal example in aspect-build/rules_jest, adding the attribute collectCoverage: true to the jest.config.js file.

Also tried on the just-released version 0.10.0 and confirm it's not working either.

Screenshot 2022-06-03 at 19 46 59

@bazaglia
Copy link
Contributor

bazaglia commented Jun 3, 2022

@alexeagle Maybe you have an idea of what's causing this?

@alexeagle
Copy link
Member

Yes, coverage under Bazel is a complex beast and only works in certain conditions that have been carefully curated. So it's not surprising this feature is missing.

@Toxicable has been our JS-bazel expert on this in the past and may have some suggestions.

Though if you're just passing a flag to Jest, I'd expect Jest itself to have the behavior they indicate under that flag, regardless of whether bazel coverage does anything correct before or after the test process spawn to understand the side-effects.

@alexeagle
Copy link
Member

Looks like a bug in Jest :(
https://github.com/alexeagle/jest-coverage-repro

I'm guessing the fix for jest-haste-map for enableSymlinks is not fixed at the right layer, so their coverage collection (maybe using some other library) doesn't respect it.

@bazaglia
Copy link
Contributor

bazaglia commented Jun 4, 2022

Thanks for sharing! I understand there is a bug in Jest and you reproduced it only by using symlinks. I found a merged PR where aparently this was discussed in first place and see you in that discussion too: jestjs/jest#9351

Just a question – how come Jest coverage reports work in rules_nodejs? The problem only happens on rules_js. I'm still trying to get a workaround given the limitation, but it would help if I can get a clue on why it works on rules_nodejs.

@alexeagle
Copy link
Member

That's because the working directory in rules_js is in the bazel-out/arch/bin folder

@gregmagolan
Copy link
Member

In rules_nodejs, it is an explicit feature in the nodejs_binary launcher https://github.com/bazelbuild/rules_nodejs/blob/stable/internal/node/launcher.sh#L463. It runs an lcov merger script after the node process exits and outputs the coverage data to the location & format that bazel expects.

We can add the same feature to the js_binary launcher as well.

@gregmagolan
Copy link
Member

I'll put that on the list for after I finish up the symlink guards so that processes don't escape the sandbox. Someone else could pick it up as well as it is a fairly self-contained feature.

@GV94
Copy link
Author

GV94 commented Jun 7, 2022

In rules_nodejs, it is an explicit feature in the nodejs_binary launcher https://github.com/bazelbuild/rules_nodejs/blob/stable/internal/node/launcher.sh#L463. It runs an lcov merger script after the node process exits and outputs the coverage data to the location & format that bazel expects.

We can add the same feature to the js_binary launcher as well.

Would that fix this issue that I am having?

@alexeagle
Copy link
Member

I think you'd also need Jest to fix that bug in this thread, it doesn't produce any coverage data when run in bazel-out.

@GV94
Copy link
Author

GV94 commented Jun 10, 2022

The problem seems to be jest setting the rootdir to where jest.config.js is during execution (unless it is specified in the rootDir property in jest.config.js). When run through bazel this will be the directory where the symlink is to jest.config.js. When deciding what files to check for coverage collection, it will only consider files that are under the rootdir and filter the others (https://github.com/facebook/jest/blob/main/packages/jest-runtime/src/index.ts#L1230).

During test execution when node imports modules, it will not preserve the symlinks, it will dereference them (since --preserve-symlinks and --preserve-symlinks-main are not present). It is these dereferenced filepaths that end up in the coverage sources collected by jest. The result is that there is a mismatch between what files jest considers when checking coverage (must be under rootdir), and the source files for which coverage is collected.

Adding --preserve-symlinks or --preserve-symlinks-main yourself to node when running jest does not appear to have any effect. Jest does not seem to respect any node flags passed.

@GV94 GV94 changed the title No code coverage support No code coverage when testing with jest Jun 10, 2022
@alexeagle
Copy link
Member

Does that mean you could workaround this with an explicit rootDir property in jest.config.js?
Maybe a fix for #114 would also naturally fix this, by keeping the jest runner from walking symlinks to the wrong place.

@GV94
Copy link
Author

GV94 commented Jun 13, 2022

Potentially. Theoretically it should, but I haven't tried it in practice. We made a quick test where we explicitly set it to the folder where our source code is. This had other implications that made some tests fail (like requiring internal packages created with bazel), but when disabling those tests test coverage worked fine. If you can reliably (across different OS:s/machines) set it to the folder to which the sandbox symlinks points to, it might work fine.

This problem is definitely related to #114, when node starts requiring modules in a jest test file, it is escaping the sandbox.

@alexeagle
Copy link
Member

@bazaglia confirms that now that #114 is fixed, jest can produce the coverage data. It's just a matter of plumbing that through to where Bazel expects now.

@GV94
Copy link
Author

GV94 commented Jun 15, 2022

Yes, we've removed the patch we made for jest and it is working as expected. Thanks!

@GV94 GV94 closed this as completed Jun 15, 2022
@bazaglia
Copy link
Contributor

bazaglia commented Jun 15, 2022

@GV94 there is still no support for integration with Bazel coverage by the rules even though many issues were solved by the PR mentioned above. Maybe good to leave this issue open so this can be tracked?

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

Successfully merging a pull request may close this issue.

5 participants