Re: [patch] Re: Apache-Test breakage

2001-10-19 Thread Stas Bekman
Doug MacEachern wrote:
On Thu, 18 Oct 2001, Stas Bekman wrote:
 

So it works when you run ./t/TEST, but when you run 'make test', for 
some reason Makefile doesn't abort on exit from test_clean target, no 
matter if I put exit -1, 0 or 1, and proceeds with run_tests target. 
Does that have anything to do with the installed __DIE__ sighandler?

probably, since __DIE__ will stop the server.  where did you put the
exit(1) call?  i think it should work in the __DIE__ handler.

the exit call is in the $@ handling code:
+eval { $self->configure; };
+if ($@) {
+error "configure() has failed:\n$@";
+warning "forcing Apache::TestConfig object save";
+$self->{test_config}->save;
+warning "run 't/TEST -clean' to clean up before continuing";
+exit;
^^^
+}
Am I correct to say that if Makefile has:
C: A B
and A's return status is not 0, make will abort? I know that I've 
committed exit; but I've tried 1 and -1 without any effect. Any ideas 
why this happens? If you want an easy way to simulate breakage just do this:

Index: Apache-Test/lib/Apache/TestConfigPerl.pm
===
RCS file: 
/home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfigPerl.pm,v
retrieving revision 1.38
diff -u -r1.38 TestConfigPerl.pm
--- Apache-Test/lib/Apache/TestConfigPerl.pm2001/10/18 04:18:16 1.38
+++ Apache-Test/lib/Apache/TestConfigPerl.pm2001/10/19 02:14:56
@@ -347,6 +347,7 @@
 if (open $fh, $file) {
 my $content = <$fh>;
 close $fh;
+require $file;
 if ($content =~ /APACHE_TEST_CONFIGURE/m) {
 eval { require $file };
 warn $@ if $@;

_
Stas Bekman JAm_pH  --   Just Another mod_perl Hacker
http://stason.org/  mod_perl Guide   http://perl.apache.org/guide
mailto:[EMAIL PROTECTED]  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/


Re: [patch] Re: Apache-Test breakage

2001-10-18 Thread Doug MacEachern
On Thu, 18 Oct 2001, Stas Bekman wrote:
 
> So it works when you run ./t/TEST, but when you run 'make test', for 
> some reason Makefile doesn't abort on exit from test_clean target, no 
> matter if I put exit -1, 0 or 1, and proceeds with run_tests target. 
> Does that have anything to do with the installed __DIE__ sighandler?

probably, since __DIE__ will stop the server.  where did you put the
exit(1) call?  i think it should work in the __DIE__ handler.
 
> I suggest to review the code and get rid of the __DIE__ sighandler all 
> together and instead wrap the relevant pieces of code in eval block. Are 
> there any exception handling modules bundled in the core?

if you can switch to eval {} that would be fine.
 
> In any case I think this patch can be committed without relation to the 
> __DIE__ issue.

looks good, +1




Re: Apache-Test breakage

2001-10-18 Thread Doug MacEachern
On Thu, 18 Oct 2001, Stas Bekman wrote:
 
> this patch does that:

nice, +1

but, indentation is messed up:
 
>   my $argv = $self->{argv};
> +my @leftovers = ();

>   }
> +push @leftovers, $_;
 
>   $self->{tests} = [EMAIL PROTECTED];
> +$self->{argv}  = [EMAIL PROTECTED];
 
>   $self->split_test_args;
> +$self->die_on_invalid_args;



[patch] Re: Apache-Test breakage

2001-10-18 Thread Stas Bekman
Doug MacEachern wrote:

I think the break in the middle of config is important to fix, right?
yes.  the problem right now is if die() is thrown, files have been
generated but the Apache::TestConfig object is not committed to
t/conf/apache_test_config.pm.  so t/TEST -clean does not know about those
generated files.  either a DESTROY or __DIE__ handler could probably be
used to solve the problem.
__DIE__ is a mess to use and DESTROY cannot be easily used unless 
Apache::TestConfig is subclassed. I think the following is quite clean 
solution. I've tested it with breaking the configure by requiring all 
.pm files (if you remember the breakage I had while developing 
APACHE_TEST_CONFIGURE functionality)

So it works when you run ./t/TEST, but when you run 'make test', for 
some reason Makefile doesn't abort on exit from test_clean target, no 
matter if I put exit -1, 0 or 1, and proceeds with run_tests target. 
Does that have anything to do with the installed __DIE__ sighandler?

I suggest to review the code and get rid of the __DIE__ sighandler all 
together and instead wrap the relevant pieces of code in eval block. Are 
there any exception handling modules bundled in the core?

In any case I think this patch can be committed without relation to the 
__DIE__ issue.

-- Index: Apache-Test/lib/Apache/TestRun.pm
===
RCS file: 
/home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestRun.pm,v
retrieving revision 1.59
diff -u -r1.59 TestRun.pm
--- Apache-Test/lib/Apache/TestRun.pm	2001/10/17 02:56:24	1.59
+++ Apache-Test/lib/Apache/TestRun.pm	2001/10/18 05:14:11
@@ -483,7 +483,16 @@
 $self->opt_clean(1);
 }

