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(test): roundtrip_sapling_tree_root and roundtrip_orchard_tree_root #7628

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

oxarbitrage
Copy link
Contributor

Motivation

The round trip tree root proptests almost always test the same end height:

running 1 test
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:467] &val.end.0 = 16777215
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:467] &val.end.0 = 16777215
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:467] &val.end.0 = 16777215
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:467] &val.end.0 = 16777215
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:467] &val.end.0 = 16777215
...
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:467] &val.end.0 = 3342791
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:467] &val.end.0 = 16777215
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:467] &val.end.0 = 16777215

Close #7443

Solution

Add suggested fix, when applied the tests will cover much more values:

running 1 test
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 13521308
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 13932221
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 15280320
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 5739578
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 4218397
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 10812062
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 5608137
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 12093244
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 1981967
[zebra-state/src/service/finalized_state/disk_format/tests/prop.rs:466] &val.end.0 = 3483660
...

Review

Anyone can review. This is medium priority as it is part of the spend before sync

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@oxarbitrage oxarbitrage added P-Medium ⚡ C-testing Category: These are tests labels Sep 26, 2023
@oxarbitrage oxarbitrage requested a review from a team as a code owner September 26, 2023 13:53
@oxarbitrage oxarbitrage self-assigned this Sep 26, 2023
@oxarbitrage oxarbitrage requested review from teor2345 and removed request for a team September 26, 2023 13:53
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Sep 26, 2023
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

Thank you!

@mergify mergify bot merged commit 90df59a into main Sep 26, 2023
@mergify mergify bot deleted the issue7443 branch September 26, 2023 23:45
@upbqdn upbqdn mentioned this pull request Oct 13, 2023
38 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-testing Category: These are tests C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix proptest coverage of note commitment subtree serialization
2 participants