Generate debug output for all regression tests#1307
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1307 +/- ##
==========================================
- Coverage 89.65% 89.51% -0.15%
==========================================
Files 57 58 +1
Lines 8441 8537 +96
Branches 8441 8537 +96
==========================================
+ Hits 7568 7642 +74
- Misses 565 580 +15
- Partials 308 315 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the Rust integration/regression test suite to always generate model debug outputs during regression runs, making CI artifacts more useful for diagnosing failures, while adding a separate CLI test to ensure example run still succeeds without debug enabled.
Changes:
- Always invoke
muse2 example runwith--debug-modelin regression tests, but only comparedebug_*.csvoutputs when explicitly enabled for a given case. - Extend regression-test macros to pass an explicit
debug_modelboolean into the runner. - Add a CLI integration test that runs
example run simplewith no additional flags to keep non-debug execution covered.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/regression.rs | Always includes --debug-model in regression runs; uses a boolean to decide whether to compare debug CSV outputs. |
| tests/common.rs | Updates regression test macros to pass a debug_model boolean to the runner (and adjusts debug-files macro accordingly). |
| tests/cli.rs | Adds a non-debug example run smoke test to retain coverage for running without debug enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Run a regression test for the given example with optional extra arguments to `muse2 run`. | ||
| fn run_regression_test(example: &str, extra_args: &[&str]) { | ||
| /// | ||
| /// The `debug-model` flag is always used so the debug files are available to examine. The debug | ||
| /// files are only tested when the `debug_model` flag is true. | ||
| fn run_regression_test(example: &str, extra_args: &[&str], debug_model: bool) { |
| ($example:ident, $extra_args:expr, $debug_model:literal) => { | ||
| #[test] | ||
| fn $example() { | ||
| run_regression_test(stringify!($example), $extra_args, $debug_model); | ||
| } | ||
| }; |
There was a problem hiding this comment.
I don't understand this. I looked up what to include as types for macros (because they are not regular types) and it seemed like a boolean required a literal. But I'll change it to expr and see if it still works
| assert_muse2_runs(&[ | ||
| "example", | ||
| "run", | ||
| "simple", | ||
| "--output-dir", | ||
| &tempdir().unwrap().path().to_path_buf().to_string_lossy(), |
There was a problem hiding this comment.
I'd do this for clarity, regardless of the point about relying on brittle lifetime rules ☝️
There was a problem hiding this comment.
Again, I don't understand this comment. But I can make this change (although I don't know what the .join("out") is doing there). My usual instinct is to avoid making temporary variables that are only being used once, which is why it's all on one line.
alexdewar
left a comment
alexdewar
left a comment
There was a problem hiding this comment.
Copilot has suggested some tweaks that I think are worth doing, but otherwise LGTM 😄
Description
This PR changes the regression tests so that they always run with
--debug-modelturned on. Therefore the output includes the debug files, which should help with inspecting failing tests.This change means there are not regression tests running without the debug flag. To ensure the code still runs without it, I have included an additional test
check_example_run_commandthat just confirms the simple model runs without failure.Fixes #1198
Type of change
Key checklist
$ cargo test$ cargo docpresent in the previous release
Further checks