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

test(pallet-market): add failure tests and code coverage #105

Merged
merged 22 commits into from
Jul 9, 2024

Conversation

th7nder
Copy link
Contributor

@th7nder th7nder commented Jul 1, 2024

Description

Before:

test result: ok. 20 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.73s

Filename                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
pallets/market/src/lib.rs               440               172    60.91%         108                61    43.52%         637               127    80.06%           0                 0         -
primitives/proofs/src/traits.rs           3                 3     0.00%           3                 3     0.00%           3                 3     0.00%           0                 0         -
primitives/proofs/src/types.rs           14                12    14.29%          12                10    16.67%          18                10    44.44%           0                 0         -
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                   457               187    59.08%         123                74    39.84%         658               140    78.72%           0                 0         -

After:

test result: ok. 49 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 58.71s

Filename                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
pallets/market/src/lib.rs               585               157    73.16%         131                64    51.15%         859               103    88.01%           0                 0         -
primitives/proofs/src/traits.rs           3                 3     0.00%           3                 3     0.00%           3                 3     0.00%           0                 0         -
primitives/proofs/src/types.rs           14                12    14.29%          12                10    16.67%          18                10    44.44%           0                 0         -
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                   602               172    71.43%         146                77    47.26%         880               116    86.82%           0                 0         -
cargo llvm-cov -p pallet-market report --ignore-filename-regex "(mock|test)" --html --output-dir coverage/pallet-market

    Finished report saved to coverage/pallet-market/html
cargo llvm-cov -p pallet-market report --ignore-filename-regex "(mock|test)" --lcov --output-path coverage/pallet-market.lcov.info

    Finished report saved to coverage/pallet-market.lcov.info

Important points for reviewers

The test coverage tool is far from ideal, it takes into the account macros and lots of other stuff which we right now don't care about. It's 'good' enough.
To validate what's missing I used my judgement and VS Code highlighting the lines that are not tested.
While coverage increased from 80% to 86% (tests from 20 to 41) and might not look impressive, we tested a lot of critical paths that could break business logic of our parachain.

DISCLAIMER: It needs to be merged after #100.

Checklist

  • Are there important points that reviewers should know?
    • If yes, which ones?
  • Make sure that you described what this change does.
  • If there are follow-ups, have you created issues for them?
    • If yes, which ones? / List them here
  • Have you tested this solution?
  • Were there any alternative implementations considered?
  • Did you document new (or modified) APIs?

@th7nder th7nder self-assigned this Jul 1, 2024
@th7nder th7nder force-pushed the feat/11/market-pallet-tests branch 2 times, most recently from f6a9529 to b4edc83 Compare July 1, 2024 14:45
.vscode/settings.json Outdated Show resolved Hide resolved
@th7nder th7nder force-pushed the feat/11/market-pallet-tests branch 2 times, most recently from eb8bd55 to 4996220 Compare July 2, 2024 15:13
@th7nder th7nder force-pushed the feat/11/market-pallet-tests branch from 4996220 to a191127 Compare July 3, 2024 05:21
@th7nder th7nder mentioned this pull request Jul 3, 2024
8 tasks
@th7nder th7nder changed the base branch from develop to feat/11/market-pallet-cron July 4, 2024 05:04
@th7nder th7nder marked this pull request as ready for review July 4, 2024 05:04
@th7nder th7nder added the ready for review Review is needed label Jul 4, 2024
@th7nder th7nder requested a review from jmg-duarte July 4, 2024 05:05
@th7nder th7nder linked an issue Jul 4, 2024 that may be closed by this pull request
@th7nder th7nder added ready for review Review is needed and removed ready for review Review is needed labels Jul 4, 2024
@th7nder th7nder removed the ready for review Review is needed label Jul 4, 2024
@th7nder th7nder added ready for review Review is needed and removed ready for review Review is needed labels Jul 5, 2024
.vscode/extensions.json Outdated Show resolved Hide resolved
pallets/market/src/lib.rs Outdated Show resolved Hide resolved
pallets/market/src/lib.rs Show resolved Hide resolved
Justfile Show resolved Hide resolved
pallets/market/src/test.rs Show resolved Hide resolved
pallets/market/src/test.rs Outdated Show resolved Hide resolved
pallets/market/src/test.rs Outdated Show resolved Hide resolved
pallets/market/src/test.rs Outdated Show resolved Hide resolved
pallets/market/src/test.rs Outdated Show resolved Hide resolved
pallets/market/src/test.rs Outdated Show resolved Hide resolved
.vscode/extensions.json Outdated Show resolved Hide resolved
@th7nder th7nder added ready for review Review is needed and removed ready for review Review is needed labels Jul 6, 2024
@th7nder th7nder added ready for review Review is needed and removed ready for review Review is needed labels Jul 8, 2024
@th7nder th7nder requested a review from jmg-duarte July 8, 2024 11:58
jmg-duarte
jmg-duarte previously approved these changes Jul 8, 2024
@jmg-duarte jmg-duarte added ready for review Review is needed and removed ready for review Review is needed labels Jul 8, 2024
Copy link
Contributor

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

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

Great job getting all those tests. Once again, the builder was a nice touch

Copy link
Member

@cernicc cernicc left a comment

Choose a reason for hiding this comment

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

Great job! Builder 👍

@cernicc cernicc merged commit 8024fa7 into develop Jul 9, 2024
3 checks passed
@cernicc cernicc deleted the feat/11/market-pallet-tests branch July 9, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Review is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Market Pallet: failure tests
3 participants