zonemaster
Better performance when running specified test. Function to exclude tests
Sometimes it is desired to skip some tests, for example in an Anycast DNS setup where all servers are in the same AS.
When running multiple test with --test, the performance is not that good since a Zonemaster::Engine->zone( $domain ) object is created for each specified test. This fixes this by only creating this object once
Thanks for the pull request, and welcome! I'll come back with a review shortly.
This seems like a nice feature! Though I have few nits to pick.
First, we're only using the git master branch as a reference to the last release. We do all development on the develop branch. If you could rebase these changes onto the develop branch and change the target branch of the pull request to develop, that would be great. (You'll notice that we accidentally merged stuff into the master branch since last release and fixed it up by adding revert commits. Take care not to carry over those commits to the develop branch when rebasing. I'm sorry for the inconvenience.)
Second, the --test and --exclude_test options don't interact well. For instance you might want to use --test MODULE --exclude_test MODULE/METHOD but that would not work as expected. Making the --test and --exclude_test mutually exclusive would solve this problem. The mutual exclusiveness should be noted in the documentation for both options. (A well-designed tighter integration between the options could be added at a later time. If you want to implement the tighter integration yourself I suggest you do it in a second pull request after this one is accepted.)
Third, in my opinion the level of documentation is really good. USING.md should probably be updated with this new option. If you want to you can make a suggestion for such an update. But you don't have to. @sandoche2k, could you advice here?
The document uses the basic module as an example. That is probably a bad example since the basic module will always be run even if not selected. Module basic could probably not be excluded.
@lowinger42, I suggest that you rebase your branch on the develop branch. Your proposal should probably handle the basic module specially since the engine will always run the basic module.
All good suggestions, I'll get back with some updated code, in the correct branch. Thanks
Your proposal should probably handle the basic module specially since the engine will always run the basic module.
Good catch! Actually, the special handling of Basic is implemented in Z::E::test_zone, which is not invoked by --test either. Oops.
I think both --test and --exclude test should work by updating the test_cases property of the effective profile, and then simply call Z::E::test_zone. This way we'd cover all the intricacies of inter-method dependencies.
The --test option should not read the previous value of the test_cases property, but rather construct a new arrayref from scratch. I.e. if --profile disables a certain case and --test enables that entire module, the entire module should be included in the new property value.
The --exclude_test option should get the test_cases property value and set it again with matching test cases removed.
If --exclude_test is handled after --test, they could coexist in a well-defined way.
Thoughts on this? @vlevigneron, @matsduf, @sandoche2k, @lowinger42.
I'm sorry @lowinger42 if you already started down a different path, and I fully understand if you don't feel like cleaning up old design flaws of ours. If that's the case, just say so and we'll take care of this. Otherwise, I'll be happy to guide you through any weeds :)
@vlevigneron, can you take a look at this and the suggestions from @mattias-p?