Retain the possibility to start a Test Case disabled in the profile #361

tgreenx · 2023-11-28T17:01:19Z

Purpose

This PR retains the possibility of starting a Test Case disabled in the profile by letting Zonemaster-CLI directly modify the profile, and notifying the user with a message.

This behavior was previously possible due to an inconsistency in Zonemaster-Engine which is being removed by zonemaster/zonemaster-engine#1294.

Context

zonemaster/zonemaster-engine#1294 (and comment zonemaster/zonemaster-engine#1294 (comment))

How to test this PR

Requires Refactoring of function Zonemaster::Engine::Test::run_one() zonemaster-engine#1294

$ git log -1 --oneline
62c4cd14 (HEAD -> update-run_one, origin/update-run_one) Add basic00, basic01 and basic02 to the profile

You can either (i) remove a test case from the default profile or (ii) provide a new custom profile:

i)

$ git diff
diff --git a/share/profile.json b/share/profile.json
index 6e30fbc6..ac667d82 100644
--- a/share/profile.json
+++ b/share/profile.json
@@ -523,7 +523,6 @@
     "test_cases": [
         "address01",
         "address02",
-        "address03",
         "basic01",
         "basic02",
         "basic03",

ii)

$ cat share/custom.json
{
        "test_cases": [
                "address01"
        ]
}

Try running the disabled Test case, in both normal and raw outputs.

i)

$ zonemaster-cli --show-testcase --no-ipv6 zonemaster.net --test address03 --level=info
Notice: Engine does not have test case 'address03' enabled in the profile. Forcing...

Seconds Level    Testcase       Message
======= ======== ============== =======
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   3.38 INFO     Address03      Every reverse DNS entry matches name server name.

$ zonemaster-cli --show-testcase --no-ipv6 zonemaster.net --test Address/address03 --level=info --raw
Notice: Engine does not have test case 'address03' enabled in the profile. Forcing...
   0.00 INFO     UNSPECIFIED    GLOBAL_VERSION   version=v4.7.3
   8.69 INFO     ADDRESS03      NAMESERVER_IP_PTR_MATCH

ii)

$ zonemaster-cli --show-testcase --no-ipv6 zonemaster.net --test Address/address03 --level=info --profile share/custom.json
Loading profile from share/custom.json.
Notice: Engine does not have test case 'address03' enabled in the profile. Forcing...

Seconds Level    Testcase       Message
======= ======== ============== =======
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   8.32 INFO     Address03      Every reverse DNS entry matches name server name.

$ zonemaster-cli --show-testcase --no-ipv6 zonemaster.net --test Address/address03 --level=info --profile share/custom.json --raw
Loading profile from share/custom.json.
Notice: Engine does not have test case 'address03' enabled in the profile. Forcing...
   0.00 INFO     Unspecified    GLOBAL_VERSION  version=v4.7.3
   8.28 INFO     Address03      NAMESERVER_IP_PTR_MATCH

Check that tests enabled in the profile are run correctly.

$ zonemaster-cli --show-testcase --no-ipv6 zonemaster.net --test Address/address01 --test Address/address03 --level=info
Notice: Engine does not have test case 'address03' enabled in the profile. Forcing...

Seconds Level    Testcase       Message
======= ======== ============== =======
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   5.86 INFO     Address01      All Nameserver addresses are in the routable public addressing space.
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   2.51 INFO     Address03      Every reverse DNS entry matches name server name.

$ zonemaster-cli --show-testcase --no-ipv6 zonemaster.net --test Address/address01 --test Address/address03 --level=info --profile share/custom.json
Loading profile from share/custom.json.
Notice: Engine does not have test case 'address03' enabled in the profile. Forcing...

Seconds Level    Testcase       Message
======= ======== ============== =======
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   5.88 INFO     Address01      All Nameserver addresses are in the routable public addressing space.
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   2.50 INFO     Address03      Every reverse DNS entry matches name server name.

Try a combination of different --test options.