-$self->configure;
+# if configure() fails for some reason before it has flushed the
+# config to a file, save it so -clean will be able to clean
+eval { $self->configure; };
+if ($@) {
+error "configure() has failed:\n$@";
+warning "forcing Apache::TestConfig object save";
+$self->{test_config}->save;
+warning "run 't/TEST -clean' to clean up before continuing";
+exit;
+}
 if ($self->{opts}->{configure}) {
 warning "reconfiguration done";

_
Stas Bekman JAm_pH  --   Just Another mod_perl Hacker
http://stason.org/  mod_perl Guide   http://perl.apache.org/guide
mailto:[EMAIL PROTECTED]  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/


Re: Apache-Test breakage

2001-10-18 Thread Stas Bekman
Doug MacEachern wrote:
On Thu, 18 Oct 2001, Stas Bekman wrote:
 

is there anything else we need to do with opts parsing at this stage?
now that we know all args and which are meant as test files, anything else
should throw an error.  for example what ken saw:
% t/TEST t/notexist
currently ignores t/notexist and runs all tests instead.

this patch does that:
Index: Apache-Test/lib/Apache/TestRun.pm
===
RCS file: 
/home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestRun.pm,v
retrieving revision 1.60
diff -u -r1.60 TestRun.pm
--- Apache-Test/lib/Apache/TestRun.pm	2001/10/18 02:44:36	1.60
+++ Apache-Test/lib/Apache/TestRun.pm	2001/10/18 03:43:04
@@ -88,6 +88,7 @@
 my(@tests);

 my $argv = $self->{argv};
+my @leftovers = ();
 for (@$argv) {
 my $arg = $_;
 #need the t/ for stat-ing, but dont want to include it in test 
output
@@ -117,11 +118,25 @@
 next;
 }
 }
+push @leftovers, $_;
 }

 $self->{tests} = [EMAIL PROTECTED];
+$self->{argv}  = [EMAIL PROTECTED];
 }
+sub die_on_invalid_args {
+my($self) = @_;
+
+# at this stage $self->{argv} should be empty
+my @invalid_argv = @{ $self->{argv} };
+if (@invalid_argv) {
+error "unknown opts or test names: @invalid_argv";
+exit;
+}
+
+}
+
 sub passenv {
 my $passenv = Apache::TestConfig->passenv;
 for (keys %$passenv) {
@@ -517,6 +532,8 @@
 $self->default_run_opts;
 $self->split_test_args;
+
+$self->die_on_invalid_args;
 $self->start;

_
Stas Bekman JAm_pH  --   Just Another mod_perl Hacker
http://stason.org/  mod_perl Guide   http://perl.apache.org/guide
mailto:[EMAIL PROTECTED]  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/


Re: Apache-Test breakage

2001-10-18 Thread Doug MacEachern
On Thu, 18 Oct 2001, Stas Bekman wrote:
 
> is there anything else we need to do with opts parsing at this stage?

now that we know all args and which are meant as test files, anything else
should throw an error.  for example what ken saw:

% t/TEST t/notexist

currently ignores t/notexist and runs all tests instead.

> I think the break in the middle of config is important to fix, right?

yes.  the problem right now is if die() is thrown, files have been
generated but the Apache::TestConfig object is not committed to
t/conf/apache_test_config.pm.  so t/TEST -clean does not know about those
generated files.  either a DESTROY or __DIE__ handler could probably be
used to solve the problem.




Re: Apache-Test breakage

2001-10-18 Thread Stas Bekman
Doug MacEachern wrote:
On Thu, 18 Oct 2001, Stas Bekman wrote:
 

no prob, let's finish with --? and then I'll try to fix this one too.
actually, it is working with your new patch.
fantastic!

ok, got it, here is a new patch which doesn't use the original @ARGV at
all:
looks good, all problems are fixed now, +1 and thanks!

:)
is there anything else we need to do with opts parsing at this stage?
I think the break in the middle of config is important to fix, right?
_
Stas Bekman JAm_pH  --   Just Another mod_perl Hacker
http://stason.org/  mod_perl Guide   http://perl.apache.org/guide
mailto:[EMAIL PROTECTED]  http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/


Re: Apache-Test breakage

2001-10-18 Thread Doug MacEachern
On Thu, 18 Oct 2001, Stas Bekman wrote:
 
> no prob, let's finish with --? and then I'll try to fix this one too.

actually, it is working with your new patch.
 
> ok, got it, here is a new patch which doesn't use the original @ARGV at
> all:

looks good, all problems are fixed now, +1 and thanks!




Re: Apache-Test breakage

2001-10-18 Thread Stas Bekman
On Wed, 17 Oct 2001, Doug MacEachern wrote:

> On Wed, 17 Oct 2001, Stas Bekman wrote:
>
> >>t/TEST -proxy t/foo/bar.t
> >>
> ...
> > I see exactly the same behavior with my patch and without, so I guess it
> > was broken before.
>
> yup, i already told you the same in private email.  but see the commit
> message:
>
>is not proxying requests:
>t/TEST -proxy
>
> i've not tried your new patch, but yesterday -proxy was not working as it
> was without the patch.

no prob, let's finish with --? and then I'll try to fix this one too.

> and you missed the part of the commit message that complains about this:
> > -for (@$argv) {
> > +for (@ARGV) {
>
> should use @ARGV as little as possible, if Getopts or whatever needs it,
> that's what this was for:
> > -local *ARGV = $self->{args};
>
> reason is that this:
> Apache::TestRun->new->run(@ARGV);
>
> where @ARGV is the args from t/TEST should be able to be called elsewhere
> like so:
>
> for (qw(prefork worker)) {
> Apache::TestRun->new->run(-httpd => "$apache2/$_/bin/httpd");
> }

ok, got it, here is a new patch which doesn't use the original @ARGV at
all:

Index: Apache-Test/README
===
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/README,v
retrieving revision 1.13
diff -u -r1.13 README
--- Apache-Test/README  2001/10/17 01:30:40 1.13
+++ Apache-Test/README  2001/10/18 01:34:51
@@ -7,6 +7,9 @@

 see t/TEST as an example test harness

+For an extensive documentation see
+modperl-2.0/docs/src/devel/writing_tests/writing_tests.pod.
+
 see t/*.t for example tests

 if the file t/conf/httpd.conf.in exists, it will be used instead of
@@ -45,16 +48,16 @@
 % t/TEST -configure

 run as user nobody:
-% t/TEST User nobody
+% t/TEST -User nobody

 run on a different port:
-% t/TEST Port 8799
+% t/TEST -Port 8799

 configure an httpd other than the default (that apxs figures out)
-% t/TEST httpd ~/ap/httpd-2.0/httpd
+% t/TEST -httpd ~/ap/httpd-2.0/httpd

 switch to another apxs
-% t/TEST apxs ~/ap/httpd-2.0-prefork/bin/apxs
+% t/TEST -apxs ~/ap/httpd-2.0-prefork/bin/apxs

 turn on tracing
 % t/TEST -preamble "PerlTrace all"
@@ -69,19 +72,19 @@
 % t/TEST -head

 GET url with authentication credentials
-% t/TEST -get /server-info username dougm password foo
+% t/TEST -get /server-info -username dougm -password foo

 POST url (read content from string)
-% t/TEST -post /TestApache::post content 'name=dougm&company=covalent'
+% t/TEST -post /TestApache::post -content 'name=dougm&company=covalent'

 POST url (read content from stdin)
-% t/TEST -post /TestApache::post content - < foo.txt
+% t/TEST -post /TestApache::post -content - < foo.txt

 POST url (generate a body of data 1024 bytes in length)
-% t/TEST -post /TestApache::post content x1024
+% t/TEST -post /TestApache::post -content x1024

 POST url (only print headers, e.g. useful to just check Content-length)
-% t/TEST -post -head /TestApache::post content x10
+% t/TEST -post -head /TestApache::post -content x10

 GET url (only print headers, e.g. useful to just check Content-length)
 % t/TEST -get -head /foo
Index: Apache-Test/lib/Apache/TestConfig.pm
===
RCS file: 
/home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfig.pm,v
retrieving revision 1.76
diff -u -r1.76 TestConfig.pm
--- Apache-Test/lib/Apache/TestConfig.pm2001/10/17 20:06:25 1.76
+++ Apache-Test/lib/Apache/TestConfig.pm2001/10/18 01:34:51
@@ -68,8 +68,9 @@
 }

 while (my($key, $val) = splice @filter, 0, 2) {
-if ($wanted_args->{$key}) {
-$keep{$key} = $val;
+if ($key =~ /^-?-?(.+)/ # optinal - or -- prefix
+&& exists $wanted_args->{$1}) {
+$keep{$1} = $val;
 }
 else {
 push @pass, $key, $val;
Index: Apache-Test/lib/Apache/TestRun.pm
===
RCS file: 
/home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestRun.pm,v
retrieving revision 1.59
diff -u -r1.59 TestRun.pm
--- Apache-Test/lib/Apache/TestRun.pm   2001/10/17 02:56:24 1.59
+++ Apache-Test/lib/Apache/TestRun.pm   2001/10/18 01:34:52
@@ -82,11 +82,12 @@
 #so we dont slurp arguments that are not tests, example:
 # httpd $HOME/apache-2.0/bin/httpd

-sub split_args {
-my($self, $argv) = @_;
+sub split_test_args {
+my($self) = @_;

-my(@tests, @args);
+my(@tests);

+my $argv = $self->{argv};
 for (@$argv) {
 my $arg = $_;
 #need the t/ for stat-ing, but dont want to include it in test output
@@ -116,12 +117,9 @@
 next;
 }
 }
-
-push @args, $_;
 }

 $self->{tests} = [EMAIL PROTECTED];
-$self->{args}  = [EMAIL PROTECTED];
 }

 sub passenv {
@@ -134,13 +132,15 @@

 sub getopts {
 my($self, $argv) = @_;
-
-$self->split_args($argv);

-#dont count test

Re: Apache-Test breakage

2001-10-17 Thread Doug MacEachern
On Wed, 17 Oct 2001, Stas Bekman wrote:

>>t/TEST -proxy t/foo/bar.t
>>
...
> I see exactly the same behavior with my patch and without, so I guess it
> was broken before.

yup, i already told you the same in private email.  but see the commit
message:

   is not proxying requests:
   t/TEST -proxy

i've not tried your new patch, but yesterday -proxy was not working as it
was without the patch.

and you missed the part of the commit message that complains about this: 
> -for (@$argv) {
> +for (@ARGV) {

should use @ARGV as little as possible, if Getopts or whatever needs it, 
that's what this was for:
> -local *ARGV = $self->{args};

reason is that this:
Apache::TestRun->new->run(@ARGV);

where @ARGV is the args from t/TEST should be able to be called elsewhere
like so:

for (qw(prefork worker)) {
Apache::TestRun->new->run(-httpd => "$apache2/$_/bin/httpd");
}




Re: Apache-Test breakage

2001-10-17 Thread Stas Bekman
Doug MacEachern wrote:

> hi stas, two problems cause by the recentish argv changes you made:
>
> 1) see the thread with ken on the test list:
>
>
>>t/TEST -httpd /path/to/httpd -start
>>
>>starts the server, and then runs all the tests.
>>
>
> looks like recent breakage, args picked up by GetOptions in
> Apache::PerlRun::getopts are being force to come before other options.
> stas can you please fix this.
> in the meantime ken, this will work:
> t/TEST -start -httpd /path/to/httpd

this was a simple fix, I've misread the explanation of the permute option
for Getopts, the fix is:

 -Getopt::Long::Configure(qw(pass_through no_permute));
 +Getopt::Long::Configure(qw(pass_through permute));

permute is enabled by default, but I prefer to explictly specify it since
the environment variable POSIXLY_CORRECT can disable the default.

> 2) args meant for Apache::TestRequest are dropped on the floor, so this
no
> longer works:
>
> t/TEST -post /echo_post -content one=1
>
> nor does
>
> t/TEST -get /authany -username dougm -password dougm
>
> if you need to know which args Apache::TestRequest recognizes, this
> function returns a hash ref:
> Apache::TestRequest::wanted_args()

that's right, I've missed the fact that we have 4 different opts groups in
ARGV :( I've tested only 3 of them.

I've added a support for the TestRequest opts now.

The tests are still correctly looked up as late as possible, so that the
autogenerated tests will have a chance to get created before tested for
existence.

>>t/TEST -proxy t/foo/bar.t
>>
>>runs all of the tests, rather that just the single specified test.
>>also does not run the tests through the proxy.  :(
>
> this may or may not be releated to the patch, cvs.apache.org is very
> slow right now.

I see exactly the same behavior with my patch and without, so I guess it
was broken before.

Here is the patch that fixes the 2 problems you've raised:

? diff
? patch
? run
? run.prefork
? run.prefork.5.6.1
? run.threaded
? run.worker
? Apache-Test/t/TEST
? ModPerl-Registry/Makefile
? ModPerl-Registry/pm_to_blib
? ModPerl-Registry/t/TEST
Index: Apache-Test/README
===
RCS file: /home/cvs/httpd-test/perl-framework/Apache-Test/README,v
retrieving revision 1.13
diff -u -r1.13 README
--- Apache-Test/README  2001/10/17 01:30:40 1.13
+++ Apache-Test/README  2001/10/17 09:18:31
@@ -7,6 +7,9 @@

 see t/TEST as an example test harness

+For an extensive documentation see
+modperl-2.0/docs/src/devel/writing_tests/writing_tests.pod.
+
 see t/*.t for example tests

 if the file t/conf/httpd.conf.in exists, it will be used instead of
@@ -45,16 +48,16 @@
 % t/TEST -configure

 run as user nobody:
-% t/TEST User nobody
+% t/TEST -User nobody

 run on a different port:
-% t/TEST Port 8799
+% t/TEST -Port 8799

 configure an httpd other than the default (that apxs figures out)
-% t/TEST httpd ~/ap/httpd-2.0/httpd
+% t/TEST -httpd ~/ap/httpd-2.0/httpd

 switch to another apxs
-% t/TEST apxs ~/ap/httpd-2.0-prefork/bin/apxs
+% t/TEST -apxs ~/ap/httpd-2.0-prefork/bin/apxs

 turn on tracing
 % t/TEST -preamble "PerlTrace all"
@@ -69,19 +72,19 @@
 % t/TEST -head

 GET url with authentication credentials
-% t/TEST -get /server-info username dougm password foo
+% t/TEST -get /server-info -username dougm -password foo

 POST url (read content from string)
-% t/TEST -post /TestApache::post content 'name=dougm&company=covalent'
+% t/TEST -post /TestApache::post -content 'name=dougm&company=covalent'

 POST url (read content from stdin)
-% t/TEST -post /TestApache::post content - < foo.txt
+% t/TEST -post /TestApache::post -content - < foo.txt

 POST url (generate a body of data 1024 bytes in length)
-% t/TEST -post /TestApache::post content x1024
+% t/TEST -post /TestApache::post -content x1024

 POST url (only print headers, e.g. useful to just check Content-length)
-% t/TEST -post -head /TestApache::post content x10
+% t/TEST -post -head /TestApache::post -content x10

 GET url (only print headers, e.g. useful to just check Content-length)
 % t/TEST -get -head /foo
Index: Apache-Test/lib/Apache/TestConfig.pm
===
RCS file: 
/home/cvs/httpd-test/perl-framework/Apache-Test/lib/Apache/TestConfig.pm,v
retrieving revision 1.74
diff -u -r1.74 TestConfig.pm
--- Apache-Test/lib/Apache/TestConfig.pm2001/10/17 01:30:40 1.74
+++ Apache-Test/lib/Apache/TestConfig.pm2001/10/17 09:18:31
@@ -68,8 +68,9 @@
 }

 while (my($key, $val) = splice @filter, 0, 2) {
-if ($wanted_args->{$key}) {
-$keep{$key} = $val;
+if ($key =~ /^-?-?(.+)/ # optinal - or -- prefix
+&& exists $wanted_args->{$1}) {
+$keep{$1} = $val;
 }
 else {
 push @pass, $key, $val;
Index: Apache-Test/lib/Apache/TestRun.pm
===
RCS file: 
/home/cvs/httpd-test/perl-f