Re: Proposal: Test::MakeVariantTestFiles - take 2

Am 07.10.2013 um 23:51 schrieb Tim Bunce <[email protected]>:

My apologizes for later reply - very, very busy and trouble'ish
weeks … :/

I try to stay short *gg*

> [sorry for the delay] Here's a more fleshed-out example...
> 
> This example is talking about two modules:
> 
> One that provides the write_variants sub and some related classes.
> It's dumb, with no DBI or DBIT knowledge at all. Let's call it
> WriteVariants for now.
> 
> The other is closer to the original DBIT 'DSN provider'. Let's call it
> DBIT::Provider for now. It calls WriteVariants with the appropriate
> arguments to create the variant test files needed for DBIT.
> That's a set of tests, plus a list of callbacks.
> 
> 
> Rough terminology:
> 
> A 'setting' is an instance of a trivial class that can provide a string
> of code for setup and teardown of a setting.
> 
> new_env_setting is a function that returns an instance of 'setting'
> object that represents an environment variable.
> 
> new_multi_setting is a function that returns an instance of 'setting'
> object that represents an ordered list of other settings.
> 
> A 'setting provider' callback returns a hash of name-settings pairs.
> 
> 
> Example:
> 
> write_variants(
>    providers => [  # the order is important
>        \&dbi_settings_provider,
>        \&driver_settings_provider,
>        \&dbd_settings_provider,
>    ],
>    test_cases => { # details not important here
>        # 
>        01basics => TC->new(...),
>        02dbidrv => TC->new(...),
>        ...
>    },
> );

Probably that test_cases part should be discussed again at weekly
DBIT meeting. Until that (and because of it looks not important
for following details), I ignore that they're mentioned here ^^

> sub dbi_settings_provider {
> 
>    my %settings = (
>        pureperl => new_env_setting(DBI_PUREPERL => 2),
>        gofer    => new_env_setting(DBI_AUTOPROXY => 'dbi:Gofer:transport=null;policy=pedantic'),
>    );
> 
>    # Add combinations:
>    # Returns the original settings plus extras created by combining.
>    # In this case returns one extra key-value pair, i.e.:
>    # $settings{pureperl_gofer} = new_multi_setting( $settings{pureperl}, $settings{gofer} );
>    %settings = add_combinations(%settings);
> 
>    # add a 'null setting' that tests plain DBI with default environment
>    $settings{plain} = undef;
> 
>    return %settings;
> }

Beside the unlucky example, that looks suitable. Initially I was a bit against
putting the "add_combinations" into this sub - but thinking over it for a while
and suddenly it makes sense …

> This dbi_settings_provider returns four sets of name-settings pairs.
> One has no settings, two have a single setting, and one has two settings.
> 
> 
> sub driver_settings_provider {
>    my ($settings_context) = @_;
> 
>    my @drivers = Test::Database->list_drivers("available");
> 
>    # filter out non-pureperl drivers if testing with DBI_PUREPERL
>    @drivers = grep { driver_is_pureperl($_) } @drivers
>        if $settings_context->env('DBI_PUREPERL');
> 
>    return map { $_ => new_env_setting(DBI_DRIVER => $_) } @drivers;
> }
> 
> This driver_settings_provider is called once for each of the
> name-settings pairs returned by dbi_settings_provider.
> 
> The $settings_context argument is an object representing the
> settings that are 'in effect' for this call.
> 
> It returns a name-settings pair for each driver that's
> testable with the current settings (e.g. DBI_PUREPERL).
> 
> 
> sub dbd_settings_provider {
>    my ($settings_context) = @_;
> 
>    # this would dispatch to plug-ins based on the value of
>    # $settings_context->env('DBI_DRIVER')
> 
>    return %settings;
> }
> 
> This dbd_settings_provider is called once for each of the
> name-settings pairs returned by driver_settings_provider
> for each of name-settings pairs returned by dbi_settings_provider.
> 
> For example, the plug-in for the DBM driver would return settings for
> combinations of dbm_mldbm and dbm_type. The plug-in for the CSV driver
> would return settings for csv_class etc. Both would have variants for
> SQL::Nano vs SQL::Statement.

Well, those variants are in dbi_settings (because of API history …) and
cannot simply move to a dbd_settings_provider. But this one could
(following you API proposal) filter or inject such settings into the
dbi tree and force a re-combination …

This is - however - a part which has to be rethought together.

> write_variants(
>    providers => [  # the order is important
>        \&dbi_settings_provider,
>        \&driver_settings_provider,
>        \&dbd_settings_provider,
>    ],
>    test_cases => { # details not important here
>        # 
>        foo => TC->new(...),
>        bar => TC->new(...),
>        ...
>    },
> );
> 
> When write_variants is called, as shown above, it would call dbi_settings_provider,
> for each setting returned by that sub it would call driver_settings_provider,
> for each setting returned by that sub it would call dbd_settings_provider
> for each setting returned by that sub it would write the test_cases
> using the current settings from each provider being iterated over.
> 
> As each test case it written it would include the setup and teardown
> code for each of the current settings. Each test case could be an
> instance of a trivial object with a simple API so people could use their
> favorite template system if they want.
> 
> When write_variants returns we'd have a directory tree like this:
> 
>    t/
>      plain/
>        csv/
>          sql_nano/
>            foo.t
>            bar.t
>          sql_statement/
>            foo.t
>            bar.t
>        SQLite/
>          foo.t
>          bar.t
>      gofer/
>        csv/
>          sql_nano/
>            ...
>          sql_statement/
>            ...
>        SQLite/
>          ...
>      pureperl_gofer/
>        .../

You suggest to go deep down into directory structure instead of doing
t/dbit/zvgn_cvp_foo.t
t/dbit/zvgn_cvx_foo.t
t/dbit/zvg_cvp_foo.t
t/dbit/zvg_cvx_foo.t
t/dbit/zvgn_cvp_bar.t
t/dbit/zvgn_cvx_bar.t
t/dbit/zvgn_cvp_bar.t
t/dbit/zvgn_cvx_bar.t

It's nicer - but you currently ignore namespace prefixes of foo.t coming
from DBI and another foo.t coming from SQL::Statement (well - we can
coordinate - but I think you get my example).

> etc.
> 
> Clearly the example above is rough but I hope it's enough to give a good
> sense of the approach I'm suggesting.
> 
> I think this gives us good separation of concerns. There's a clear
> division of responsibility between WriteVariants on one hand, plug-in
> modules providing variants on the other, and DBIT::Provider glueing them
> together in the middle. All with clear responsibilities and interfaces.

You improved the internal API of DBI::Test::Conf - that's what currently
happens there ;)

> WriteVariants (or whatever it gets called) is clearly not dependant on DBI
> and I can't see any reason DBIT::Provider (or whatever it gets called)
> should be either.
> 
> Any thouhts?


Just for clarification how it's currently (it needs API design, no question!):

And finally - current DBIT (ignoring already outsourced DSN::Provider ^^) is
70% test variant generator
20% DBI::Mock
8% test support (connect_ok …)
2% test cases for DB[ID] 

What is that DBI::Mock for? It makes it much easier to write same test for
Database engine than for a DBD by using a suitable (RootClass => ) in connect.

Cheers
-- 
Jens Rehsack
[email protected]