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

fix: projects should use collectCoverageFrom option #15249

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jadutter
Copy link

@jadutter jadutter commented Aug 7, 2024

TL;DR; running a single project defined in the projects config option fails to correctly use the collectCoverageFrom config option for that project.

Summary

I upgraded my nx repo to 19 and attempted to migrate from groups of tests defined as configurations in project.json, to using jest projects.
However, when I would run a single jest project, it would report the code coverage for the whole nx project instead of only the source code files being tested. This resulted in a non-zero exit code, causing nx to not cache the results (even when all the tests that ran passed).

I tried again with a non-mono repo and got the same result.
From what I can tell, a project config is meant to be nearly identical to the overall config, including using collectCoverageFrom.

I wrote up some tests to illustrate this behavior.

 FAIL  e2e/__tests__/multiProjectRunner.test.ts (9.321 s)
  correctly handle coverage reporting
    no projects option
      ✓ baseline (1044 ms)
      middleware with testNamePatterns
        ✓ without collectCoverageFrom within cli args (969 ms)
        ✓ with collectCoverageFrom within cli args (951 ms)
      controllers with testNamePatterns
        ✓ without collectCoverageFrom within cli args (942 ms)
        ✓ with collectCoverageFrom within cli args (727 ms)
    with projects option
      ✓ test all src code (998 ms)
      test middleware project
        ✕ without collectCoverageFrom within cli args (934 ms)
        ✓ with collectCoverageFrom within cli args (792 ms)
      test controllers project
        ✕ without collectCoverageFrom within cli args (907 ms)
        ✓ with collectCoverageFrom within cli args (670 ms)

I left comments in the test file PROJECT_COVERAGE_BUG_NOTE where the tests are failing, but ought to be passing.

While writing these tests, I came across some unexpected behavior. The stderr was missing some text I'd have expected, and when I ran it manually the text was present. I commented those out and left a POSSIBLE_BUG_NOTE comment.

Test plan

I wrote tests to check combinations of

  • meets code coverage threshold vs not
  • defines projects in the jest config vs not
  • uses testPathPattern in the cli arguments vs not
  • uses collectCoverageFrom in the cli arguments vs not (Contrasts how when the option is defined in a single project it is ignored)

Those tests can be run by

yarn jest e2e/__tests__/multiProjectRunner.test.ts

TODO

This is a draft, providing tests to check if all works as expected. What remains:

  • write a fix
  • update CHANGELOG.md
  • clean up test file after fix is written
    • remove describe.only
    • change from using dir to DIR
  • remove unused code
  • lint

Copy link

CLA Not Signed

Copy link

netlify bot commented Aug 7, 2024

Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 654dec2
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/66b3fbad2f090a0008628b65
😎 Deploy Preview https://deploy-preview-15249--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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 this pull request may close these issues.

1 participant