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: avoid panic if node is not properly encoded #573

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

gabriele-0201
Copy link
Contributor

@gabriele-0201 gabriele-0201 commented Nov 22, 2024

It could happen that a leaf or branch node is not properly encoded,
causing the first key to be larger than the last one.
This could have led to a panic during the validation of the output.

Copy link
Contributor Author

gabriele-0201 commented Nov 22, 2024

Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

The PR description isn't clear to me.

Why would we not want to panic if the btree node is not properly encoded? That sounds like a bug, and something we'd want tests to catch, no?

Can you add some context on the motivation?

@gabriele-0201
Copy link
Contributor Author

Totally, that is a bug. This pr is not removing this check, but just making it explicit within the validation of the output, instead of possibly leaving expected_values.range(first..=last) to panic because last < first.

If an error like this currently occurs the output is something like

running 1 test
thread 'beatree::ops::update::tests::leaf_stage' panicked at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/alloc/src/collections/btree/search.rs:120:21:
range start is greater than range end in BTreeMap
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'beatree::ops::update::tests::leaf_stage' panicked at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/alloc/src/collections/btree/search.rs:120:21:
range start is greater than range end in BTreeMap
thread 'beatree::ops::update::tests::leaf_stage' panicked at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/alloc/src/collections/btree/search.rs:120:21:
range start is greater than range end in BTreeMap
...

while the check added in this pr becomes an explicit property required for the nodes, and the test output not passing becomes:

running 1 test
thread 'beatree::ops::update::tests::leaf_stage' panicked at /home/gabriele/.cargo/registry/src/index.crates.io-6f17d22bba15001f/quickcheck-1.0.3/src/tester.rs:165:28:
[quickcheck] TEST FAILED. Arguments: ({}, [0])
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
lead_stage failed with seed: [168, 16, 49, 83, 106, 24, 11, 24, 0, 0, 0, 0, 0, 0, 0, 0]
test beatree::ops::update::tests::leaf_stage ... FAILED

Base automatically changed from gm_quickcheck_print_on_panic to master November 25, 2024 03:36
It could happen that a leaf or branch node is not properly encoded,
causing the first key to be larger than the last one.
This can lead to a panic during the validation of the output
@gabriele-0201 gabriele-0201 force-pushed the gm_quickcheck_is_valid_fix branch from 5e24523 to 70a500c Compare November 28, 2024 03:52
@gabriele-0201 gabriele-0201 merged commit 6b054a8 into master Nov 28, 2024
8 checks passed
@gabriele-0201 gabriele-0201 deleted the gm_quickcheck_is_valid_fix branch November 28, 2024 04:12
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