httpd-test-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Randy Kobes <ra...@theoryx5.uwinnipeg.ca>
Subject Re: need help to add per-user config to Apache::Test
Date Sat, 06 Sep 2003 19:17:05 GMT
On Sat, 6 Sep 2003, Stas Bekman wrote:

> 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.

OK, I'll do that. I put in the win32_ to indicate that it'll
do something for Win32, and otherwise just return what was
passed in.

> 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?

I haven't found an equivalent one in File::Spec ... The
problem here is that cwd (or any file/directory name) on
Win32 has two representations - a long path name (that can
include spaces) and a short path name, limited to 6
characters (this is a holdout from early days), plus 3 if an
extension is present.  Here we want to match cwd to
/Apache-Test/, but cwd may return the short path name (eg,
Apache~1.0), depending on if, for example, you have
different Apache-Test* directories at the same level. So
GetLongPathName can be used to get the full long path name.

The thing with naming it, eg, just long_path, is that
Unix users (the majority) wouldn't know what it does.

> > +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

Good point - thanks.

>
> > +        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;

I was thinking here if someone, after installation, had
mis-edited Apache::TestConfigData, either the system one or
one found in some path in @INC being added thru, eg, use
lib). But this might be overkill - not worrying about this
at this time will simplify things here.

> 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.

OK - thanks.

> 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.

I forgot about that - thanks.

> > +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);

Thanks - I'll add that.

I was also thinking about the problem of $ENV{HOME} not
being available in mod_perl. As Apache::TestConfigData is
being loaded in order to configure things, shouldn't
this not be a problem, because at this point one isn't
yet in a mod_perl environment?

-- 
best regards,
randy

Mime
View raw message