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: allow test coverage to run #4555

Merged
merged 2 commits into from
Feb 21, 2024
Merged

fix: allow test coverage to run #4555

merged 2 commits into from
Feb 21, 2024

Conversation

dandanlen
Copy link
Collaborator

@dandanlen dandanlen commented Feb 21, 2024

Pull Request

This commit prevents a compiler issue with the code coverage test runner. The issue was a compiler overflow when evaluating some trait bounds related to the Grandpa pallet.

I wasn't able to solve the underlying problem - presumably it's something related to the internals of llvm-cov, and presumably it will be fixed in due course.

For now I have avoided the issue by using llvm-cov's cfg(coverage) compiler flag. I used this to conditionally compiler a more 'friendly' version of the offending trait, which is irrelevant to our code coverage. The 'normal' version of this remains active for all other tests and CI checks.

Turns out there's a much simpler solution. Thanks @AlastairHolmes

This commit prevents a compiler issue with the code coverage test runner. The issue was a compiler overflow when evaluating some trait bounds related to the Grandpa pallet.

I wasn't able to solve the underlying problem - presumably it's something related to the internals of llvm-cov, and presumably it will be fixed in due course.

For now I have avoided the issue by using llvm-cov's cfg(coverage) compiler flag. I used this to conditionally compiler a more 'friendly' version of the offending trait, which is irrelevant to our code coverage. The 'normal' version of this remains active for all other tests and CI checks.
Copy link

codecov bot commented Feb 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cb12b67) 73% compared to head (50c9c4a) 72%.
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #4555     +/-   ##
=======================================
- Coverage     73%     72%     -0%     
=======================================
  Files        401     401             
  Lines      68209   65968   -2241     
  Branches   68209   65968   -2241     
=======================================
- Hits       49617   47796   -1821     
+ Misses     15964   15858    -106     
+ Partials    2628    2314    -314     

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

@dandanlen dandanlen enabled auto-merge (squash) February 21, 2024 14:08
@dandanlen dandanlen merged commit 4fa34da into main Feb 21, 2024
43 checks passed
@dandanlen dandanlen deleted the fix/code-cov-compiler-bug branch February 21, 2024 14:27
syan095 added a commit that referenced this pull request Feb 26, 2024
…ero-liquidity

* origin/main: (28 commits)
  feat(custom-rpc): add broker info [WEB-925] (#4560)
  chore: upgrade solana version (#4567)
  fix: continuous adapter (PRO-684) (#4503)
  fix: Wait for ThresholdSignature success before switching to NewKeysActivated (#4534)
  chore: remove un-needed serde derives from EncodedAddress (#4565)
  fix: more lenient max deposit fee in bouncer test (#4564)
  chore: build persa bins instead of fetch (#4554)
  feat: deploy L2 contracts upon localnet startup and send L2 TXs 📑 (#4558)
  feat: debug logs on runtime upgrade test (#4556)
  feature/PRO-1038/pool-fee-rpc (#4459)
  chore: update bootnodes in chainspec ✨ (#4456)
  fix: allow test coverage to run (#4555)
  refactor/pro-1160/remove-side-side-map (#4489)
  fix: activate missing migrations (#4550)
  chore: remove cf-build (#4551)
  feat: extensible multi-key rotator (#4546)
  fix: ensure channel open fee can be paid in benchmarks (#4544)
  feat: bump substrate deps to polkadot-sdk 1.6 (#4504)
  fix: upgrade-test should restart the chainflip-nodes on an incompatible upgrade (#4490)
  test: add small v3 migration test (#4543)
  ...

# Conflicts:
#	state-chain/pallets/cf-pools/src/tests.rs
syan095 added a commit that referenced this pull request Feb 26, 2024
…-price

* origin/main:
  feat(custom-rpc): add broker info [WEB-925] (#4560)
  chore: upgrade solana version (#4567)
  fix: continuous adapter (PRO-684) (#4503)
  fix: Wait for ThresholdSignature success before switching to NewKeysActivated (#4534)
  chore: remove un-needed serde derives from EncodedAddress (#4565)
  fix: more lenient max deposit fee in bouncer test (#4564)
  chore: build persa bins instead of fetch (#4554)
  feat: deploy L2 contracts upon localnet startup and send L2 TXs 📑 (#4558)
  feat: debug logs on runtime upgrade test (#4556)
  feature/PRO-1038/pool-fee-rpc (#4459)
  chore: update bootnodes in chainspec ✨ (#4456)
  fix: allow test coverage to run (#4555)
  refactor/pro-1160/remove-side-side-map (#4489)
  fix: activate missing migrations (#4550)
  chore: remove cf-build (#4551)

# Conflicts:
#	state-chain/pallets/cf-pools/src/lib.rs
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