Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

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

Closed
ntyni opened this Issue Mar 31, 2012 · 4 comments

Comments

Projects
None yet
3 participants

ntyni commented Mar 31, 2012

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 commented Mar 31, 2012

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");
}

Owner

bigpresh commented Jun 10, 2012

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 commented Jun 28, 2012

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

Owner

bigpresh commented Sep 13, 2012

This was fixed in 85e8cd0 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 :)

@bigpresh bigpresh closed this Sep 13, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment