Re: Proposal: Test::MakeVariantTestFiles - take 2

Am 15.10.2013 um 13:54 schrieb Jens Rehsack <[email protected]>:

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


Example DBD::DBM tests …

My current perlbrew for dbi/dbit contains DB_File, SDBM_File and MLDBM …

==> dbd-config contains

	drivers => {
		     dbm => {
			      variants   => {
					    mldbm => {
						       f => { dbm_mldbm => 'FreezeThaw' },
						       d => { dbm_mldbm => 'Data::Dumper' },
						       s => { dbm_mldbm => 'Storable' },
						       j => { dbm_mldbm => 'JSON' },
						     },
					    type => {
						      s => { dbm_type => 'SDBM_File' },
						      d => { dbm_type => 'DB_File' },
						    },
					  },
			      name => "DSN for DBM",
			    },

==> $ find t/DBI -name "*dbm*"
t/DBI/simple/dvd_dbm.t
t/DBI/simple/dvds_dbm.t
t/DBI/simple/dvdsd_dbm.t
t/DBI/simple/dvdsj_dbm.t
t/DBI/simple/dvdss_dbm.t
t/DBI/simple/zvg_dvd_dbm.t
t/DBI/simple/zvg_dvds_dbm.t
t/DBI/simple/zvg_dvdsd_dbm.t
t/DBI/simple/zvg_dvdsj_dbm.t
t/DBI/simple/zvg_dvdss_dbm.t
t/DBI/simple/zvgp_dvd_dbm.t
t/DBI/simple/zvgp_dvds_dbm.t
t/DBI/simple/zvgp_dvdsd_dbm.t
t/DBI/simple/zvgp_dvdsj_dbm.t
t/DBI/simple/zvgp_dvdss_dbm.t
t/DBI/simple/zvp_dvd_dbm.t
t/DBI/simple/zvp_dvds_dbm.t
t/DBI/simple/zvp_dvdsd_dbm.t
t/DBI/simple/zvp_dvdsj_dbm.t
t/DBI/simple/zvp_dvdss_dbm.t

eg. /DBI/simple/dvdss_dbm.t expands attributes => {dbm_mldbm => 'Storable',dbm_type => 'SDBM_File'}

-- 
Jens Rehsack
pkgsrc, Perl5
[email protected]