From c24f90011769c5a416761307fe0ad472118b8436 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 27 May 2016 08:35:57 +0200 Subject: [PATCH] Bug 16607: Add quiet flag to builder sub in TestBuilder Content-Type: text/plain; charset=utf-8 If you know what you are doing, you sometimes do not like to see the warnings raised by TestBuilder. This patch allows you to add a quiet flag to an individual build call. Test plan: [1] Add a CPL and a MPL branch (if not present). [2] Run t/db_dependent/TestBuilder.t. You should see no difference. The fix from bug 16500 (suppressing the warn already) has been replaced by using the quiet flag. [3] Run t/db_dependent/Accounts.t too. Before this patch, an existing MPL branch also raised a warn. [4] Run t/db_dependent/Items/AutomaticItemModificationByAge.t too. This test included a check for the existence of a CPL branch. The check has been removed. You should see no warning. Signed-off-by: Marcel de Rooy --- t/db_dependent/Accounts.t | 7 +++--- .../Items/AutomaticItemModificationByAge.t | 6 ++--- t/db_dependent/TestBuilder.t | 4 +--- t/lib/TestBuilder.pm | 23 +++++++++++++++----- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/t/db_dependent/Accounts.t b/t/db_dependent/Accounts.t index 0fb71f2..a48d445 100644 --- a/t/db_dependent/Accounts.t +++ b/t/db_dependent/Accounts.t @@ -52,9 +52,10 @@ my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; my $dbh = C4::Context->dbh; -# this test needs a MPL branch; also create another one -my $builder = t::lib::TestBuilder->new(); -$builder->build({ source => 'Branch', value => { branchcode => 'MPL' }}); +# this test needs a MPL branch; suppress warn if it already exists +my $builder = t::lib::TestBuilder->new; +$builder->build({ source => 'Branch', value => { branchcode => 'MPL' }, quiet => 1 }); +# also create another branch my $library = $builder->build({ source => 'Branch' }); $dbh->do(q|DELETE FROM accountlines|); diff --git a/t/db_dependent/Items/AutomaticItemModificationByAge.t b/t/db_dependent/Items/AutomaticItemModificationByAge.t index 6d7abe2..13cb47d 100644 --- a/t/db_dependent/Items/AutomaticItemModificationByAge.t +++ b/t/db_dependent/Items/AutomaticItemModificationByAge.t @@ -17,11 +17,9 @@ my $dbh = C4::Context->dbh; $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; -# this test assumes a CPL branch +# this test assumes a CPL branch; suppress warn if it exists my $builder = t::lib::TestBuilder->new; -if( !$builder->schema->resultset('Branch')->find('CPL') ) { - $builder->build( { source => 'Branch', value => { branchcode => 'CPL' }}); -} +$builder->build( { source => 'Branch', value => { branchcode => 'CPL' }, quiet => 1 }); $dbh->do(q| DELETE FROM marc_subfield_structure diff --git a/t/db_dependent/TestBuilder.t b/t/db_dependent/TestBuilder.t index 95f986a..e57a59d 100644 --- a/t/db_dependent/TestBuilder.t +++ b/t/db_dependent/TestBuilder.t @@ -53,9 +53,7 @@ subtest 'Start with some trivial tests' => sub { # return undef if a record exists my $param = { source => 'Branch', value => { branchcode => 'MPL' } }; - local $SIG{__WARN__} = sub {}; - $builder->build( $param ); # at least it should exist now - delete local $SIG{__WARN__}; + $builder->build({ %$param, quiet => 1 }); # at least it should exist now warning_like { $builder->build( $param ) } qr/Violation of unique constraint/, 'Catch warn on adding existing record'; diff --git a/t/lib/TestBuilder.pm b/t/lib/TestBuilder.pm index ed28458..d6a5a67 100644 --- a/t/lib/TestBuilder.pm +++ b/t/lib/TestBuilder.pm @@ -13,6 +13,7 @@ sub new { $self->schema->storage->sql_maker->quote_char('`'); $self->{gen_type} = _gen_type(); + $self->{verbose} = 1; # default behavior; see also quiet flag in build return $self; } @@ -57,6 +58,9 @@ sub build { my $source = $params->{source} || return; my $value = $params->{value}; + # suppress some warns during this build? + $self->{verbose} = !$params->{quiet}; + my $col_values = $self->_buildColumnValues({ source => $source, value => $value, @@ -198,7 +202,7 @@ sub _buildColumnValues { return $col_values if $build_value > 0; # if you get here, we have a problem - warn "Violation of unique constraint in $source"; + warn "Violation of unique constraint in $source" if $self->{verbose}; return; } @@ -263,7 +267,8 @@ sub _buildColumnValue { my $retvalue = []; if( $col_info->{is_auto_increment} ) { if( exists $value->{$col_name} ) { - warn "Value not allowed for auto_incr $col_name in $source"; + warn "Value not allowed for auto_incr $col_name in $source" + if $self->{verbose}; return; } # otherwise: no need to assign a value @@ -271,7 +276,8 @@ sub _buildColumnValue { if( exists $value->{$col_name} ) { if( !defined $value->{$col_name} && !$col_info->{is_nullable} ) { # This explicit undef is not allowed - warn "Null value for $col_name in $source not allowed"; + warn "Null value for $col_name in $source not allowed" + if $self->{verbose}; return; } if( ref( $value->{$col_name} ) ne 'HASH' ) { @@ -281,12 +287,13 @@ sub _buildColumnValue { } } elsif( ref( $value->{$col_name} ) eq 'HASH' ) { # this is not allowed for a column that is not a FK - warn "Hash not allowed for $col_name in $source"; + warn "Hash not allowed for $col_name in $source" if $self->{verbose}; return; } elsif( exists $value->{$col_name} ) { if( !defined $value->{$col_name} && !$col_info->{is_nullable} ) { # This explicit undef is not allowed - warn "Null value for $col_name in $source not allowed"; + warn "Null value for $col_name in $source not allowed" + if $self->{verbose}; return; } push @$retvalue, $value->{$col_name}; @@ -296,7 +303,8 @@ sub _buildColumnValue { if( my $hdlr = $self->{gen_type}->{$data_type} ) { push @$retvalue, &$hdlr( $self, { info => $col_info } ); } else { - warn "Unknown type $data_type for $col_name in $source"; + warn "Unknown type $data_type for $col_name in $source" + if $self->{verbose}; return; } } @@ -484,6 +492,9 @@ Note that you should wrap these actions in a transaction yourself. Realize that passing primary key values to build may result in undef if a record with that primary key already exists. + The build method also supports a boolean quiet parameter to suppress + some warnings. It only pertains to the current build call. + =head1 AUTHOR Yohann Dufour -- 1.7.10.4