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

Adding codecov uploading to github and buildkite with monorepo flags #1146

Closed
wants to merge 14 commits into from

Conversation

MichaelGHSeg
Copy link
Contributor

@MichaelGHSeg MichaelGHSeg commented Sep 13, 2024

  • I've included a changeset (psst. run yarn changeset. Read about changesets here).

Copy link

changeset-bot bot commented Sep 13, 2024

⚠️ No Changeset found

Latest commit: dc649b0

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

@@ -22,6 +22,9 @@ const createJestTSConfig = (
* No need to manually run yarn build all the time.
* This resolve packages for ts-jest so typescript compilation happens in-memory.
*/
collectCoverage: process.env.CI === true,
Copy link
Contributor Author

@MichaelGHSeg MichaelGHSeg Sep 13, 2024

Choose a reason for hiding this comment

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

This causes some spurrious validation warnings. I didn't see a definitive answer about whether a new jest version would solve it. We can add --coverage to the test command lines instead

Copy link
Contributor

@silesky silesky Sep 13, 2024

Choose a reason for hiding this comment

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

    ...([process.env.CI, process.env.COVERAGE].includes('true') && isRootConfig
      ? { collectCoverage: true }
      : {}),
    coverageDirectory: '<rootDir>/coverage',

I've tested this --

Observations on this config:

  1. on Buildkite/CI, when yarn run -T core test is run -- the coverage directory is in the packages/core. I am not sure that codecov can handle a bunch of individual "coverage" folders like that, or it can only handle one big honking coverage folder. Sticking with this pattern would be ideal.

  2. We also support running yarn test --coverage at the monorepo root (which is a currently not a command we run on CI) which runs the test suite on every package -- and creates an aggregated coverage folder. We only don't do this on CI because we like having the CI steps be more atomic (test core, test browser, etc) -- it also is more flexible in the future if want to power up turborepo with distributed build caching.

I'm still kinda on the fence about whether we or not we should use the CI env variable for coverage, or whether that's confusing and we use the new COVERAGE env variable. (or ditch the process.env.CI and just run with --coverage, or, all three). There's too many ways to do this ;-)

@MichaelGHSeg MichaelGHSeg deleted the MichaelGHSeg/codecov branch December 5, 2024 17:29
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