You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Geoffrey Young <ge...@modperlcookbook.org> on 2003/11/07 18:56:28 UTC

Re: cvs commit: modperl-2.0/lib/ModPerl TestRun.pm


stas@apache.org wrote:
> stas        2003/11/05 01:52:18
> 
>   Modified:    .        Makefile.PL Changes
>                ModPerl-Registry/t TEST.PL
>                lib/ModPerl TestRun.pm
>   Log:
>   When 'make test' fails we now print the info on what to do next


the attached patch makes it possible to use -bugreport with the 
Apache::TestRun(Perl)->generate_script() form, which is nice for end users 
who are not interested in creating a t/TEST.PL template (which is most at 
this point I'd think).

I realize now that $self->can('bug_report') was a better idea - using the 
no-op method throws warnings that are trapped with fatal => ALL.

--Geoff

Re: cvs commit: modperl-2.0/lib/ModPerl TestRun.pm

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
>> Also I'd like to suggest another change. If you introduce key=>val 
>> arguments, the old arg should support that as well, like so:
>>
>> generate_script( file      => 't/TEST',
>>                  bugreport => My::Foo::bugreport());
>>
>> What do you think? And we keep the back-compat:
>>
>> generate_script('t/TEST');
> 
> 
> 
> I already took care of that:
> 
> +    if (@opts == 1) {
> +        $opts{file} = $opts[0];
> +    }
> +    else {
> +        %opts = @opts;
> +        $opts{file} ||= catfile 't', 'TEST';
> +    }

Ah, of course! my embed-brain-perl has failed to dive into the else branch ;)


__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


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


Re: cvs commit: modperl-2.0/lib/ModPerl TestRun.pm

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Also I'd like to suggest another change. If you introduce key=>val 
> arguments, the old arg should support that as well, like so:
> 
> generate_script( file      => 't/TEST',
>                  bugreport => My::Foo::bugreport());
> 
> What do you think? And we keep the back-compat:
> 
> generate_script('t/TEST');


I already took care of that:

+    if (@opts == 1) {
+        $opts{file} = $opts[0];
+    }
+    else {
+        %opts = @opts;
+        $opts{file} ||= catfile 't', 'TEST';
+    }

:)

--Geoff


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


Re: cvs commit: modperl-2.0/lib/ModPerl TestRun.pm

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>  but I kinda thought the delayed eval made it a bit
> 
>>> neater - that's the added value :)
>>
>>
>>
>> Delayed eval? I fail to see where the delay is coming from? You run 
>> eval {} in generate_script, 
> 
> 
> don't take delay too literally.  it was just a bad choice of words on my 
> part - we both know what the code is doing :)

Yes, but I thought that you really wanted to have a hook that will run from 
t/TEST. That's why I couldn't figure out how have you planned to accomplish 
that with passing a CODEREF ;)

You said that it gives the benefit of having an access to config stuff:

# subroutines are eval'd when t/TEST is generated, so config-time
# variables such as Apache::TestConfig::IS_MOD_PERL_2 can be used

but you get them anyway ;)


>> so why not just run it in Makefile.PL and pass the result? 
> 
> 
> because that's messy and not really what people want from an API.

Is this more messier:
   generate_script(bugreport => My::Foo::bugreport());
than:
   generate_script(bugreport => \&My::Foo::bugreport);

I believe on the opposite the first form is clear, whereas the second is 
obscure, since you have no idea when is it going to be executed, leading some 
to think that it will be executed from t/TEST.

>> I think with the proposed patch these 2 things do exactly the same 
>> thing at exactly (well almost) the same time:
>>
>> generate_script(bugreport => \&My::Foo::bugreport);
>> generate_script(bugreport => My::Foo::bugreport());
> 
> 
> sure, you can run the second form as well.
> 
> as I said I don't really care, so I'll just take the cv logic out.

I guess if people need to write a real function that does things besides 
printing some string, they should use t/TEST.PL

Also I'd like to suggest another change. If you introduce key=>val arguments, 
the old arg should support that as well, like so:

generate_script( file      => 't/TEST',
                  bugreport => My::Foo::bugreport());

What do you think? And we keep the back-compat:

generate_script('t/TEST');

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


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


Re: cvs commit: modperl-2.0/lib/ModPerl TestRun.pm

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
  but I kinda thought the delayed eval made it a bit
>> neater - that's the added value :)
> 
> 
> Delayed eval? I fail to see where the delay is coming from? You run eval 
> {} in generate_script, 

don't take delay too literally.  it was just a bad choice of words on my 
part - we both know what the code is doing :)

> so why not just run it in Makefile.PL and pass 
> the result? 

because that's messy and not really what people want from an API.

> I think with the proposed patch these 2 things do exactly 
> the same thing at exactly (well almost) the same time:
> 
> generate_script(bugreport => \&My::Foo::bugreport);
> generate_script(bugreport => My::Foo::bugreport());

sure, you can run the second form as well.

as I said I don't really care, so I'll just take the cv logic out.

--Geoff


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


Re: cvs commit: modperl-2.0/lib/ModPerl TestRun.pm

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
>> Cool.
>>
>> Though I'm not sure about the CODEREF part. bugreport is a just a 
>> function to run if 'make test' fails. Though you hardcoded it to be a 
>> function to print something. Since eval of that CODEREF happens during 
>> generate_script() you could just do it and pass that return value to 
>> bugreport. i.e. it provides no added value.
> 
> 
> well, I was thinking that this might be useful and a bit more dwimmy.
> 
> generate_script(bugreport => \&My::Foo::bugreport);
> 
> of course you could, as you say, evaluate the sub then pass in the 
> resulting string, but I kinda thought the delayed eval made it a bit 
> neater - that's the added value :)

Delayed eval? I fail to see where the delay is coming from? You run eval {} in 
generate_script, so why not just run it in Makefile.PL and pass the result? I 
think with the proposed patch these 2 things do exactly the same thing at 
exactly (well almost) the same time:

generate_script(bugreport => \&My::Foo::bugreport);
generate_script(bugreport => My::Foo::bugreport());

If I miss something please explain.

> but whatever, it doesn't matter to me.




__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


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


Re: cvs commit: modperl-2.0/lib/ModPerl TestRun.pm

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Cool.
> 
> Though I'm not sure about the CODEREF part. bugreport is a just a 
> function to run if 'make test' fails. Though you hardcoded it to be a 
> function to print something. Since eval of that CODEREF happens during 
> generate_script() you could just do it and pass that return value to 
> bugreport. i.e. it provides no added value.

well, I was thinking that this might be useful and a bit more dwimmy.

generate_script(bugreport => \&My::Foo::bugreport);

of course you could, as you say, evaluate the sub then pass in the resulting 
string, but I kinda thought the delayed eval made it a bit neater - that's 
the added value :)

but whatever, it doesn't matter to me.

--Geoff


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


Re: cvs commit: modperl-2.0/lib/ModPerl TestRun.pm

Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> 
>> the attached patch makes it possible to use -bugreport with the 
>> Apache::TestRun(Perl)->generate_script() form

You do: eval { $report->() } but don't check for $@. Better just drop eval {} 
and let it fail.

> sorry, I meant to include examples.  with this patch, you can use the 
> following formats in your Makefile.PL
> 
> # scalars are printed in the post-error banner
> Apache::TestRun(Perl)->generate_script(bugreport => 'some message');
> 
> # subroutines are eval'd when t/TEST is generated, so config-time
> # variables such as Apache::TestConfig::IS_MOD_PERL_2 can be used
> Apache::TestRun(Perl)->generate_script(bugreport => sub { 'some message' 
> });
> 
> # continues to work as before (if anyone actually uses it)
> Apache::TestRun->generate_script('t/TEST');

Cool.

Though I'm not sure about the CODEREF part. bugreport is a just a function to 
run if 'make test' fails. Though you hardcoded it to be a function to print 
something. Since eval of that CODEREF happens during generate_script() you 
could just do it and pass that return value to bugreport. i.e. it provides no 
added value.

The only useful way I can see we can benefit from passing a code ref is this:

sub gen_bugreport {
     print 'EOI';
require Foo;
Foo->Bar;
EOI

}

Apache::TestRun(Perl)->generate_script(bugreport => \&gen_bugreport);

and in generate_script:

    if (my $report = $opts{bugreport}) {
         my $sub;
         if (UNIVERSAL::isa($report, 'CODE')) {
             my $code = $report->();
             $sub = "sub bug_report {\n    $code\n}\n";
         }
         else {
             $sub = "sub bug_report {\n    print '$report'\n}\n";
         }
         $body .= "\n\npackage $class;\n" . "$sub\n\n";
     }

i.e. where we pass a CODEREF which when run returns an uncompiled body of the 
bug_report, which is then written to t/TEST.

__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


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


Re: cvs commit: modperl-2.0/lib/ModPerl TestRun.pm

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> the attached patch makes it possible to use -bugreport with the 
> Apache::TestRun(Perl)->generate_script() form

sorry, I meant to include examples.  with this patch, you can use the 
following formats in your Makefile.PL

# scalars are printed in the post-error banner
Apache::TestRun(Perl)->generate_script(bugreport => 'some message');

# subroutines are eval'd when t/TEST is generated, so config-time
# variables such as Apache::TestConfig::IS_MOD_PERL_2 can be used
Apache::TestRun(Perl)->generate_script(bugreport => sub { 'some message' });

# continues to work as before (if anyone actually uses it)
Apache::TestRun->generate_script('t/TEST');

--Geoff


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