Perl's not the only TAP source

[David E. Wheeler]

On Apr 13, 2009, at 3:22 PM, Steve Purkis wrote:

> Righty-o,
>
> There's a workable draft in trunk.  Before you panic, don't: it's  
> not yet used by TAP::Parser, I just wanted to commit something as a  
> discussion point.

Nice, thanks!

> If this is the right way, to finish this I reckon we'd need to:
>
> 1. make TAP::Parser::Source more generic
> * not everything's a cmd anymore
> * so subclassing 'pgTAP' would make sense

Yeah, this is obviously something I'd be interested in contributing.  
On a quick look, it appears that you essentially rely on file names to  
decide what to do with things, yes? So would we need to standardize on  
some kind of file extension for that? Is that really the position we  
want to take?

Also, how would a source class collect options from the user? For  
example, pgTAP needs a database name username, and possibly a host  
name and port to connect to. The current implementation of [pg_prove] 
[] collects this information, assembles a command, and then simply  
passes it via the `exec` parameter. How would I go about using a  
source to do the same stuff?

[pg_prove]: http://github.com/theory/pgtap/raw/master/bin/pg_prove

Maybe my questions will be mooted by the subclassing guidee.

However, I'm also not convinced that this needs to be quite so, um,  
complex. Do I really have to implement two different classes (Source  
and SourceDetector) to add support for a new source? Why not just one  
module or class with the necessary goodies in it?

> 2. integrate the SourceFactory into TAP::Parser
> * replace all the source stuff in _initialize() with it
> * maybe replace the 'exec' stuff too?

I guess you'd add an Exec source?

> ** if not, introduce an 'exec_source_class'
>
> 3. update docs
> * subclassing guide
>
> All of this requires a deprecation cycle... which means a release.   
> I suppose all the work could be done in a sub-class of TAP::Parser.

What backwards compatibility would be broken?

> What do people think - is this the right way forward?

It looks good, I'm thrilled you're taking this on, and I look forward  
to putting it to good use! It should greatly simplify how I set this  
stuff up in a Build.PL file, which right now looks like this:

     my $class = Module::Build->subclass(code => <<'EOF');
     sub tap_harness_args {
         return {
             exec => sub {
                 my ( $harness, $test_file ) = @_;
                 # Let Perl tests run.
                 return undef if $test_file =~ /[.]t$/;
                 return [qw( psql -d try -f ), $test_file ]
                     if $test_file =~ /[.]s$/;
             },
         };
     }
     EOF

     my $build = $class->new(
         module_name        => 'MyTapTest',
         license            => 'perl',
         test_file_exts     => [qw(.t .s)], # <<<< LOOK AT THAT!
         use_tap_harness    => 1,
         configure_requires => { 'Module::Build' => '0.280802' },
         build_requires     => {
             'Module::Build' => '0.280802',
             'TAP::Harness'  => '3.12',
             'Test::More'    => '0.17',
         },
     );
     $build->create_build_script;

It's pretty ugly, and the need to actually use a code reference has  
forced me to subclass, rather than just pass parameters that can be  
written to Module::Build's cache. I'd much rather do something like  
this:

     my $build = $class->new(
         module_name        => 'MyTapTest',
         license            => 'perl',
         test_file_exts     => [qw(.t .s)], # <<<< LOOK AT THAT!
         use_tap_harness    => 1,
         tap_harness_args   => { psql => [qw( psql -d try -f )] },
         configure_requires => { 'Module::Build' => '0.280802' },
         build_requires     => {
             'Module::Build' => '0.280802',
             'TAP::Harness'  => '3.12',
             'Test::More'    => '0.17',
         },
     );
     $build->create_build_script;

Where the `psql` parameter is something that the pgTAP source could  
then use to run its tests, but that the Perl source would ignore.

Thanks,

David