From 623438fa6d3e13b59f3690f336fd625dd5f82038 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 28 Jan 2025 15:12:39 +0100 Subject: [PATCH] Bug 38944: Correctly catch warnings raised by DBI --- .../Circulation/MarkIssueReturned.t | 23 ++-- t/db_dependent/Koha/Account.t | 39 ++++--- t/db_dependent/Koha/ApiKey.t | 25 ++-- t/db_dependent/Koha/Checkouts/Renewal.t | 37 +++--- t/db_dependent/Koha/Checkouts/ReturnClaim.t | 12 +- t/db_dependent/Koha/Object.t | 110 +++++++++--------- t/db_dependent/Koha/Patron.t | 19 +-- t/db_dependent/Koha/Patron/Relationship.t | 36 +++--- t/db_dependent/Koha/Suggestions.t | 84 +++++++------ t/db_dependent/Suggestions.t | 24 ++-- t/db_dependent/api/v1/item_groups.t | 9 +- t/db_dependent/selenium/01-installation.t | 1 - 12 files changed, 202 insertions(+), 217 deletions(-) diff --git a/t/db_dependent/Circulation/MarkIssueReturned.t b/t/db_dependent/Circulation/MarkIssueReturned.t index 131ac5fe76d..632289ecce8 100755 --- a/t/db_dependent/Circulation/MarkIssueReturned.t +++ b/t/db_dependent/Circulation/MarkIssueReturned.t @@ -20,6 +20,7 @@ use Modern::Perl; use Test::NoWarnings; use Test::More tests => 6; use Test::Exception; +use Test::Warn; use t::lib::Mocks; use t::lib::TestBuilder; @@ -128,7 +129,7 @@ subtest 'Anonymous patron tests' => sub { subtest 'Manually pass a return date' => sub { - plan tests => 3; + plan tests => 4; $schema->storage->txn_begin; @@ -158,17 +159,17 @@ subtest 'Manually pass a return date' => sub { $issue = C4::Circulation::AddIssue( $patron, $item->barcode ); - { - # Hiding the expected warning displayed by DBI - # DBD::mysql::st execute failed: Incorrect datetime value: 'bad_date' for column 'returndate' - local *STDERR; - open STDERR, '>', '/dev/null'; - throws_ok - { $issue_id = C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item->itemnumber, 'bad_date', 0 ); } + warning_like( + sub { + throws_ok { + $issue_id = + C4::Circulation::MarkIssueReturned( $patron->borrowernumber, $item->itemnumber, 'bad_date', 0 ); + } 'Koha::Exceptions::Object::BadValue', - 'An exception is thrown on bad date'; - close STDERR; - } + 'An exception is thrown on bad date'; + }, + qr{Incorrect datetime value: 'bad_date' for column .*returndate} + ); $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Account.t b/t/db_dependent/Koha/Account.t index 8c009aff5fd..f4235c98e11 100755 --- a/t/db_dependent/Koha/Account.t +++ b/t/db_dependent/Koha/Account.t @@ -23,6 +23,7 @@ use Test::NoWarnings; use Test::More tests => 16; use Test::MockModule; use Test::Exception; +use Test::Warn; use DateTime; @@ -326,7 +327,7 @@ subtest 'add_credit() tests' => sub { subtest 'add_debit() tests' => sub { - plan tests => 14; + plan tests => 15; $schema->storage->txn_begin; @@ -353,24 +354,26 @@ subtest 'add_debit() tests' => sub { } ); } 'Koha::Exceptions::Account::AmountNotPositive', 'Expected validation exception thrown (amount)'; - throws_ok { - local *STDERR; - open STDERR, '>', '/dev/null'; - $account->add_debit( - { - amount => 5, - description => 'type validation failure', - library_id => $patron->branchcode, - note => 'this should fail anyway', - type => 'failure', - user_id => $patron->id, - interface => 'commandline' + warning_like( + sub { + throws_ok { + $account->add_debit( + { + amount => 5, + description => 'type validation failure', + library_id => $patron->branchcode, + note => 'this should fail anyway', + type => 'failure', + user_id => $patron->id, + interface => 'commandline' + } + ); } - ); - close STDERR; - } - 'Koha::Exceptions::Account::UnrecognisedType', - 'Expected validation exception thrown (type)'; + 'Koha::Exceptions::Account::UnrecognisedType', + 'Expected validation exception thrown (type)'; + }, + qr{a foreign key constraint fails} + ); throws_ok { $account->add_debit( diff --git a/t/db_dependent/Koha/ApiKey.t b/t/db_dependent/Koha/ApiKey.t index 8e65b55b46e..93b489c0dfd 100755 --- a/t/db_dependent/Koha/ApiKey.t +++ b/t/db_dependent/Koha/ApiKey.t @@ -21,6 +21,7 @@ use Test::NoWarnings; use Test::More tests => 5; use Test::MockModule; use Test::Exception; +use Test::Warn; use t::lib::TestBuilder; @@ -33,7 +34,7 @@ my $builder = t::lib::TestBuilder->new; subtest 'store() tests' => sub { - plan tests => 16; + plan tests => 17; $schema->storage->txn_begin; @@ -98,18 +99,16 @@ subtest 'store() tests' => sub { my $deleted_id = $patron_to_delete->id; $patron_to_delete->delete; - { # hide useless warnings - local *STDERR; - open STDERR, '>', '/dev/null'; - throws_ok { - Koha::ApiKey->new( - { patron_id => $deleted_id, description => 'a description' } ) - ->store - } - 'Koha::Exceptions::Object::FKConstraint', - 'Invalid patron ID raises exception'; - close STDERR; - } + warning_like( + sub { + throws_ok { + Koha::ApiKey->new( { patron_id => $deleted_id, description => 'a description' } )->store + } + 'Koha::Exceptions::Object::FKConstraint', + 'Invalid patron ID raises exception'; + }, + qr{a foreign key constraint fails} + ); is( $@->message, 'Broken FK constraint', 'Exception message is correct' ); is( $@->broken_fk, 'patron_id', 'Exception field is correct' ); diff --git a/t/db_dependent/Koha/Checkouts/Renewal.t b/t/db_dependent/Koha/Checkouts/Renewal.t index 56b5e1c46ce..ae4540dcc45 100755 --- a/t/db_dependent/Koha/Checkouts/Renewal.t +++ b/t/db_dependent/Koha/Checkouts/Renewal.t @@ -85,31 +85,22 @@ subtest "store() tests" => sub { 'Renewal stored on the DB' ); - { # hide useless warnings - local *STDERR; - open STDERR, '>', '/dev/null'; - - my $another_checkout = - $builder->build_object( { class => 'Koha::Checkouts' } ); - my $checkout_id = $another_checkout->id; - $another_checkout->delete; - - my $THE_renewal; - - throws_ok { - $THE_renewal = Koha::Checkouts::Renewal->new( - { - checkout_id => $checkout_id, - interface => 'intranet' - } - )->store; - } - 'Koha::Exceptions::Object::FKConstraint', - 'An exception is thrown on invalid checkout_id'; - close STDERR; + my $another_checkout = $builder->build_object( { class => 'Koha::Checkouts' } ); + my $checkout_id = $another_checkout->id; + $another_checkout->delete; - is( $@->broken_fk, 'checkout_id', 'Exception field is correct' ); + throws_ok { + Koha::Checkouts::Renewal->new( + { + checkout_id => $checkout_id, + interface => 'intranet' + } + )->store; } + 'Koha::Exceptions::Object::FKConstraint', + 'An exception is thrown on invalid checkout_id'; + + is( $@->broken_fk, 'checkout_id', 'Exception field is correct' ); $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Checkouts/ReturnClaim.t b/t/db_dependent/Koha/Checkouts/ReturnClaim.t index 0635152c0b3..dc6f4acd643 100755 --- a/t/db_dependent/Koha/Checkouts/ReturnClaim.t +++ b/t/db_dependent/Koha/Checkouts/ReturnClaim.t @@ -20,6 +20,7 @@ use Modern::Perl; use Test::NoWarnings; use Test::More tests => 7; use Test::Exception; +use Test::Warn; use Koha::Database; use Koha::DateUtils qw( dt_from_string output_pref ); @@ -115,7 +116,7 @@ subtest "store() tests" => sub { subtest "resolve() tests" => sub { - plan tests => 10; + plan tests => 11; $schema->storage->txn_begin; @@ -159,17 +160,12 @@ subtest "resolve() tests" => sub { my $deleted_patron_id = $deleted_patron->id; $deleted_patron->delete; - { # hide useless warnings - local *STDERR; - open STDERR, '>', '/dev/null'; - + warning_like(sub{ throws_ok { $claim->resolve({ resolution => "X", resolved_by => $deleted_patron_id }) } 'Koha::Exceptions::Object::FKConstraint', "Exception thrown on invalid resolver"; - - close STDERR; - } + }, qr{a foreign key constraint fails}); my $today = dt_from_string; my $tomorrow = dt_from_string->add( days => 1 ); diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t index 66cdc28112b..21d72c21db7 100755 --- a/t/db_dependent/Koha/Object.t +++ b/t/db_dependent/Koha/Object.t @@ -946,7 +946,7 @@ subtest "Test update method" => sub { subtest 'store() tests' => sub { - plan tests => 16; + plan tests => 18; # Using Koha::Library::Groups to test Koha::Object>-store # Simple object with foreign keys and unique key @@ -966,49 +966,53 @@ subtest 'store() tests' => sub { ); my $dbh = $schema->storage->dbh; - { - local *STDERR; - open STDERR, '>', '/dev/null'; - throws_ok { $library_group->store } - 'Koha::Exceptions::Object::FKConstraint', - 'Exception is thrown correctly'; - is( - $@->message, - "Broken FK constraint", - 'Exception message is correct' - ); - is( - $@->broken_fk, - 'branchcode', - 'Exception field is correct' - ); - - $library_group = $builder->build_object( { class => 'Koha::Library::Groups' } ); + warning_like( + sub { + throws_ok { $library_group->store } + 'Koha::Exceptions::Object::FKConstraint', + 'Exception is thrown correctly'; + }, + qr{a foreign key constraint fails} + ); + is( + $@->message, + "Broken FK constraint", + 'Exception message is correct' + ); + is( + $@->broken_fk, + 'branchcode', + 'Exception field is correct' + ); - my $new_library_group = Koha::Library::Group->new( - { - branchcode => $library_group->branchcode, - title => $library_group->title, - } - ); + $library_group = $builder->build_object( { class => 'Koha::Library::Groups' } ); - throws_ok { $new_library_group->store } - 'Koha::Exceptions::Object::DuplicateID', - 'Exception is thrown correctly'; + my $new_library_group = Koha::Library::Group->new( + { + branchcode => $library_group->branchcode, + title => $library_group->title, + } + ); - is( - $@->message, - 'Duplicate ID', - 'Exception message is correct' - ); + warning_like( + sub { + throws_ok { $new_library_group->store } + 'Koha::Exceptions::Object::DuplicateID', + 'Exception is thrown correctly'; + }, + qr{Duplicate entry.* for key 'title'} + ); - like( - $@->duplicate_id, - qr/(library_groups\.)?title/, - 'Exception field is correct (note that MySQL 8 is displaying the tablename)' - ); - close STDERR; - } + is( + $@->message, + 'Duplicate ID', + 'Exception message is correct' + ); + like( + $@->duplicate_id, + qr/(library_groups\.)?title/, + 'Exception field is correct (note that MySQL 8 is displaying the tablename)' + ); # Successful test $library_group->set( { title => 'Manuel' } ); @@ -1065,21 +1069,23 @@ subtest 'store() tests' => sub { subtest 'Bad value tests' => sub { - plan tests => 3; + plan tests => 4; my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - try { - local *STDERR; - open STDERR, '>', '/dev/null'; - $patron->lastseen('wrong_value')->store; - close STDERR; - } catch { - ok( $_->isa('Koha::Exceptions::Object::BadValue'), 'Exception thrown correctly' ); - like( $_->property, qr/(borrowers\.)?lastseen/, 'Column should be the expected one' ) - ; # The table name is not always displayed, it depends on the DBMS version - is( $_->value, 'wrong_value', 'Value should be the expected one' ); - }; + warning_like( + sub { + try { + $patron->lastseen('wrong_value')->store; + } catch { + ok( $_->isa('Koha::Exceptions::Object::BadValue'), 'Exception thrown correctly' ); + like( $_->property, qr/(borrowers\.)?lastseen/, 'Column should be the expected one' ) + ; # The table name is not always displayed, it depends on the DBMS version + is( $_->value, 'wrong_value', 'Value should be the expected one' ); + }; + }, + qr{Incorrect datetime value: 'wrong_value' for column .*lastseen} + ); }; $schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t index 23acb9aa802..9ae864dc2f6 100755 --- a/t/db_dependent/Koha/Patron.t +++ b/t/db_dependent/Koha/Patron.t @@ -368,7 +368,7 @@ subtest 'is_active' => sub { subtest 'add_guarantor() tests' => sub { - plan tests => 6; + plan tests => 7; $schema->storage->txn_begin; @@ -397,15 +397,16 @@ subtest 'add_guarantor() tests' => sub { is( $guarantors->count, 1, 'No guarantors added' ); - { - local *STDERR; - open STDERR, '>', '/dev/null'; - throws_ok - { $patron_1->add_guarantor({ guarantor_id => $patron_2->borrowernumber, relationship => 'father2' }); } + warning_like( + sub { + throws_ok { + $patron_1->add_guarantor( { guarantor_id => $patron_2->borrowernumber, relationship => 'father2' } ); + } 'Koha::Exceptions::Patron::Relationship::DuplicateRelationship', - 'Exception is thrown for duplicated relationship'; - close STDERR; - } + 'Exception is thrown for duplicated relationship'; + }, + qr{Duplicate entry.* for key 'guarantor_guarantee_idx'} + ); $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Patron/Relationship.t b/t/db_dependent/Koha/Patron/Relationship.t index 8fd9768aafd..513371c8054 100755 --- a/t/db_dependent/Koha/Patron/Relationship.t +++ b/t/db_dependent/Koha/Patron/Relationship.t @@ -22,6 +22,7 @@ use Modern::Perl; use Test::NoWarnings; use Test::More tests => 2; use Test::Exception; +use Test::Warn; use Koha::Database; use Koha::Patrons; @@ -36,7 +37,7 @@ my $builder = t::lib::TestBuilder->new; subtest 'store() tests' => sub { - plan tests => 13; + plan tests => 14; $schema->storage->txn_begin; @@ -107,23 +108,24 @@ subtest 'store() tests' => sub { } ); - { - local *STDERR; - open STDERR, '>', '/dev/null'; - throws_ok - { $relationship_2->store; } + warning_like( + sub { + throws_ok { $relationship_2->store; } 'Koha::Exceptions::Patron::Relationship::DuplicateRelationship', - 'Exception is thrown for duplicated relationship'; - - is( "$@", - "There already exists a relationship for the same guarantor (" - . $patron_2->borrowernumber - . ") and guarantee (" - . $patron_1->borrowernumber - . ") combination", - 'Exception stringified correctly' - ); - } + 'Exception is thrown for duplicated relationship'; + }, + qr{Duplicate entry.* for key 'guarantor_guarantee_idx'} + ); + + is( + "$@", + "There already exists a relationship for the same guarantor (" + . $patron_2->borrowernumber + . ") and guarantee (" + . $patron_1->borrowernumber + . ") combination", + 'Exception stringified correctly' + ); t::lib::Mocks::mock_preference( 'borrowerRelationship', '' ); diff --git a/t/db_dependent/Koha/Suggestions.t b/t/db_dependent/Koha/Suggestions.t index 11516ee5709..46b91263cb2 100755 --- a/t/db_dependent/Koha/Suggestions.t +++ b/t/db_dependent/Koha/Suggestions.t @@ -22,6 +22,7 @@ use Modern::Perl; use Test::NoWarnings; use Test::More tests => 12; use Test::Exception; +use Test::Warn; use Koha::Suggestions; use Koha::Notice::Messages; @@ -133,12 +134,9 @@ is( Koha::Suggestions->search->count, $nb_of_suggestions + 1, 'Delete should hav $schema->storage->txn_rollback; subtest 'constraints' => sub { - plan tests => 11; + plan tests => 15; $schema->storage->txn_begin; - my $print_error = $schema->storage->dbh->{PrintError}; - $schema->storage->dbh->{PrintError} = 0; - my $patron = $builder->build_object( { class => "Koha::Patrons" } ); my $biblio = $builder->build_sample_biblio(); my $branch = $builder->build_object( { class => "Koha::Libraries" } ); @@ -178,16 +176,16 @@ subtest 'constraints' => sub { "The suggestion is not deleted when the related branch is deleted" ); # managerid - { # hide useless warnings - local *STDERR; - open STDERR, '>', '/dev/null'; - throws_ok { - $suggestion->managedby($nonexistent_borrowernumber)->store; - } - 'Koha::Exceptions::Object::FKConstraint', - 'store raises an exception on invalid managerid'; - close STDERR; - } + warning_like( + sub { + throws_ok { + $suggestion->managedby($nonexistent_borrowernumber)->store; + } + 'Koha::Exceptions::Object::FKConstraint', + 'store raises an exception on invalid managerid'; + }, + qr{a foreign key constraint fails} + ); my $manager = $builder->build_object( { class => "Koha::Patrons" } ); $suggestion->managedby( $manager->borrowernumber )->store; $manager->delete; @@ -196,16 +194,16 @@ subtest 'constraints' => sub { "The suggestion is not deleted when the related manager is deleted" ); # acceptedby - { # hide useless warnings - local *STDERR; - open STDERR, '>', '/dev/null'; - throws_ok { - $suggestion->acceptedby($nonexistent_borrowernumber)->store; - } - 'Koha::Exceptions::Object::FKConstraint', - 'store raises an exception on invalid acceptedby id'; - close STDERR; - } + warning_like( + sub { + throws_ok { + $suggestion->acceptedby($nonexistent_borrowernumber)->store; + } + 'Koha::Exceptions::Object::FKConstraint', + 'store raises an exception on invalid acceptedby id'; + }, + qr{a foreign key constraint fails} + ); my $acceptor = $builder->build_object( { class => "Koha::Patrons" } ); $suggestion->acceptedby( $acceptor->borrowernumber )->store; $acceptor->delete; @@ -214,16 +212,16 @@ subtest 'constraints' => sub { "The suggestion is not deleted when the related acceptor is deleted" ); # rejectedby - { # hide useless warnings - local *STDERR; - open STDERR, '>', '/dev/null'; - throws_ok { - $suggestion->rejectedby($nonexistent_borrowernumber)->store; - } - 'Koha::Exceptions::Object::FKConstraint', - 'store raises an exception on invalid rejectedby id'; - close STDERR; - } + warning_like( + sub { + throws_ok { + $suggestion->rejectedby($nonexistent_borrowernumber)->store; + } + 'Koha::Exceptions::Object::FKConstraint', + 'store raises an exception on invalid rejectedby id'; + }, + qr{a foreign key constraint fails} + ); my $rejecter = $builder->build_object( { class => "Koha::Patrons" } ); $suggestion->rejectedby( $rejecter->borrowernumber )->store; $rejecter->delete; @@ -232,15 +230,14 @@ subtest 'constraints' => sub { "The suggestion is not deleted when the related rejecter is deleted" ); # budgetid - { # hide useless warnings - local *STDERR; - open STDERR, '>', '/dev/null'; - - throws_ok { $suggestion->budgetid($nonexistent_borrowernumber)->store; } - 'Koha::Exceptions::Object::FKConstraint', - 'store raises an exception on invalid budgetid'; - close STDERR; - } + warning_like( + sub { + throws_ok { $suggestion->budgetid($nonexistent_borrowernumber)->store; } + 'Koha::Exceptions::Object::FKConstraint', + 'store raises an exception on invalid budgetid'; + }, + qr{a foreign key constraint fails} + ); my $fund = $builder->build_object( { class => "Koha::Acquisition::Funds" } ); $suggestion->budgetid( $fund->id )->store; $fund->delete; @@ -248,7 +245,6 @@ subtest 'constraints' => sub { is( $suggestion->budgetid, undef, "The suggestion is not deleted when the related budget is deleted" ); - $schema->storage->dbh->{PrintError} = $print_error; $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Suggestions.t b/t/db_dependent/Suggestions.t index 09ad198f51a..b73d826d411 100755 --- a/t/db_dependent/Suggestions.t +++ b/t/db_dependent/Suggestions.t @@ -19,7 +19,7 @@ use Modern::Perl; use DateTime::Duration; use Test::NoWarnings; -use Test::More tests => 95; +use Test::More tests => 96; use Test::Warn; use t::lib::Mocks; @@ -284,21 +284,17 @@ $status = ModSuggestion($mod_suggestion5); $messages = C4::Letters::GetQueuedMessages( { borrowernumber => $borrowernumber } ); is( @$messages, 3, 'ModSuggestions does send a message if the status has been changed' ); -{ - # Hiding the expected warning displayed by DBI - # DBD::mysql::st execute failed: Incorrect date value: 'invalid date!' for column 'manageddate' - local *STDERR; - open STDERR, '>', '/dev/null'; - $mod_suggestion4->{manageddate} = 'invalid date!'; - ModSuggestion($mod_suggestion4); - $messages = C4::Letters::GetQueuedMessages({ - borrowernumber => $borrowernumber2 - }); +$mod_suggestion4->{manageddate} = 'invalid date!'; +warning_like( + sub { + ModSuggestion($mod_suggestion4); + }, + qr{Incorrect date value: 'invalid date!' for column .*manageddate} +); +$messages = C4::Letters::GetQueuedMessages( { borrowernumber => $borrowernumber2 } ); - close STDERR; - is (scalar(@$messages), 1, 'No new letter should have been generated if the update raised an error'); -} +is( scalar(@$messages), 1, 'No new letter should have been generated if the update raised an error' ); is( GetSuggestionInfo(), undef, 'GetSuggestionInfo without the suggestion id returns undef' ); $suggestion = GetSuggestionInfo($my_suggestionid); diff --git a/t/db_dependent/api/v1/item_groups.t b/t/db_dependent/api/v1/item_groups.t index 6fbca6950a1..32039cc7d6d 100755 --- a/t/db_dependent/api/v1/item_groups.t +++ b/t/db_dependent/api/v1/item_groups.t @@ -112,13 +112,8 @@ subtest 'add() tests' => sub { ->status_is( 201, 'REST3.2.1' ); # Invalid biblio id - { # hide useless warnings - local *STDERR; - open STDERR, '>', '/dev/null'; - $t->post_ok( "//$auth_userid:$password@/api/v1/biblios/XXX/item_groups" => json => $item_group ) - ->status_is( 404 ); - close STDERR; - } + $t->post_ok( "//$auth_userid:$password@/api/v1/biblios/XXX/item_groups" => json => $item_group ) + ->status_is( 404 ); $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/selenium/01-installation.t b/t/db_dependent/selenium/01-installation.t index 73e7f501f50..521842ffb7d 100755 --- a/t/db_dependent/selenium/01-installation.t +++ b/t/db_dependent/selenium/01-installation.t @@ -22,7 +22,6 @@ use Modern::Perl; -use Test::NoWarnings; use Test::More tests => 3; use t::lib::Selenium; -- 2.34.1