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(ci): set CODECOV_TOKEN after action upgrade #442

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

coopernetes
Copy link
Contributor

@coopernetes coopernetes commented Feb 7, 2024

Fixes #441

  • codecov-action@v4 requires the token to be set
  • flags are not necessary at this time. its used to differentiate test types and/or sub-components in a monorepo. For now, we just want to see all test coverage across the whole project.

- codecov-action@v4 requires the token to be set
- flags are not necessary at this time. its used to differentiate test types and/or
  sub-components in a monorepo. For now, we just want to see all test coverage across
  the whole project.
Copy link

netlify bot commented Feb 7, 2024

Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name Link
🔨 Latest commit 18c12f0
🔍 Latest deploy log https://app.netlify.com/sites/endearing-brigadeiros-63f9d0/deploys/65c3c39502db100008f1137a

@coopernetes coopernetes marked this pull request as ready for review February 7, 2024 16:22
@coopernetes
Copy link
Contributor Author

@JamieSlome before we merge, let me try modifying our "skip on empty coverage step", validating that Codecov gets the full report then I'll revert the change.

- this commit will be reverted; DO NOT MERGE
Copy link

codecov bot commented Feb 7, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (fc86cf4) 73.58% compared to head (18c12f0) 56.85%.
Report is 9 commits behind head on main.

Files Patch % Lines
src/proxy/routes/index.js 87.50% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #442       +/-   ##
===========================================
- Coverage   73.58%   56.85%   -16.74%     
===========================================
  Files           1       39       +38     
  Lines          53     1036      +983     
===========================================
+ Hits           39      589      +550     
- Misses         14      447      +433     
Flag Coverage Δ
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@coopernetes
Copy link
Contributor Author

nyc.config.js wasn't working as intended. I'm going to permanently remove this logic since it just produces the following error:

https://github.com/finos/git-proxy/pull/442/files#diff-9f5e2e8ee9ae83e176cbe50f37f106679cb18f4b5c3c19163fdea8854454e477L16-L38

Generating coverage report for changed files...
fatal: ambiguous argument 'origin/': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
Error:  Error: Command failed: git rev-parse origin/
fatal: ambiguous argument 'origin/': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

@JamieSlome codecov handles the difference in changed files already and I think some of the numbers coming back from the tool are misreported because we were monkeying around with the files to include into nyc's report generation. If we run with the full report and use codecov instead to report the differences in changed files, I do not believe we need our own logic to do this.

The token change is sound and confirmed in the workflow that reports are being posted again. I'll need to merge this into main to generate a baseline, then rebase on my other branch on #415 to get accurate coverage differences.

- remove the parsing of changed files for PRs via nyc.config.js
- add codecov.yml and set reports to informational only (dont block PRs)
- add codecov badge to README
Copy link
Member

@JamieSlome JamieSlome left a comment

Choose a reason for hiding this comment

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

LGTM! 🍰

@coopernetes coopernetes merged commit dbcd124 into finos:main Feb 8, 2024
12 checks passed
@coopernetes coopernetes mentioned this pull request Feb 8, 2024
@coopernetes coopernetes deleted the hotfix/codecov-token branch July 29, 2024 17:44
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.

bug: codecov cannot post after upgrading to v4
2 participants