More flexible --test option
Purpose
This PR implements flexible command line overriding of which test cases to run from the command line.
Context
-
This is an extension to the improvements in #333.
-
Replaces #130. The point of that PR is to allow users to exclude test cases from the command line. This PR includes the same functionality using a different command line syntax.
-
Leverages zonemaster/zonemaster-engine#1312 to properly handle the Basic test module (c.f. https://github.com/zonemaster/zonemaster-cli/pull/130#issuecomment-572948201).
-
Handles the interaction of
--testand--profilein accordance with https://github.com/zonemaster/zonemaster-cli/pull/130#issuecomment-573012219. -
For what it's worth, the idea for this design came to me in a discussion about https://github.com/zonemaster/zonemaster-engine/pull/1294.
Changes
-
The set of values accepted by
--testis extended. -
Which test cases to include in the run is now controlled by setting the
test_casesprofile property and running Zonemaster::Engine->test_zone(). -
When
--test=basicor--test=basic02is specified, and basic02 considers the domain too broken to test, a CANNOT_CONTINUE message is now emitted, whereas it wasn't emitted before this PR.
Reviewing
- Look especially for lacking test coverage.
Testing
- Unit tests should pass.
Before this PR CLI would call either
test_zone,test_moduleortest_methodin Zonemaster::Engine to run the test. After this PR CLI always callstest_zoneand uses thetest_casesprofile property to control which tests are run. This means that the Basic tests are always included even when a single test case is specified.Since the semantics of specifying a single test case or single test module is changed so that the Basic tests are included, an argument could be made that this is a breaking change, but I believe an argument could also be made that it's still just an addition.
That is a bad change. When working with test zones you want to be able to run just the specific test case and nothing else.
Secondly, with that change, if Basic fails completely no other test case is run, and that is not what we want when testing and creating test zones.
I am against a change that makes it impossible to run just the selected test case.
All I'm hearing is "no" and "bad". Is there anything about this proposal that you do like?
Also, I'm trying to understand your objection here. AFAICT you're saying that when a user requests that a single test case is run on a domain that doesn't fulfill the requirements posed by Basic, the requested test case still needs to run. Is that correct? And if so, could you explain why this is important, because it's not obvious to me.
All I'm hearing is "no" and "bad". Is there anything about this proposal that you do like?
It would be good to be able to exclude test cases without creating a new profile. It would be good to be able to select two or more test cases and they are run without overlap.
Sometimes it is probably good that Basic is run before a selected test.
Also, I'm trying to understand your objection here. AFAICT you're saying that when a user requests that a single test case is run on a domain that doesn't fulfill the requirements posed by Basic, the requested test case still needs to run. Is that correct? And if so, could you explain why this is important, because it's not obvious to me.
I want to see what happens in that test case for the test zone even when there are errors. And I want minimal output without the extra output from Basic when testing test zones. Sometimes I have to run with --level debug which gives many lines, which will be anymore if Basic is run.
Give the possibility to run a test without adding Basic.
If Basic is not in the profile, why should Basic still be run? The magic could rather be to put the Basic test cases in the profile first, if there are any.
I want to see what happens in that test case for the test zone even when there are errors. And I want minimal output without the extra output from Basic when testing test zones. Sometimes I have to run with --level debug which gives many lines, which will be anymore if Basic is run.
If Basic is not in the profile, why should Basic still be run? The magic could rather be to put the Basic test cases in the profile first, if there are any.
Ok. Yeah, I see how this could be very valuable in development contexts. And I also don't particularly like that Basic is forced.
I have an idea of how we could simplify things in Engine in a way that supports this. I think I'll make a draft PR there too.
I won't make any more updates to the code here before Engine is in a better shape to support it. But if anyone has more feedback on the direction of this PR, please let me know. Maybe it'll affect the updates in Engine.