t/01-basic.t failure due to hashref stringification problems

[ntyni]

Hi,

as reported in http://bugs.debian.org/665221 we're seeing a test failure in t/01-basic.t:

PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t

Testing Dancer::Plugin::Database 1.81, Perl 5.014002, /usr/bin/perl

t/00-load.t ....... ok

Failed test 'database_connection_failed hook fires'

at t/01-basic.t line 155.

got: ''

expected: '1'

Looks like you failed 1 test of 41.

t/01-basic.t ...... Dubious, test returned 1 (wstat 256, 0x100) Failed 1/41 subtests

Testing Dancer::Plugin::Database::Handle 0.12, Perl 5.014002, /usr/bin/perl

t/02-handle.t ..... ok t/manifest.t ...... skipped: Author tests not required for installation t/pod-coverage.t .. ok t/pod.t ........... ok

Test Summary Report

t/01-basic.t (Wstat: 256 Tests: 41 Failed: 1) Failed test: 41 Non-zero exit status: 1 Files=6, Tests=50, 2 wallclock secs ( 0.07 usr 0.02 sys + 0.68 cusr 0.11 csys = 0.88 CPU) Result: FAIL Failed 1/6 test programs. 1/50 subtests failed. make[1]: *\ [test_dynamic] Error 255

This is reproducible for me with 'make test' (but not with 'prove -b t/*.t').

The problem is in using of argument hashref stringification as a database handle index in Dancer::Plugin::Database::database(). This fails when memory is reused so that a different argument hash gets the same memory address and hence the same stringification.

Adding this instrumentation into Database.pm:60 or thereabouts:

use Data::Dumper; my $new = Dumper($handle_key);

if (exists $strings{$handle_key}) { my $old = $strings{$handle_key}; warn "hashref collision: $old vs. $new" if $old ne $new; } $strings{$handle_key} = $new;

gives this output when the test fails:

t/01-basic.t ...... 40/41 hashref collision: $VAR1 = { 'database' => ':memory:', 'driver' => 'SQLite' }; vs. $VAR1 = { 'dsn' => 'dbi:SQLite:/Please/Tell/Me/This/File/Does/Not/Exist!', 'dbi_params' => { 'HandleError' => sub { "DUMMY" }, 'RaiseError' => 0, 'PrintError' => 0 } };

Failed test 'database_connection_failed hook fires'

at t/01-basic.t line 155.

got: ''

expected: '1'

Looks like you failed 1 test of 41.

which clearly shows the problem: the test is making sure that a nonexistent database can't be opened, but the code is reusing a handle for an in-memory database that's working fine.

I'm attaching a test script that should demonstrate this in a reproducible way.

I suppose the handle index should include a serialization of the arguments. However, I'm afraid that's hard to do for the general case as they may include code references (like the 'HandleError' subroutine above).

Many thanks for your work,

Niko Tyni (Debian Perl Group) ntyni@debian.org

[ntyni]

Wow, sorry about the strange rendering.

I can't see a way to attach files here, so I guess I'll have to cut and paste this. You can find it as an attachment through the Debian bug report (http://bugs.debian.org/665221).

!perl

use Test::More tests => 3; use Dancer::Plugin::Database;

my $opt = { dsn => "dbi:SQLite:dbname=:memory:", dbi_params => { HandleError => sub { return 0 }, # gobble connect failed message RaiseError => 0, PrintError => 0, }, };

my $handle;

$handle = database($opt); ok($handle, "got handle for an in-memory database");

$opt->{dsn} = "dbi:SQLite:dbname=/Please/Tell/Me/This/File/Does/Not/Exist!", my $opt2; %$opt2 = %$opt;

$handle = database($opt2); ok(!$handle, "no handle returned for a nonexistent database");

TODO: { local $TODO = "broken indexing of cached database handles"; $handle = database($opt); ok(!$handle, "no cached handle returned for a nonexistent database"); }

[bigpresh]

Somehow I'd missed this one! Hmm, that's a very good point indeed. I'll have to have a think on the best way to handle that.

[dams]

What you really want is have a reasonably unique signature of the args passed to database(). Instead of using the address of the given hashref, you could also use some of its key / value to build the signature. Maybe concatenate string representations of its content, to a reasonable extend ? like

$signature = join('|', map { join( '-', (@$_ )[0..9] ) } [ keys %$arg ], [ values %$arg ] )

Or something like that. keys and values returning always the same order, that'd work. Or something more clever

[bigpresh]

This was fixed in 85e8cd06ff some time ago, and the bug on bugs.debian.org closed, but looks like I never got round to closing this issue here on GH :)