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

Run only tests for PRs and run coverage report on main #1762

Merged
merged 1 commit into from
Aug 11, 2023
Merged

Conversation

noisekit
Copy link
Contributor

Even though we run coverage report on each commit we never actually submit those reports to Codecov

@dbeal-eth suggested not to run cov report on PRs. That makes sense as main takes like 15min to do cov report (because running on hardhat net is slow!)

Because of that now we only run tests on PRs (must be pretty quick) and coverage report (which also includes all the tests) will only run on main branch

This should speed up day to day dev a lot buy running tests much much faster in CI

@noisekit noisekit self-assigned this Aug 11, 2023
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #1762 (673684c) into main (fa1857c) will decrease coverage by 15.89%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             main    #1762       +/-   ##
===========================================
- Coverage   72.77%   56.88%   -15.89%     
===========================================
  Files          57       96       +39     
  Lines         720     1299      +579     
  Branches      236      451      +215     
===========================================
+ Hits          524      739      +215     
- Misses        167      521      +354     
- Partials       29       39       +10     
Flag Coverage Δ
core-contracts 93.26% <ø> (ø)
core-modules 90.47% <ø> (ø)
core-utils 68.57% <ø> (ø)
hardhat-storage 21.07% <ø> (?)
oracle-manager 88.52% <ø> (?)
sample-project 56.66% <ø> (?)

see 39 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@dbeal-eth dbeal-eth left a comment

Choose a reason for hiding this comment

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

Definitely goodness in here

"@synthetixio/hardhat-storage",
"@synthetixio/sample-project",

# "@synthetixio/legacy-market", # tests fail
Copy link
Contributor

Choose a reason for hiding this comment

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

Tests shouldn't fail, what can I say. The tests were refreshed just a couple weeks ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's enable them when tests are not failing

Getting bunch of errors like these:
image

Also legacy market was not tested in CI before

@noisekit noisekit changed the base branch from fail-build-if-tests-fail to main August 11, 2023 04:31
@noisekit noisekit force-pushed the coverage branch 3 times, most recently from f897fe1 to bf6c805 Compare August 11, 2023 05:01
@noisekit noisekit merged commit 5b06e5d into main Aug 11, 2023
18 checks passed
@noisekit noisekit deleted the coverage branch August 11, 2023 05:43
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