httpd-test-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject Re: need help to add per-user config to Apache::Test
Date Sat, 06 Sep 2003 16:35:14 GMT
Randy Kobes wrote:
> On Thu, 4 Sep 2003, Stas Bekman wrote:
> 
> 
>>In the effort to remove some of the Win32 noise, I was
>>thinking that we can write a generic function which gets a
>>path as an argument and figures out internally if it needs
>>to keep the argument as passed or mangle it. So it'll do
>>something like:
>>
>>     my $cwd =  Apache::TestUtil::path(cwd);
>>
>>probably need a more intuitive name for this function.
> 
> 
> That'd be nice - a version that does this appears below.
> I named it win32_long_path - it'll just return what was
> passed into it if not on Win32.

But my idea was to eliminate any os-specific words win32. I think it should 
just be long_path... Think of it as File::Spec function.

I'm still not quite happy about the naming of the function, what exactly 
GetLongPathName($file) does?
can this be done using some File::Spec function?

> +require File::Spec;

> +        my $candidate = File::Spec->catfile($_, 'Apache', $file);

we import catfile in all other Apache::Test files, can do that here as well... 
will make code simpler

> +        if (-e $candidate) {
> +            $sys_config = $candidate;
> +            last;
> +        }
> +    }
> +    if ($sys_config) {
> +        eval {require $sys_config};
> +        return $sys_config if (not $@ and IN_APACHE_TEST);
> +        $sys_config = undef if $@;
> +    }

Hmm, I thought we agreed that eval {require $sys_config} will always succeed, 
since that file is always available. so this code should be needed:

+ die 'Could not find a suitable Apache::TestConfigData'
+    unless defined CONFIG_DATA;

One more problem is TestRun.pm's layout: subs should go after the declarations 
(uses/constants/etc). use 'use subs' if you need to predeclare them.

Another issue, is in_apache_test. Since a user may get it with modperl-2.0 or 
separately it should return true in either case.

> +sub write_config {
> +    my $self = shift;
> +    my $fh = Symbol::gensym();
> +    my $vars = $self->{test_config}->{vars};
> +    my $conf_opts = $self->{conf_opts};
> +    my $file = IN_APACHE_TEST ?
> +        catfile($vars->{top_dir}, CONFIG_DATA) :
> +        CONFIG_DATA;

it's easier to parse when written as:

my $file = IN_APACHE_TEST
     ? catfile($vars->{top_dir}, CONFIG_DATA)
     : CONFIG_DATA;

> +    my $pkg = << "EOC";
> +package Apache::TestConfigData;

better have it strict, to avoid misterious errors (same in the pod below)

use strict;
use warnings;

use vars (\$Apache::TestConfigData);

> +\$Apache::TestConfigData = {
> +$config_dump
> +};
> +1;
> +
> +=head1 NAME
> +
> +Apache::TestConfigData - Configuration file for Apache::Test
> +
> +=cut
> +EOC
> +    print $fh $pkg;
> +    close $fh;
> +    return 1;
> +}
> +
>  1;



__________________________________________________________________
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


Mime
View raw message