Support for doctests report event#91
Conversation
|
Thanks! I'm on vacation at the moment but will take a look soon! |
There was a problem hiding this comment.
Pull request overview
This PR adds support for the "report" event that is emitted by Rust edition 2024 doctests. In edition 2024, rustdoc combines doctests into a single binary, which produces an additional { "type": "report" ... } event after execution containing timing information. The implementation correctly handles this event by ignoring it, as all necessary test results are already captured from the standard test events.
Key Changes
- Added
DoctestsReportvariant to theEventenum to deserialize the new report event type - Implemented parsing logic to safely ignore report events since they contain only informational timing data
- Added comprehensive test coverage with a new edition 2024 doctest generator and corresponding test input/output files
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | Added DoctestsReport event variant, parsing logic to ignore it, and test case for edition 2024 doctests |
| src/test_inputs/doctests_edition2024.json | Test input containing edition 2024 doctest events including the new report event |
| src/expected_outputs/doctests_edition2024.out | Expected XML output for the doctests edition 2024 test case |
| test_input_generators/doctests_edition2024/src/main.rs | Empty main function for the test input generator |
| test_input_generators/doctests_edition2024/src/lib.rs | Library with various doctest scenarios (passing, failing, should_panic, ignore) |
| test_input_generators/doctests_edition2024/Cargo.toml | Cargo manifest configured for edition 2024 to generate report events |
| test_input_generators/doctests_edition2024/Cargo.lock | Lock file for the test input generator |
| regenerate_test_inputs.sh | New script to regenerate test inputs from test generators |
| rebuild_expected_outputs.sh | Updated with new exclusion for unknown_type.json and modified error handling |
| .gitignore | Changed from /target/ to **/target/ to ignore all target directories including those in test generators |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Duration::nanoseconds(duration_ns) | ||
| } | ||
| }, | ||
| Event::DoctestsReport { .. } => panic!(), |
There was a problem hiding this comment.
The panic!() call here could be problematic if get_duration() is ever called on a DoctestsReport event. While this may never happen in the current code flow (since report events are ignored in parsing), it would be more robust to either return a default Duration (like Duration::nanoseconds(0)) or add a comment explaining why this panic is safe. Consider using unreachable!("DoctestsReport events do not have duration") to make the intent clearer.
| Event::DoctestsReport { .. } => panic!(), | |
| Event::DoctestsReport { .. } => unreachable!("DoctestsReport events do not have duration"), |
| if [[ "$f" == 'src/test_inputs/one_suite_no_tests.json' ]]; then continue; fi | ||
| cargo run < "$f" > "${f%.json}.out" || ( echo $f | grep "fail" ) | ||
| if [[ "$f" == 'src/test_inputs/unknown_type.json' ]]; then continue; fi | ||
| cargo run < "$f" > "${f%.json}.out" || true |
There was a problem hiding this comment.
The change from || ( echo $f | grep "fail" ) to || true changes the error handling behavior. The original version allowed only files with "fail" in their name to fail without causing the script to exit, while the new version suppresses all errors indiscriminately. This could mask legitimate failures when processing test inputs. Consider either reverting to the original pattern or documenting why all failures should now be ignored.
| cargo run < "$f" > "${f%.json}.out" || true | |
| cargo run < "$f" > "${f%.json}.out" || ( echo "$f" | grep "fail" ) |
|
@anatols I will merge this, then touch some things up before release |
Fix #90 by handling (ignoring) "report" event