Problem Description:

In PR #421, tests are incorrectly passing even though there are mismatches between Conjure solutions and Conjure-Oxide solutions for some models. This issue arises because when running tests with ACCEPT=true (i.e., ACCEPT=true cargo test ), the expected solution files are automatically rewritten with the generated solutions, even if they don’t match Conjure solutions. As a result, the next time the tests run, they always pass since the test framework compares the newly generated solutions with the updated expected files (which are identical).

Solution:

To resolve this, the key is to avoid overwriting the expected files when the ACCEPT=true flag is set, unless the generated solutions match the Conjure solutions.

Refactoring Approach:

1. When ACCEPT=true:

   • Generate all files (parse, rewrite, solve) as usual.
   • Only assert that the generated solutions match the Conjure solutions. Parsed, rewrite, and trace are assumed to be correct.
   • If the solutions match, rewrite the expected files (i.e., save_model_json and save_minion_solutions_json).
   • If the solutions don’t match, fail the test without rewriting anything.

2. When ACCEPT=false:

   • Generate all files (parse, rewrite, solve) as usual.
   • Assert the generated files against the expected solutions.

Implementation Details:

The integration_test_inner function will be refactored to:

• Generate all files first for each test stage (parsing, rewriting, solving, rule tracing), regardless of the ACCEPT flag.
• When ACCEPT=false, assert the generated files against the expected solutions.
• When ACCEPT=true, assert that the generated solutions match the Conjure solutions. If they match, rewrite the expected files.

@ozgurakgun Looks good?

• cargo test ACCEPT=true does this work? I thougth ACCEPT had to go first.
• "Only assert that the generated solutions match the Conjure solutions." we assert this for the solutions, parsed and rewritten are assumed to be correct if the solutions are.
• "rewrite expected files" - instead of calling those existing files, I sugget we rename all *generated* files to *expected*. simpler logic, easier to extend in the future.
• in the accept=false case, "Do not rewrite any files, unless the assertions pass." we never rewrite any files

I stopped reading at "implementation details".

reads a bit like gpt (mostly correct, well structured, but slightly incorrect in a few important ways), which is not a problem per se, but make sure you agree with what it says. I would just remove the "Implementation Details:" and "Key Change:" sections as they are repetitious.

As discussed in #561, we should also allow for command line arguments to enable / disable config values.

Long term, I think it would be good to move to a custom test-harness, instead of continuing to hack around the default one. As well as allowing easier arg and config parsing, this could also simplify our recurring concurrency problems, and test discovery.

• https://stackoverflow.com/a/65794217.
• https://www.infinyon.com/blog/2021/04/rust-custom-test-harness/