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

chore: expose jest coverage reports for CI runs @W-14785065 #3939

Merged
merged 6 commits into from
Jan 11, 2024

Conversation

wjhsf
Copy link
Contributor

@wjhsf wjhsf commented Jan 10, 2024

Details

This adds the jest coverage summary to the GitHub Actions job summary and exposes the full coverage reports as artifacts.

screenshot of the unit test job summary with the jest coverage stats

Does this pull request introduce a breaking change?

  • ✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • ✅ No, it does not introduce an observable change.

GUS work item

W-14785065

@wjhsf wjhsf requested a review from a team as a code owner January 10, 2024 02:32
Comment on lines +75 to +77
# 1. Remove leading/trailing "border" lines from output
# 2. Wrap file names in backticks
# 3. Convert leading whitespace to non-breaking to approximate plaintext output
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just trying to bludgeon the CLI output into a passable markdown table. Happy to consider alternative bludgeoning.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an impressively gnarly sed command, but it's not a big deal since this is only for CI. If we really wanted to improve the code for this, though, we might:

  1. Use the json reporter
  2. Write our own .js script to process it and print out markdown
  3. (Ideally) apply it to the Karma tests too, since they use Istanbul as well and I'm sure it can be configured to output JSON

That said, that might be over-engineering for this feature. It's just great to see the coverage report in the GitHub UI!

.github/workflows/unit.yml Outdated Show resolved Hide resolved
// Jest's default reporters are [clover, json, lcov, text]. We add the second text reporter to
// use the output as the step summary when running tests in GitHub Actions. (Ideally, we'd use
// a markdown reporter, but there don't seem to be any...)
coverageReporters: ['clover', 'json', 'lcov', 'text', ['text', { file: 'coverage.txt' }]],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at using the html reporter instead, but it only seems to give a high-level directory overview rather than per-file (unless you want to deeply traverse into HTML files). So yeah, text seems most convenient here.

Copy link
Collaborator

@nolanlawson nolanlawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM modulo my one comment about  .

Another small thing I noticed is that we are (oddly) reporting coverage for a test file:

Screenshot 2024-01-10 at 3 47 46 PM

We should probably fix this, but that can go in a follow-up PR.

@wjhsf wjhsf merged commit b1ac0aa into master Jan 11, 2024
9 checks passed
@wjhsf wjhsf deleted the wjh/upload-jest-coverage branch January 11, 2024 18:47
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.

2 participants