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

Report on test coverage #675

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Report on test coverage #675

wants to merge 9 commits into from

Conversation

72636c
Copy link
Member

@72636c 72636c commented Nov 16, 2021

This is just the first step of dumping a coverage table out to a Buildkite annotation and GitHub check run. We can later build towards:

  • Providing Buildkite annotations of failing tests
  • Providing test coverage as a GitHub PR comment
  • Comparing test coverage against the default branch per Annotate test coverage #652

@changeset-bot
Copy link

changeset-bot bot commented Nov 16, 2021

⚠️ No Changeset found

Latest commit: 39441a9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Buildkite.md.terminal(coverage),
].join('\n\n');

await Buildkite.annotate(buildkiteOutput, {
Copy link
Member Author

Choose a reason for hiding this comment

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

@samchungy how do you feel about having this reporter support both Buildkite and GitHub 😬

If we want coverage integrations across both, it may be wasteful to define two reporters and to run renderCoverageText twice.

Copy link
Contributor

@samchungy samchungy Nov 16, 2021

Choose a reason for hiding this comment

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

I don't love it but renderCoverageText is kinda costly I guess. Even the loop processing this I guess.

Do reporters run in series? I wonder if we could store it in memory? 🤔

If we do this -> might want to rename the reporter? maybe reporters/annotate/index.ts?

@72636c 72636c added the dino:snooze Snooze in Review Dino label Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dino:snooze Snooze in Review Dino
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants