fixing an httpd test which is confused about which modules are loaded in the test conf

[Jeff Trawick <tr...@gmail.com>]

The mod_info test (t/modules/info.t) asks Apache::Test::config() for a
list of modules.  It then expects each of those modules to be reported
by mod_info.  But some of those modules are not actually loaded
because of modules which are skipped by TestConfigParse.pm

#XXX mod_jk requires JkWorkerFile or JkWorker to be configured
#skip it for now, tomcat has its own test suite anyhow.
#XXX: mod_casp2.so requires other settings in addition to LoadModule
#XXX: mod_bwshare.so blocks fast requests that tests are doing
my @autoconfig_skip_module = qw(mod_jk.c mod_casp2.c mod_bwshare.c);

If mod_jk is in the configuration being tested, the mod_info test will
fail because it expects mod_jk to be reported by mod_info, even though
the LoadModule for mod_jk is skipped.

One possible fix is to consult should_skip_module() to see if it was
skipped from the test httpd.conf file too:

Index: t/modules/info.t
===================================================================
--- t/modules/info.t	(revision 825306)
+++ t/modules/info.t	(working copy)
@@ -28,7 +28,7 @@
 }

 foreach (sort keys %$mods) {
-    push(@expected,$_) if $mods->{$_};
+    push(@expected,$_) if $mods->{$_} && !$config->should_skip_module($_);
 }
 @actual = sort @actual;
 @expected = sort @expected;

Another possible fix is to omit skipped modules from the module list:

Index: Apache-Test/lib/Apache/TestConfigParse.pm
===================================================================
--- Apache-Test/lib/Apache/TestConfigParse.pm	(revision 822728)
+++ Apache-Test/lib/Apache/TestConfigParse.pm	(working copy)
@@ -224,15 +224,15 @@

         $name = $modname_alias{$name} if $modname_alias{$name};

-        # remember all found modules
-        $self->{modules}->{$name} = $file;
-        debug "Found: $modname => $name";
-
         if ($self->should_skip_module($name)) {
             debug "Skipping LoadModule of $name";
             next;
         }

+        # remember all found modules that aren't skipped
+        $self->{modules}->{$name} = $file;
+        debug "Found: $modname => $name";
+
         debug "LoadModule $modname $name";

         # sometimes people have broken system-wide httpd.conf files,

So is either of these a reasonable fix?  (It doesn't seem right that
info.t has to worry about modules which were found in the
configuration but not loaded, but I don't know what is expected of the
configuration object.)

Re: fixing an httpd test which is confused about which modules are loaded in the test conf

[Joe Orton <jo...@redhat.com>]

On Fri, Oct 16, 2009 at 03:32:04PM -0400, Jeff Trawick wrote:
..
> --- Apache-Test/lib/Apache/TestConfigParse.pm	(revision 822728)
> +++ Apache-Test/lib/Apache/TestConfigParse.pm	(working copy)
> @@ -224,15 +224,15 @@
> 
>          $name = $modname_alias{$name} if $modname_alias{$name};
> 
> -        # remember all found modules
> -        $self->{modules}->{$name} = $file;
> -        debug "Found: $modname => $name";
> -
...
> 
> So is either of these a reasonable fix?  (It doesn't seem right that
> info.t has to worry about modules which were found in the
> configuration but not loaded, but I don't know what is expected of the
> configuration object.)

Either seems reasonable to me, though the use of the word "all" in the 
original comment above might imply the placement of that code is 
deliberate, so I'd probably go for the info.t patch.

Regards, Joe

Re: fixing an httpd test which is confused about which modules are loaded in the test conf

[Joe Orton <jo...@redhat.com>]

On Fri, Oct 16, 2009 at 03:32:04PM -0400, Jeff Trawick wrote:
..
> --- Apache-Test/lib/Apache/TestConfigParse.pm	(revision 822728)
> +++ Apache-Test/lib/Apache/TestConfigParse.pm	(working copy)
> @@ -224,15 +224,15 @@
> 
>          $name = $modname_alias{$name} if $modname_alias{$name};
> 
> -        # remember all found modules
> -        $self->{modules}->{$name} = $file;
> -        debug "Found: $modname => $name";
> -
...
> 
> So is either of these a reasonable fix?  (It doesn't seem right that
> info.t has to worry about modules which were found in the
> configuration but not loaded, but I don't know what is expected of the
> configuration object.)

Either seems reasonable to me, though the use of the word "all" in the 
original comment above might imply the placement of that code is 
deliberate, so I'd probably go for the info.t patch.

Regards, Joe