You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Torsten Förtsch <to...@gmx.net> on 2010/08/23 14:47:01 UTC

2 A-T patches and a bug without patch

Hi,

the first patch fixes a behavior that is a bug in my humble opinion. 
"t/TEST -start-httpd" can be used to start the httpd. Later on one can use
"t/TEST -run-tests t/*.t" to perform the actual test run. Unfortunately, this 
2nd call overwrites the configuration. Normally, this should not be a problem 
because the same configuration should be generated. But it the server already 
runs and has port-based VHosts enabled those ports are already in use. So, the 
new config contains other ports. In my case '-start-httpd' has created a 
config that used ports 8529, 8530 and 8531. Then the '-run-tests' recreates 
the config but now it contains the ports 8529, 8532 and 8533. In my tests I 
then use for example

  Apache::TestRequest::module 'ssl';
  $sslhostport=Apache::TestRequest::hostport;

That reads the new/wrong config and the tests fail.

Not to the patch, it mainly shifts the time when the config is saved a few 
steps further in the program. split_test_args and die_on_invalid_args finish 
processing of the command line and die if something is wrong. This is a good 
thing to do before anything is saved (IMO).

default_run_opts then makes sure that at least one of -start-httpd,
-stop-httpd or -run-tests is specified. If nothing is given all 3 are set.

After that the new code checks if either -start-httpd or -configure is given. 
Only in these cases the config is written. Strictly speaking, the -configure 
check is not necessary because if only -configure is set -start-httpd is set 
by default_run_opts.

Please comment!

Does my reasoning make sense? Did I miss something?

Index: lib/Apache/TestRun.pm
===================================================================
--- lib/Apache/TestRun.pm       (revision 987941)
+++ lib/Apache/TestRun.pm       (working copy)
@@ -719,9 +719,15 @@
         $self->opt_clean(1);
     }
 
+    $self->split_test_args;
+
+    $self->die_on_invalid_args;
+
+    $self->default_run_opts;
+
     # if configure() fails for some reason before it has flushed the
     # config to a file, save it so -clean will be able to clean
-    unless ($self->{opts}->{clean}) {
+    if ($self->{opts}->{'start-httpd'} || $self->{opts}->{'configure'}) {
         eval { $self->configure };
         if ($@) {
             error "configure() has failed:\n$@";
@@ -737,12 +743,6 @@
         exit_perl 1;
     }
 
-    $self->default_run_opts;
-
-    $self->split_test_args;
-
-    $self->die_on_invalid_args;
-
     $self->start unless $self->{opts}->{'no-httpd'};
 
     $self->run_tests;


The next patch fixes a bug that occurs if LWP is installed. The function below 
check whether the server is up. It issues a request for /index.html. Now, if 
LWP is not installed and the server is not running Apache::TestRequest::GET 
returns undef. That is correctly handled by the old code. But if LWP is 
installed even if the server is down a response object is returned. The HTTP 
code of this response is set to 500 by LWP and a response header indicates the 
connection failure. That part is not caught by the old code and hence

  t/TEST -ping=block

returns immediately even if the server is down.

Index: lib/Apache/TestServer.pm
===================================================================
--- lib/Apache/TestServer.pm    (revision 987941)
+++ lib/Apache/TestServer.pm    (working copy)
@@ -672,8 +672,10 @@
     my $server_up = sub {
         local $SIG{__WARN__} = sub {}; #avoid "cannot connect ..." warnings
         # avoid fatal errors when LWP is not available
-        my $r = eval { Apache::TestRequest::GET('/index.html') };
-        return !$@ && defined $r ? $r->code : 0;
+        return eval {
+           my $r=Apache::TestRequest::GET('/index.html');
+           $r->code!=500 or $r->header('client-warning')!~/internal/i;
+       } || 0;
     };
 
     if ($server_up->()) {

Again, please comment!

The only thing that bothers me here is: what if the server if it is running 
returns an INTERNAL_SERVER_ERROR for /index.html and emits a Client-Warning 
HTTP header that contains the word "internal"? It is not very likely that that 
happens but you never know...


Now on a bug that I haven't found a satisfying patch yet. The A-T can work 
with Test.pm and with Test::More. Apache/Test.pm contains a global variable 
@testmore to tell these cases apart. test_pm_refresh() then checks @testmore 
to reset eihter Test.pm or Test::More.

Now, what happens if the same apache worker (meaning the same perl 
interpreter) first performs a server based Test::More test like 
t/more/02testmore.t and then a server based Test.pm test like 
t/more/03testpm.t? The Test::More test loads Test::More but it also 
initializes the global @testmore variable. When the Test.pm test hits after 
that test_pm_refresh thinks it works for Test::More. So, Test.pm is not reset 
and the next test is at least out of order.

This happens in the current A-T test suite.

A possible fix could be to check defined(&Test::plan) and 
defined(&Test::More::plan) and reinitialize both if both are loaded. The same 
should be done for plan() ($real_plan).

Thoughts?

Torsten Förtsch

-- 
Need professional modperl support? Hire me! (http://foertsch.name)

Like fantasy? http://kabatinte.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org