$ zonemaster-cli --test DnssEc/dNsseC03 --test bAsIc --test adDress03 zonemaster.net --no-ipv6 --level=info --show-testcase --profile share/custom.json
Loading profile from share/custom.json.
Notice: Engine does not have test case 'dnssec03' enabled in the profile. Forcing...
Notice: Engine does not have test case 'address03' enabled in the profile. Forcing...

Seconds Level    Testcase       Message
======= ======== ============== =======
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   1.35 INFO     DNSSEC03       The zone does not use NSEC3. Testing for NSEC3 has been skipped. Fetched from name servers "ns2.nic.fr/192.93.0.4;nsa.dnsnode.net/194.58.192.46;nsp.dnsnode.net/194.58.198.32;nsu.dnsnode.net/185.42.137.98".
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   7.70 INFO     Basic01        The parent zone is "net" as returned from name servers "a.gtld-servers.net/192.5.6.30;b.gtld-servers.net/192.33.14.30;c.gtld-servers.net/192.26.92.30;d.gtld-servers.net/192.31.80.30;e.gtld-servers.net/192.12.94.30;f.gtld-servers.net/192.35.51.30;g.gtld-servers.net/192.42.93.30;h.gtld-servers.net/192.54.112.30;i.gtld-servers.net/192.43.172.30;j.gtld-servers.net/192.48.79.30;k.gtld-servers.net/192.52.178.30;l.gtld-servers.net/192.41.162.30;m.gtld-servers.net/192.55.83.30".
   7.70 INFO     Basic01        The zone "zonemaster.net" is found.
   8.01 INFO     Basic02        Authoritative answer on SOA query for "zonemaster.net" is returned by name servers "ns2.nic.fr/192.93.0.4;nsa.dnsnode.net/194.58.192.46;nsp.dnsnode.net/194.58.198.32;nsu.dnsnode.net/185.42.137.98".
   8.01 INFO     Unspecified    Functional nameserver found. "A" query for www.zonemaster.net test skipped.
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   2.57 INFO     Address03      Every reverse DNS entry matches name server name.

zonemaster/zonemaster-engine#1294 removes the possibility for Zonemaster-Engine to start a Test Case which is explicitly disabled in its profile. This commit retains the possibility by letting Zonemaster-CLI directly modify the profile, and notifying the user with a notice message.

tgreenx · 2023-11-29T10:41:59Z

@mattias-p After the merging of #333, conflicts have arised and are now fixed. I also provided a small refactoring which considers the changes in zonemaster/zonemaster-engine#1294.

lib/Zonemaster/CLI.pm

matsduf

With this PR on top of zonemaster/zonemaster-engine#1294 I get no output of messages when using --raw:

# zonemaster-cli --profile ~/.zonemaster/profile-zone09-only.json --show-testcase --level info iis.se --test Zone/Zone01 --raw
Loading profile from /home/matsd/.zonemaster/profile-zone09-only.json.
Notice: Engine does not have test case 'zone01' enabled in the profile. Forcing...

It does not help to remove --profile and --test.

When going back to current develop (Engine and CLI) it works as it should.

Without --raw I get the following:

# zonemaster-cli --profile ~/.zonemaster/profile-zone09-only.json --show-testcase --level info iis.se --test Zone/Zone01
Loading profile from /home/matsd/.zonemaster/profile-zone09-only.json.

Seconds Level    Testcase       Message
======= ======== ============== =======
Notice: Engine does not have test case 'zone01' enabled in the profile. Forcing...
   0.00 INFO     UNSPECIFIED    Using version v4.7.3 of the Zonemaster engine.
   0.66 INFO     ZONE01         SOA MNAME name server "ns.nic.se" is not listed as NS record for the zone.

I do not think there is any need of the "notice", but if it should be there, it should be above the headline or below all messages, not between the headline and the messages.

matsduf · 2023-12-01T22:03:15Z

lib/Zonemaster/CLI.pm

+                     ( ($method, $module) = $t =~ m#^ ( ( [a-z]+ ) [0-9]{2} ) $#ix ) )
+                {
+                    if ( not grep( /^$method$/, @{ Zonemaster::Engine::Profile->effective->get( 'test_cases' ) } ) ) {
+                        say $fh_diag __x( "Notice: Engine does not have test case '$method' enabled in the profile. Forcing...");


I do not see the purpose of this warning message, and especially where it is put.

Updated to display it earlier in the code

matsduf

I still dislike where the "Notice: Engine does not have test case 'zone01' enabled in the profile. Forcing..." message comes in the response. I do not see the purpose of it.

--raw is also meant to be machine-readable output. Now something can come in. Does that make it a V-Major change?

The new line is also added to the JSON output, but before JSON comes. At least jq seems to ignore it.

tgreenx · 2023-12-04T12:38:14Z

I still dislike where the "Notice: Engine does not have test case 'zone01' enabled in the profile. Forcing..." message comes in the response. I do not see the purpose of it.

The new line is also added to the JSON output, but before JSON comes. At least jq seems to ignore it.

I have just provided an update to display the message earlier in the code. See commit 08c2a77.

--raw is also meant to be machine-readable output. Now something can come in. Does that make it a V-Major change?

I'm not sure. This is no different that other already present messages such as when loading a custom profile.

Move input checks earlier in the code Add input check for multiple slash characters Refactoring

matsduf · 2023-12-04T12:52:35Z

I have just provided an update to display the message earlier in the code. See commit 08c2a77.

Nice.

matsduf · 2023-12-04T12:58:47Z

--raw is also meant to be machine-readable output. Now something can come in. Does that make it a V-Major change?

I'm not sure. This is no different that other already present messages such as when loading a custom profile.

I know, but this is a new line that you have to ignore. But now I see that they go to STDERR whereas the result lines go to STDOUT so it is possible to filter them out.

mattias-p

I had another look at this and I believe I found a couple of misbehaviors. And I have proposals for how to fix them.

lib/Zonemaster/CLI.pm

Add input validation, based on the content of 'Zonemaster::Engine->all_methods' Construct the 'test_cases' profile property from the combination of all --test switches Refactoring

tgreenx

@mattias-p All your comments have been addressed. See commit 5c68764. Now:

$ cat ../zonemaster-engine/share/custom.json
{
    "test_cases": [
        "address01"
    ]
}

$ git log -1 --oneline
5c68764 (HEAD -> retains-single_test) Update after review - more refactoring of --test

$ zonemaster-cli --test DnssEc/dNsseC03 --test bAsIc --test adDress03 zonemaster.net --no-ipv6 --level=info --show-testcase --profile ../zonemaster-engine/share/custom.json
Loading profile from ../zonemaster-engine/share/custom.json.
Notice: Engine does not have test case 'dnssec03' enabled in the profile. Forcing...
Notice: Engine does not have test case 'address03' enabled in the profile. Forcing...

Seconds Level    Testcase       Message
======= ======== ============== =======
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   1.33 INFO     DNSSEC03       The zone does not use NSEC3. Testing for NSEC3 has been skipped. Fetched from name servers "ns2.nic.fr/192.93.0.4;nsa.dnsnode.net/194.58.192.46;nsp.dnsnode.net/194.58.198.32;nsu.dnsnode.net/185.42.137.98".
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   2.66 INFO     Basic01        The parent zone is "net" as returned from name servers "a.gtld-servers.net/192.5.6.30;b.gtld-servers.net/192.33.14.30;c.gtld-servers.net/192.26.92.30;d.gtld-servers.net/192.31.80.30;e.gtld-servers.net/192.12.94.30;f.gtld-servers.net/192.35.51.30;g.gtld-servers.net/192.42.93.30;h.gtld-servers.net/192.54.112.30;i.gtld-servers.net/192.43.172.30;j.gtld-servers.net/192.48.79.30;k.gtld-servers.net/192.52.178.30;l.gtld-servers.net/192.41.162.30;m.gtld-servers.net/192.55.83.30".
   2.66 INFO     Basic01        The zone "zonemaster.net" is found.
   3.02 INFO     Basic02        Authoritative answer on SOA query for "zonemaster.net" is returned by name servers "ns2.nic.fr/192.93.0.4;nsa.dnsnode.net/194.58.192.46;nsp.dnsnode.net/194.58.198.32;nsu.dnsnode.net/185.42.137.98".
   3.02 INFO     Unspecified    Functional nameserver found. "A" query for www.zonemaster.net test skipped.
   0.00 INFO     Unspecified    Using version v4.7.3 of the Zonemaster engine.
   2.56 INFO     Address03      Every reverse DNS entry matches name server name.

lib/Zonemaster/CLI.pm

tgreenx · 2023-12-05T15:10:42Z

@mattias-p I realize now that there is still one remaining case that I overlooked: that is when running a module (e.g. Address) with a profile containing only one test case (e.g. Address01) of that module, all Address test cases (i.e. Address{01-03}) will be run. I'll try and provide an update.

mattias-p

This looks good to me. I just have one last quibble.

mattias-p · 2023-12-05T15:42:48Z

lib/Zonemaster/CLI.pm

+            else {
+                # Get the test module with the right case
+                ( $module ) = grep { lc( $module ) eq lc( $_ ) } @existing_test_modules;
+                push @actual_test_cases, @{ $existing_tests{$module} };


In current develop if you specify --profile file --test dnssec and the given profile contains a strict subset of all dnssec test cases, only that subset will be run. With this implementation all dnssec test cases will be run. I believe you agree the behavior in current develop is more reasonable. So after expanding the module name to a list of test case names but before pushing onto @actual_test_cases we should grep the list to only keep those test cases that are currently included in the test_cases profile property. I'm sorry I wasn't clear on this last time around.

Oh, you already spotted it. I didn't see your comment before.

Done.

$ git log -1 --oneline d640b9c (HEAD -> retains-single_test, origin/retains-single_test) Respect profile configuration when there are already test cases of a requested module in the profile $ zonemaster-cli --show-testcase --no-ipv6 zonemaster.net --test addRess --test bAsic --test baSic02 --test basIc --test adDress01 --test adDress/addrEss02 --level=info --profile ../zonemaster-engine/share/custom.json Loading profile from ../zonemaster-engine/share/custom.json. Notice: Engine does not have test case 'address02' enabled in the profile. Forcing... Seconds Level Testcase Message ======= ======== ============== ======= 0.00 INFO Unspecified Using version v4.7.3 of the Zonemaster engine. 0.85 INFO Address01 All Nameserver addresses are in the routable public addressing space. 3.37 INFO Address02 Reverse DNS entry exists for each Nameserver IP address. 0.00 INFO Unspecified Using version v4.7.3 of the Zonemaster engine. 12.73 INFO Basic01 The parent zone is "net" as returned from name servers "a.gtld-servers.net/192.5.6.30;b.gtld-servers.net/192.33.14.30;c.gtld-servers.net/192.26.92.30;d.gtld-servers.net/192.31.80.30;e.gtld-servers.net/192.12.94.30;f.gtld-servers.net/192.35.51.30;g.gtld-servers.net/192.42.93.30;h.gtld-servers.net/192.54.112.30;i.gtld-servers.net/192.43.172.30;j.gtld-servers.net/192.48.79.30;k.gtld-servers.net/192.52.178.30;l.gtld-servers.net/192.41.162.30;m.gtld-servers.net/192.55.83.30". 12.73 INFO Basic01 The zone "zonemaster.net" is found. 13.04 INFO Basic02 Authoritative answer on SOA query for "zonemaster.net" is returned by name servers "ns2.nic.fr/192.93.0.4;nsa.dnsnode.net/194.58.192.46;nsp.dnsnode.net/194.58.198.32;nsu.dnsnode.net/185.42.137.98". 13.04 INFO Unspecified Functional nameserver found. "A" query for www.zonemaster.net test skipped. 0.00 INFO Unspecified Using version v4.7.3 of the Zonemaster engine. 0.07 INFO Basic02 Authoritative answer on SOA query for "zonemaster.net" is returned by name servers "ns2.nic.fr/192.93.0.4;nsa.dnsnode.net/194.58.192.46;nsp.dnsnode.net/194.58.198.32;nsu.dnsnode.net/185.42.137.98". 0.00 INFO Unspecified Using version v4.7.3 of the Zonemaster engine. 0.07 INFO Basic01 The parent zone is "net" as returned from name servers "a.gtld-servers.net/192.5.6.30;b.gtld-servers.net/192.33.14.30;c.gtld-servers.net/192.26.92.30;d.gtld-servers.net/192.31.80.30;e.gtld-servers.net/192.12.94.30;f.gtld-servers.net/192.35.51.30;g.gtld-servers.net/192.42.93.30;h.gtld-servers.net/192.54.112.30;i.gtld-servers.net/192.43.172.30;j.gtld-servers.net/192.48.79.30;k.gtld-servers.net/192.52.178.30;l.gtld-servers.net/192.41.162.30;m.gtld-servers.net/192.55.83.30". 0.07 INFO Basic01 The zone "zonemaster.net" is found. 0.15 INFO Basic02 Authoritative answer on SOA query for "zonemaster.net" is returned by name servers "ns2.nic.fr/192.93.0.4;nsa.dnsnode.net/194.58.192.46;nsp.dnsnode.net/194.58.198.32;nsu.dnsnode.net/185.42.137.98". 0.15 INFO Unspecified Functional nameserver found. "A" query for www.zonemaster.net test skipped. 0.00 INFO Unspecified Using version v4.7.3 of the Zonemaster engine. 0.09 INFO Address01 All Nameserver addresses are in the routable public addressing space. 0.00 INFO Unspecified Using version v4.7.3 of the Zonemaster engine. 0.10 INFO Address02 Reverse DNS entry exists for each Nameserver IP address.

…requested module in the profile

tgreenx · 2023-12-05T16:58:05Z

@mattias-p @matsduf please re-review.

mattias-p · 2023-12-05T19:18:18Z

lib/Zonemaster/CLI.pm

+                if ( not grep( /^$module$/, keys %actual_test_modules ) ) {
+                    # Get the test module with the right case
+                    ( $module ) = grep { lc( $module ) eq lc( $_ ) } @existing_test_modules;
+                    # No need to bother to check for duplicates here
+                    push @actual_test_cases, @{ $existing_tests{$module} };
+                }


This is a new feature, right? It's nice though.

It is not fully consistent, but maybe good enough for now? The most common use case for --test is probably when all test cases are listed in the profile, and for that is seems to work as it should.

The inconsistency is when the profile only lists some test cases.

My profile has only Zone09 listed.

$ cat ~/.zonemaster/profile-zone09-only.json | jq .test_cases [ "zone09" ]

When no --test is given the behavior is as expected:

$ zonemaster-cli --profile ~/.zonemaster/profile-zone09-only.json --show-testcase --level info iis.se --raw Loading profile from /home/matsd/.zonemaster/profile-zone09-only.json. 0.00 INFO Unspecified GLOBAL_VERSION version=v4.7.3 2.14 INFO Basic01 B01_PARENT_FOUND domain=se; ns_ip_list=a.ns.se/192.36.144.107;a.ns.se/2a01:3f0:0:301::53;b.ns.se/192.36.133.107;b.ns.se/2001:67c:254c:301::53;c.ns.se/192.36.135.107;c.ns.se/2001:67c:2554:301::53;f.ns.se/192.71.53.53;f.ns.se/2a01:3f0:0:305::53;g.ns.se/130.239.5.114;g.ns.se/2001:6b0:e:3::1;i.ns.se/194.146.106.22;i.ns.se/2001:67c:1010:5::53;m.ns.se/194.0.11.112;m.ns.se/2001:678:e:112::53;x.ns.se/2001:67c:124c:e000::4;x.ns.se/213.108.25.4;y.ns.se/185.159.197.150;y.ns.se/2620:10a:80aa::150;z.ns.se/185.159.198.150;z.ns.se/2620:10a:80ab::150 2.14 INFO Basic01 B01_CHILD_FOUND domain=iis.se 2.59 INFO Basic02 B02_AUTH_RESPONSE_SOA domain="iis.se"; ns_list=nsa.dnsnode.net/194.58.192.46;nsa.dnsnode.net/2a01:3f1:46::53;nsp.dnsnode.net/194.58.198.32;nsp.dnsnode.net/2a01:3f1:3032::53;nsu.dnsnode.net/185.42.137.98;nsu.dnsnode.net/2a01:3f0:400::32 2.59 INFO Unspecified HAS_NAMESERVER_NO_WWW_A_TEST zname="iis.se" 2.68 INFO Zone09 Z09_MX_DATA mailtarget_list=mx1.iis.se.;mx2.iis.se.; ns_ip_list=194.58.198.32;185.42.137.98;2a01:3f1:3032::53;2a01:3f1:46::53;194.58.192.46;2a01:3f0:400::32

If --test Address is selcted, then all Address test cases are run and unexpectedly no warning that they are not listed in the profile:

$ zonemaster-cli --profile ~/.zonemaster/profile-zone09-only.json --show-testcase --level info iis.se --test Address --raw Loading profile from /home/matsd/.zonemaster/profile-zone09-only.json. 0.00 INFO Unspecified GLOBAL_VERSION version=v4.7.3 0.51 INFO Address01 NO_IP_PRIVATE_NETWORK 2.23 INFO Address02 NAMESERVERS_IP_WITH_REVERSE 2.24 INFO Address03 NAMESERVER_IP_PTR_MATCH

If --test Zone01 is selected only that test case is run with a warning that it is not listed in the profile:

$ zonemaster-cli --profile ~/.zonemaster/profile-zone09-only.json --show-testcase --level info iis.se --raw --test Zone01 Loading profile from /home/matsd/.zonemaster/profile-zone09-only.json. Notice: Engine does not have test case 'zone01' enabled in the profile. Forcing... 0.00 INFO Unspecified GLOBAL_VERSION version=v4.7.3 0.59 INFO Zone01 Z01_MNAME_NOT_IN_NS_LIST nsname=ns.nic.se

If --test Zone is selected unexpectedly only Zone09 is run:

$ zonemaster-cli --profile ~/.zonemaster/profile-zone09-only.json --show-testcase --level info iis.se --raw --test Zone Loading profile from /home/matsd/.zonemaster/profile-zone09-only.json. 0.00 INFO Unspecified GLOBAL_VERSION version=v4.7.3 0.58 INFO Zone09 Z09_MX_DATA mailtarget_list=mx2.iis.se.;mx1.iis.se.; ns_ip_list=185.42.137.98;2a01:3f1:3032::53;2a01:3f0:400::32;2a01:3f1:46::53;194.58.198.32;194.58.192.46

I am ready to approve if the behavior above is accepted.

It is not fully consistent, but maybe good enough for now? The most common use case for --test is probably when all test cases are listed in the profile, and for that is seems to work as it should.

The inconsistency is when the profile only lists some test cases.

All of the scenarios that you mention are expected behaviors. The warning was supposed to appear only when wanting to run a single test case that is not in the profile (i.e. using Zonemaster::Engine::test_method()). Running a test module (i.e. using Zonemaster::Engine::test_module()) will either only run the test cases of that module if any are specified in the profile, else all of them.

This is a new feature, right? It's nice though.

It is not, this is to keep the behavior of current develop (i.e. see your comment).

Running a test module (i.e. using Zonemaster::Engine::test_module()) will either only run the test cases of that module if any are specified in the profile, else all of them.

Well, I find it strange that all Addresses are run when none of them are included in the profile, but only one Zone test case because that single test case is included in the profile. That is inconsistent.

The behavior is not bad enough to block it, but I think it should be straighten out in next release.

I guess it is debatable. We can look at it for next release yes.

ghost · 2024-01-08T17:55:53Z

v2023.2 release testing

Tested following the "How to test" section, works as expected.

tgreenx added the V-Patch Versioning: The change gives an update of patch in version. label Nov 28, 2023

tgreenx added this to the v2023.2 milestone Nov 28, 2023

tgreenx requested review from a user, hannaeko, marc-vanderwal, matsduf and mattias-p November 28, 2023 17:01

tgreenx mentioned this pull request Nov 28, 2023

Refactoring of function Zonemaster::Engine::Test::run_one() zonemaster/zonemaster-engine#1294

Merged

mattias-p previously approved these changes Nov 29, 2023

View reviewed changes

tgreenx added 2 commits November 29, 2023 11:06

Small refactoring in --test

a66d559

tgreenx dismissed mattias-p’s stale review via a66d559 November 29, 2023 10:38

tgreenx force-pushed the retains-single_test branch from 675557c to a66d559 Compare November 29, 2023 10:38

tgreenx requested a review from mattias-p November 29, 2023 10:42

mattias-p previously approved these changes Nov 30, 2023

View reviewed changes

lib/Zonemaster/CLI.pm Outdated Show resolved Hide resolved

matsduf requested changes Dec 1, 2023

View reviewed changes

matsduf previously approved these changes Dec 4, 2023

View reviewed changes

tgreenx dismissed stale reviews from matsduf and mattias-p via 2b00c6c December 4, 2023 12:32

tgreenx force-pushed the retains-single_test branch from 2b00c6c to a762c8d Compare December 4, 2023 12:36

tgreenx requested review from matsduf and mattias-p December 4, 2023 12:38

Update after review - refactoring in --test

08c2a77

Move input checks earlier in the code Add input check for multiple slash characters Refactoring

tgreenx force-pushed the retains-single_test branch from a762c8d to 08c2a77 Compare December 4, 2023 12:50

matsduf previously approved these changes Dec 4, 2023

View reviewed changes

mattias-p requested changes Dec 5, 2023

View reviewed changes

lib/Zonemaster/CLI.pm Outdated Show resolved Hide resolved

lib/Zonemaster/CLI.pm Outdated Show resolved Hide resolved

lib/Zonemaster/CLI.pm Outdated Show resolved Hide resolved

Update after review - more refactoring of --test

5c68764

Add input validation, based on the content of 'Zonemaster::Engine->all_methods' Construct the 'test_cases' profile property from the combination of all --test switches Refactoring

tgreenx dismissed matsduf’s stale review via 5c68764 December 5, 2023 13:49

tgreenx commented Dec 5, 2023

View reviewed changes

lib/Zonemaster/CLI.pm Outdated Show resolved Hide resolved

lib/Zonemaster/CLI.pm Outdated Show resolved Hide resolved

lib/Zonemaster/CLI.pm Outdated Show resolved Hide resolved

tgreenx requested review from matsduf and mattias-p December 5, 2023 13:50

Fix newline for --test in human readable output

583157e

mattias-p reviewed Dec 5, 2023

View reviewed changes

Respect profile configuration when there are already test cases of a …

d640b9c

…requested module in the profile

tgreenx requested a review from mattias-p December 5, 2023 16:59

mattias-p approved these changes Dec 5, 2023

View reviewed changes

matsduf approved these changes Dec 6, 2023

View reviewed changes

tgreenx merged commit 353e708 into zonemaster:develop Dec 6, 2023

tgreenx deleted the retains-single_test branch December 6, 2023 10:03

matsduf mentioned this pull request Dec 11, 2023

PRs for v2023.2 zonemaster/zonemaster#1232

Closed

ghost added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Jan 8, 2024

Retain the possibility to start a Test Case disabled in the profile #361

Retain the possibility to start a Test Case disabled in the profile #361

Uh oh!

Conversation

tgreenx commented Nov 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Context

How to test this PR

Uh oh!

tgreenx commented Nov 29, 2023

Uh oh!

Uh oh!

matsduf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matsduf left a comment

Choose a reason for hiding this comment

Uh oh!

tgreenx commented Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matsduf commented Dec 4, 2023

Uh oh!

matsduf commented Dec 4, 2023

Uh oh!

mattias-p left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tgreenx left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tgreenx commented Dec 5, 2023

Uh oh!

mattias-p left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgreenx Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgreenx commented Dec 5, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matsduf Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgreenx Dec 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Jan 8, 2024

v2023.2 release testing

Uh oh!

Reviewers

Assignees

Labels

tgreenx commented Nov 28, 2023 •

edited

Loading

tgreenx commented Dec 4, 2023 •

edited

Loading

tgreenx left a comment •

edited

Loading

tgreenx Dec 5, 2023 •

edited

Loading

matsduf Dec 5, 2023 •

edited

Loading

tgreenx Dec 6, 2023 •

edited

Loading