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

refactor: trace decoder tests #596

Merged
merged 9 commits into from
Sep 5, 2024
Merged

Conversation

0xaatif
Copy link
Contributor

@0xaatif 0xaatif commented Sep 5, 2024

It was very difficult to see which test was failing and why in #583.

I've rewritten the integration tests to be more succint, simpler, test more things, and have a better ux:

  • Use assert2 to make multiple assertions at once, with more informative output on assertion failure.
  • Use libtest_mimic to create test cases for each input file. Test selection, failure attribution all work as expected.
  • Move the test cases for trace_decoder::wire into trace_decoder/src - they are completely unrelated to the tests in trace_decoder/tests.
  • Made the directories clearer and less nested.
  • Documented test cases and test binaries where possible.
  • Cover batch sizes of 3 AND 1

Here's the new UX for a failing test with multiple conditions tripping:
image

And for passing tests:

$ cargo test --quiet --release --package trace_decoder --test simulate-execution
loading test vectors...done

running 176 tests
test b19840104_main/0  ... ok
test b19840104_main/1  ... ok
test b20240052_main/9  ... ok
...
test b20240058_main/27 ... ok

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

Note the simulate-execution test needs --quiet because of this noise (#597)

@0xaatif 0xaatif requested a review from atanmarko September 5, 2024 17:10
@0xaatif 0xaatif self-assigned this Sep 5, 2024
@github-actions github-actions bot added the crate: trace_decoder Anything related to the trace_decoder crate. label Sep 5, 2024
@0xaatif 0xaatif force-pushed the 0xaatif/trace-decoder-tests branch from 02e258a to f760883 Compare September 5, 2024 17:12
trace_decoder/Cargo.toml Show resolved Hide resolved
trace_decoder/tests/simulate-execution.rs Outdated Show resolved Hide resolved
trace_decoder/tests/simulate-execution.rs Outdated Show resolved Hide resolved
trace_decoder/tests/consistent-with-header.rs Outdated Show resolved Hide resolved
@0xaatif 0xaatif added this to the Testing and Validation milestone Sep 5, 2024
@0xaatif 0xaatif merged commit 6f8199b into develop Sep 5, 2024
15 checks passed
@0xaatif 0xaatif deleted the 0xaatif/trace-decoder-tests branch September 5, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: trace_decoder Anything related to the trace_decoder crate.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants