From 4a90b4d488bb106087dffb11c84e2a0566f1f1f0 Mon Sep 17 00:00:00 2001
From: "[Thomas Green]" <thomas.green@afnic.fr>
Date: Wed, 20 Sep 2023 10:59:02 +0200
Subject: [PATCH 1/4] Refactoring of function
 Zonemaster::Engine::Test::run_one()

Harmonize input arguments (with respect to 'run_all_for()' and 'run_module()')
Remove message tag 'TEST_ARGS'
Add conditional check for Test Case - trying to run a Test Case disabled by the profile will now return an error.
---
 lib/Zonemaster/Engine.pm            |  6 +++---
 lib/Zonemaster/Engine/Test.pm       | 33 +++++++++--------------------
 share/profile.json                  |  1 -
 share/profile.yaml                  |  1 -
 t/Test-zone.t                       |  1 +
 t/profiles/Test-all-levels.json     |  1 -
 t/profiles/Test-nameserver-all.json |  7 +++++-
 7 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/lib/Zonemaster/Engine.pm b/lib/Zonemaster/Engine.pm
index fdeb6f998..5f4af2e5e 100644
--- a/lib/Zonemaster/Engine.pm
+++ b/lib/Zonemaster/Engine.pm
@@ -71,9 +71,9 @@ sub test_module {
 }
 
 sub test_method {
-    my ( $class, $module, $method, @arguments ) = @_;
+    my ( $class, $module, $method, $zname ) = @_;
 
-    return Zonemaster::Engine::Test->run_one( $module, $method, @arguments );
+    return Zonemaster::Engine::Test->run_one( $module, $method, $class->zone( $zname ) );
 }
 
 sub all_tags {
@@ -260,7 +260,7 @@ Runs all available tests and returns a list of L<Zonemaster::Engine::Logger::Ent
 
 Runs all available tests for the zone with the given name in the specified module.
 
-=item test_method($module, $method, @arguments)
+=item test_method($module, $method, $name)
 
 Run one particular test method in one particular module. The requested module must be in the list of active loaded modules (that is, not the Basic
 module and not a module disabled by the current profile), and the method must be listed in the metadata the module exports. If those requirements
diff --git a/lib/Zonemaster/Engine/Test.pm b/lib/Zonemaster/Engine/Test.pm
index 01c6ee1d9..f144cbeb3 100644
--- a/lib/Zonemaster/Engine/Test.pm
+++ b/lib/Zonemaster/Engine/Test.pm
@@ -185,25 +185,18 @@ sub run_all_for {
     Zonemaster::Engine->start_time_now();
     push @results, info( START_TIME => { time_t => time(), string => strftime( "%F %T %z", ( localtime() ) ) } );
     push @results, info( TEST_TARGET => { zone => $zone->name->string, module => 'all' } );
-
-    info(
-        MODULE_VERSION => {
-            module  => 'Zonemaster::Engine::Test::Basic',
-            version => Zonemaster::Engine::Test::Basic->version
-        }
-    );
     _log_versions();
 
     if ( not( Zonemaster::Engine::Profile->effective->get( q{net.ipv4} ) or Zonemaster::Engine::Profile->effective->get( q{net.ipv6} ) ) ) {
         return info( NO_NETWORK => {} );
     }
 
+    info( MODULE_VERSION => { module  => 'Zonemaster::Engine::Test::Basic', version => Zonemaster::Engine::Test::Basic->version } );
     push @results, Zonemaster::Engine::Test::Basic->all( $zone );
     info( MODULE_END => { module => 'Zonemaster::Engine::Test::Basic' } );
 
     if ( Zonemaster::Engine::Test::Basic->can_continue( $zone, @results ) and Zonemaster::Engine->can_continue() ) {
         foreach my $mod ( __PACKAGE__->modules ) {
-
             my $module = "Zonemaster::Engine::Test::$mod";
             info( MODULE_VERSION => { module => $module, version => $module->version } );
             my @res = eval { $module->all( $zone ) };
@@ -219,8 +212,8 @@ sub run_all_for {
             info( MODULE_END => { module => $module } );
 
             push @results, @res;
-        } ## end foreach my $mod ( __PACKAGE__...)
-    } ## end if ( Zonemaster::Engine::Test::Basic...)
+        }
+    }
     else {
         push @results, info( CANNOT_CONTINUE => { domain => $zone->name->string } );
     }
@@ -259,6 +252,7 @@ sub run_module {
     push @res, info( START_TIME => { time_t => time(), string => strftime( "%F %T %z", ( localtime() ) ) } );
     push @res, info( TEST_TARGET => { zone => $zone->name->string, module => $requested } );
     _log_versions();
+
     if ( not( Zonemaster::Engine::Profile->effective->get( q{net.ipv4} ) or Zonemaster::Engine::Profile->effective->get( q{net.ipv6} ) ) ) {
         return info( NO_NETWORK => {} );
     }
@@ -312,15 +306,14 @@ Returns a list of L<Zonemaster::Engine::Logger::Entry> objects.
 =cut
 
 sub run_one {
-    my ( $class, $requested, $test, @arguments ) = @_;
+    my ( $class, $requested, $test, $zone ) = @_;
     my @res;
     my ( $module ) = grep { lc( $requested ) eq lc( $_ ) } $class->modules;
     $module = 'Basic' if ( not $module and lc( $requested ) eq 'basic' );
 
     Zonemaster::Engine->start_time_now();
     push @res, info( START_TIME => { time_t => time(), string => strftime( "%F %T %z", ( localtime() ) ) } );
-    push @res,
-      info( TEST_ARGS => { module => $requested, testcase => $test, args => join( ';', map { "$_" } @arguments ) } );
+    push @res, info( TEST_TARGET => { zone => $zone->name->string, module => $requested, testcase => $test } );
     _log_versions();
     
     if ( not( Zonemaster::Engine::Profile->effective->get( q{net.ipv4} ) or Zonemaster::Engine::Profile->effective->get( q{net.ipv6} ) ) ) {
@@ -330,9 +323,9 @@ sub run_one {
     if ( Zonemaster::Engine->can_continue() ) {
         if ( $module ) {
             my $m = "Zonemaster::Engine::Test::$module";
-            if ( $m->metadata->{$test} ) {
+            if ( $m->metadata->{$test} and Zonemaster::Engine::Util::should_run_test( $test ) ) {
                 info( MODULE_VERSION => { module => $m, version => $m->version } );
-                push @res, eval { $m->$test( @arguments ) };
+                push @res, eval { $m->$test( $zone ) };
                 if ( $@ ) {
                     my $err = $@;
                     if ( blessed $err and $err->isa( 'Zonemaster::Engine::Exception' ) ) {
@@ -348,19 +341,13 @@ sub run_one {
             else {
                 info( UNKNOWN_METHOD => { module => $m, testcase => $test } );
             }
-        } ## end if ( $module )
+        }
         else {
             info( UNKNOWN_MODULE => { module => $requested, testcase => $test, module_list => join( ':', sort $class->modules ) } );
         }
     }
     else {
-        my $zname = q{};
-        foreach my $arg ( @arguments ) {
-            if ( ref($arg) eq q{Zonemaster::Engine::Zone} ) {
-                $zname = $arg->name;
-            }
-        }
-        info( CANNOT_CONTINUE => { domain => $zname } );
+        info( CANNOT_CONTINUE => { domain => $zone->name->string } );
     }
 
     return;
diff --git a/share/profile.json b/share/profile.json
index 687af16e6..115fea971 100644
--- a/share/profile.json
+++ b/share/profile.json
@@ -457,7 +457,6 @@
             "SKIP_IPV6_DISABLED": "DEBUG",
             "START_TIME": "DEBUG",
             "TEST_TARGET": "DEBUG",
-            "TEST_ARGS": "DEBUG",
             "UNKNOWN_METHOD" : "CRITICAL",
             "UNKNOWN_MODULE" : "CRITICAL"
         },
diff --git a/share/profile.yaml b/share/profile.yaml
index 5e48e281b..cff97ea31 100644
--- a/share/profile.yaml
+++ b/share/profile.yaml
@@ -498,7 +498,6 @@ test_levels:
     SKIP_IPV4_DISABLED: DEBUG
     SKIP_IPV6_DISABLED: DEBUG
     START_TIME: DEBUG
-    TEST_ARGS: DEBUG
     TEST_TARGET: DEBUG
     UNKNOWN_METHOD: CRITICAL
     UNKNOWN_MODULE: CRITICAL
diff --git a/t/Test-zone.t b/t/Test-zone.t
index 8efdaed7d..ccbc3caac 100644
--- a/t/Test-zone.t
+++ b/t/Test-zone.t
@@ -106,6 +106,7 @@ subtest 'user defined SOA values' => sub {
 
     # reset the profile
     Zonemaster::Engine::Profile->effective->merge( Zonemaster::Engine::Profile->default );
+    Zonemaster::Engine::Profile->effective->merge( $profile_test );
 };
 
 
diff --git a/t/profiles/Test-all-levels.json b/t/profiles/Test-all-levels.json
index 20970dc31..a5c7dbc62 100644
--- a/t/profiles/Test-all-levels.json
+++ b/t/profiles/Test-all-levels.json
@@ -223,7 +223,6 @@
             "SKIP_IPV6_DISABLED": "DEBUG",
             "START_TIME": "DEBUG",
             "TEST_TARGET": "DEBUG",
-            "TEST_ARGS": "DEBUG",
             "UNKNOWN_METHOD" : "CRITICAL",
             "UNKNOWN_MODULE" : "CRITICAL",
             "IS_BLACKLISTED" : "DEBUG",
diff --git a/t/profiles/Test-nameserver-all.json b/t/profiles/Test-nameserver-all.json
index a9ba22dc4..7f39faebf 100644
--- a/t/profiles/Test-nameserver-all.json
+++ b/t/profiles/Test-nameserver-all.json
@@ -8,6 +8,11 @@
         "nameserver06",
         "nameserver07",
         "nameserver08",
-        "nameserver09"
+        "nameserver09",
+        "nameserver10",
+        "nameserver11",
+        "nameserver12",
+        "nameserver13",
+        "nameserver15"
     ]
 }

From b14ca9dbf0cb0fa65de006789777dda6caa85d8c Mon Sep 17 00:00:00 2001
From: tgreenx <96772376+tgreenx@users.noreply.github.com>
Date: Tue, 28 Nov 2023 18:17:06 +0100
Subject: [PATCH 2/4] Apply suggestion from review

Update documentation of Zonemaster::Engine::test_method()

Co-authored-by: Marc van der Wal <103426270+marc-vanderwal@users.noreply.github.com>
---
 lib/Zonemaster/Engine.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Zonemaster/Engine.pm b/lib/Zonemaster/Engine.pm
index 5f4af2e5e..a22ad3987 100644
--- a/lib/Zonemaster/Engine.pm
+++ b/lib/Zonemaster/Engine.pm
@@ -262,7 +262,7 @@ Runs all available tests for the zone with the given name in the specified modul
 
 =item test_method($module, $method, $name)
 
-Run one particular test method in one particular module. The requested module must be in the list of active loaded modules (that is, not the Basic
+Run one particular test method in one particular module for one particular zone. The requested module must be in the list of active loaded modules (that is, not the Basic
 module and not a module disabled by the current profile), and the method must be listed in the metadata the module exports. If those requirements
 are fulfilled, the method will be called with the provided arguments.
 

From f7bac455de925bd7db95832a6f9a09e3c8f02d80 Mon Sep 17 00:00:00 2001
From: "[Thomas Green]" <thomas.green@afnic.fr>
Date: Wed, 29 Nov 2023 10:18:36 +0100
Subject: [PATCH 3/4] Update documentation of
 Zonemaster::Engine::Test::run_one()

---
 lib/Zonemaster/Engine/Test.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/Zonemaster/Engine/Test.pm b/lib/Zonemaster/Engine/Test.pm
index f144cbeb3..967994a96 100644
--- a/lib/Zonemaster/Engine/Test.pm
+++ b/lib/Zonemaster/Engine/Test.pm
@@ -295,7 +295,8 @@ Runs the given Test Case of the given Test module for the given zone.
 
 The Test module must be in the list of actively loaded modules (that is,
 a module defined in the B<share/modules.txt> file), and the Test Case
-must be listed in the L<metadata|/metadata()> of the Test module exports.
+must be listed both in the L<metadata|/metadata()> of the Test module
+exports and in the L<profile|Zonemaster::Engine::Profile/test_cases>.
 
 Takes a string (module name), a string (test case name) and an array of L<Zonemaster::Engine::Zone> objects.
 

From 62c4cd14e09947f4d1b85ccf25627b75ab560c88 Mon Sep 17 00:00:00 2001
From: "[Thomas Green]" <thomas.green@afnic.fr>
Date: Wed, 29 Nov 2023 11:33:05 +0100
Subject: [PATCH 4/4] Add basic00, basic01 and basic02 to the profile

---
 share/profile.json | 2 ++
 share/profile.yaml | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/share/profile.json b/share/profile.json
index 115fea971..6e30fbc6a 100644
--- a/share/profile.json
+++ b/share/profile.json
@@ -524,6 +524,8 @@
         "address01",
         "address02",
         "address03",
+        "basic01",
+        "basic02",
         "basic03",
         "connectivity01",
         "connectivity02",
diff --git a/share/profile.yaml b/share/profile.yaml
index cff97ea31..4f1c931ce 100644
--- a/share/profile.yaml
+++ b/share/profile.yaml
@@ -25,6 +25,8 @@ test_cases:
   - address01
   - address02
   - address03
+  - basic01
+  - basic02
   - basic03
   - connectivity01
   - connectivity02