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

CSUB-882: Fix failing CI jobs & add more asserts #1432

Merged
merged 4 commits into from
Nov 29, 2023

Conversation

atodorov
Copy link
Contributor

Description of proposed changes


Practical tips for PR review & merge:

  • All GitHub Actions report PASS
  • Newly added code/functions have unit tests
    • Coverage tools report all newly added lines as covered
    • The positive scenario is exercised
    • Negative scenarios are exercised, e.g. assert on all possible errors
    • Assert on events triggered if applicable
    • Assert on changes made to storage if applicable
  • Modified behavior/functions - try to make sure above test items are covered
  • Integration tests are added if applicable/needed

not sure why this was removed in 86c777d
however it is being imported in unit tests and without it
tests are failing to compile
@atodorov atodorov changed the title CSUB-882: Fix failing CI jobs CSUB-882: Fix failing CI jobs & add more asserts Nov 28, 2023
Copy link

For full LLVM coverage report click here!

@codecov-commenter
Copy link

codecov-commenter commented Nov 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (86c777d) 19.68% compared to head (6c0b320) 13.68%.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1432      +/-   ##
==========================================
- Coverage   19.68%   13.68%   -6.00%     
==========================================
  Files          31       31              
  Lines        1001     1001              
  Branches      128      128              
==========================================
- Hits          197      137      -60     
- Misses        794      864      +70     
+ Partials       10        0      -10     

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

…rrors

RawOrigin::Root yields errors:
Error: Input("Benchmark pallet_creditcoin::burn_all failed: Bad origin")
Error: Input("Benchmark pallet_creditcoin::burn failed: Bad origin")

because the extrinsic signature is checked like so::

	let who = ensure_signed(origin)?;

RawOrigin::Root should be used when the extrinsic signature check is::

	ensure_root(origin)?;

See for example set_gate_faucet()!
NOTE: always use a multiple of the existential deposit because
make_free_balance_be() checks that, see
https://paritytech.github.io/polkadot-sdk/master/src/pallet_balances/impl_currency.rs.html#476

The reason we were getting the error:
Error: Input("Benchmark pallet_creditcoin::burn failed: BurnInsufficientFunds")

is that MIN is 500 and the original chosen value of 100 is lower which
causes the check above to fail.

WARNING: the values `100` and `500` are in CREDO, not in CTC. That is
10^-18 CTC!
@atodorov atodorov force-pushed the testing/CSUB-882-fix-failing-ci branch from 3aa449e to 6c0b320 Compare November 28, 2023 18:33
@atodorov atodorov marked this pull request as ready for review November 28, 2023 19:16
@atodorov atodorov merged commit a8314df into dev Nov 29, 2023
35 of 37 checks passed
@atodorov atodorov deleted the testing/CSUB-882-fix-failing-ci branch November 29, 2023 12:17
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