Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Explore: warn on missing repeats, add explicit creation when missing
Followup from [this discussion](#150 (comment)) The idea here is: 1. Explicit repeat creation in tests will improve test clarity 2. Introduce a clear way to make similar changes in JavaRosa as they come up 3. Detect missing repeats (with a still naive approach[^1], albeit now recursive) and **log with a stack trace** so explicit calls can be introduced (conditionally, with parameterization like many other cases where we make adjustments to the JavaRosa direct port) 4. Add a new proposed `Scenario` method which… - Makes clear where explicit repeat creation calls are added, in a way that can be traced directly in test source, whenever convenient - Assumes the call occurs in such a sub-suite parameterizing whether to explicitly add repeats as detected; adds repeats as explicitly specified in the true condition, suppresses logging in the false condition This approach already detected one test which would have passed if adding repeats had been explicit. The test is updated here to demonstrate that. Notice that the test’s **PORTING NOTES** have also been removed. This is because the notes were wrong! This is an excellent example of how misleading it is that tests fail for lack of this implicit behavior! The actual test logic is not substantially noisier or more complex as a result. This feels like a clear win to me. [^1]: Keeping this naive seems fine for the limited scope of usage. The reference expressions which reach this point are limited to `Scenario.answer` calls with an explicit reference. If we’re using references of arbitrary complexity in those calls, I think we’ve got much bigger problems than this functionality being so narrowly scoped.
- Loading branch information
I think some things you said earlier finally just came together in my brain. Like yes, it would be nice to have well-named and tested engine-level implementations for some of these concepts for Scenario's purposes but it's not actually functionality we actually want to expose. Yes, that makes sense. JavaRosa can do the implicit repeat creation at a relatively high level in Scenario because the public APIs do expose a ton of internal details. The fact that everything is public has been its own source of big headaches so I agree with avoiding that.
With all that said, I think we should just stop the implicit creation and make all tests be explicit about it. This feels like a fine way to bridge to that eventually when we make the changes in JR.