Merge pull request zonemaster#1294 from tgreenx/update-run_one

Refactoring of function Zonemaster::Engine::Test::run_one()
marc-vanderwal · Dec 6, 2023 · 6d0c913 · 6d0c913
2 parents 7f2c5cd + 62c4cd1
commit 6d0c913
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 32 deletions.
diff --git 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,9 +260,9 @@ 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
+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.
 

diff --git 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 => {} );
     }
@@ -301,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.
 
@@ -312,15 +307,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 +324,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 +342,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
@@ -457,7 +457,6 @@
             "SKIP_IPV6_DISABLED": "DEBUG",
             "START_TIME": "DEBUG",
             "TEST_TARGET": "DEBUG",
-            "TEST_ARGS": "DEBUG",
             "UNKNOWN_METHOD" : "CRITICAL",
             "UNKNOWN_MODULE" : "CRITICAL"
         },
@@ -525,6 +524,8 @@
         "address01",
         "address02",
         "address03",
+        "basic01",
+        "basic02",
         "basic03",
         "connectivity01",
         "connectivity02",

diff --git a/share/profile.yaml b/share/profile.yaml
@@ -25,6 +25,8 @@ test_cases:
   - address01
   - address02
   - address03
+  - basic01
+  - basic02
   - basic03
   - connectivity01
   - connectivity02
@@ -498,7 +500,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
@@ -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
@@ -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
@@ -8,6 +8,11 @@
         "nameserver06",
         "nameserver07",
         "nameserver08",
-        "nameserver09"
+        "nameserver09",
+        "nameserver10",
+        "nameserver11",
+        "nameserver12",
+        "nameserver13",
+        "nameserver15"
     ]
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -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 );
     };
@@ Expand Down @@