gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 42391: (follow-up) Add missing warning assertions for Object::store 

The previous follow-up restored warn in Object::store but missed
several test files that trigger DB exceptions via ->store(). These
tests now fail Test::NoWarnings because the warning is emitted but
not expected.

This patch adds warning_like wrappers to 15 additional test files,
asserting that the expected DB exception warning is emitted.

Test plan:
1. Apply patch
2. Run:
   $ ktd --shell
   k$ prove t/db_dependent/api/v1/biblios.t \
       t/db_dependent/Koha/Holds.t \
       t/db_dependent/api/v1/return_claims.t \
       t/db_dependent/api/v1/libraries.t \
       t/db_dependent/Koha/Booking.t \
       t/db_dependent/api/v1/patrons.t \
       t/db_dependent/SIP/Message.t \
       t/db_dependent/Koha/Patron.t \
       t/db_dependent/Koha/Acquisition/Fund.t \
       t/db_dependent/api/v1/acquisitions_orders.t \
       t/db_dependent/Koha/ILL/Requests.t \
       t/db_dependent/Koha/Patron/Relationship.t \
       t/db_dependent/Koha/Hold.t \
       t/db_dependent/Circulation/Returns.t \
       t/db_dependent/api/v1/bookings.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Tomás Cohen Arazi 2026-07-21 11:02:29 -03:00 committed by Pedro Amorim
parent 4189d42ee6
commit 86d54c2c4a
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
15 changed files with 144 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
t/db_dependent/Circulation/Returns.t
View File

@ -305,11 +305,11 @@ subtest 'Handle ids duplication' => sub {
my $old_checkout = Koha::Old::Checkouts->find($issue_id);
my ( $doreturn, $messages, $new_checkout, $borrower );
warning_like {
warnings_like {
( $doreturn, $messages, $new_checkout, $borrower ) =
AddReturn( $item->barcode, undef, undef, undef, dt_from_string );
}
{ carped => qr{The checkin for the following issue failed.*Duplicate ID.*} },
[ qr/Duplicate ID/, { carped => qr{The checkin for the following issue failed.*Duplicate ID.*} } ],
'AddReturn should have carped about the duplicate id';
is( $doreturn, 0, 'Return should not have been done' );

11
t/db_dependent/Koha/Acquisition/Fund.t
View File

@ -61,7 +61,7 @@ subtest 'budget ()' => sub {
};
subtest 'budget' => sub {
plan tests => 2;
plan tests => 3;
$schema->storage->txn_begin;
my $f = $builder->build_object(
@ -78,9 +78,12 @@ subtest 'budget' => sub {
);
# Cannot set aqbudgets.budget_period_id as NULL
throws_ok { $fund->budget_period_id(undef)->store }
'Koha::Exceptions::Object::NotNull',
'Storing a fund with NULL budget_period_id should throw a NotNull exception';
warning_like {
throws_ok { $fund->budget_period_id(undef)->store }
'Koha::Exceptions::Object::NotNull',
'Storing a fund with NULL budget_period_id should throw a NotNull exception';
}
qr/required column value/, 'Warning from store on NotNull exception';
$schema->storage->txn_rollback;
};

22
t/db_dependent/Koha/Booking.t
View File

@ -457,7 +457,7 @@ subtest 'store() tests' => sub {
};
subtest 'confirmation notice trigger' => sub {
plan tests => 3;
plan tests => 4;
# FIXME: This is a bandaid solution to prevent test failures when running
# the Koha_Main_My8 job because notices are not added at upgrade time.
@ -500,11 +500,14 @@ subtest 'store() tests' => sub {
)->count;
# Reuse previous booking to produce a clash
throws_ok {
Koha::Booking->new( $booking->unblessed )->store
warning_like {
throws_ok {
Koha::Booking->new( $booking->unblessed )->store
}
'Koha::Exceptions::Object::DuplicateID',
'Exception is thrown correctly';
}
'Koha::Exceptions::Object::DuplicateID',
'Exception is thrown correctly';
qr/Duplicate ID/, 'Warning from store on DuplicateID exception';
my $post_notices_count = Koha::Notice::Messages->search(
{
@ -704,12 +707,15 @@ subtest 'store() tests' => sub {
};
subtest 'status change exception' => sub {
plan tests => 2;
plan tests => 3;
$booking->discard_changes;
my $status = $booking->status;
throws_ok { $booking->update( { status => 'blah' } ) } 'Koha::Exceptions::Object::BadValue',
'Throws exception when passed booking status would fail enum constraint';
warning_like {
throws_ok { $booking->update( { status => 'blah' } ) } 'Koha::Exceptions::Object::BadValue',
'Throws exception when passed booking status would fail enum constraint';
}
qr/Invalid value/, 'Warning from store on BadValue exception';
# Status unchanged
$booking->discard_changes;

12
t/db_dependent/Koha/Hold.t
View File

@ -24,6 +24,7 @@ use Test::More tests => 24;
use Test::Exception;
use Test::MockModule;
use Test::Warn;
use t::lib::Mocks;
use t::lib::TestBuilder;
@ -76,7 +77,7 @@ subtest 'store() tests' => sub {
subtest 'biblio() tests' => sub {
plan tests => 1;
plan tests => 2;
$schema->storage->txn_begin;
@ -86,9 +87,12 @@ subtest 'biblio() tests' => sub {
}
);
throws_ok { $hold->biblionumber(undef)->store; }
'Koha::Exceptions::Object::NotNull',
'reserves.biblionumber cannot be null, exception thrown';
warning_like {
throws_ok { $hold->biblionumber(undef)->store; }
'Koha::Exceptions::Object::NotNull',
'reserves.biblionumber cannot be null, exception thrown';
}
qr/required column value/, 'Warning from store on NotNull exception';
$schema->storage->txn_rollback;
};

22
t/db_dependent/Koha/Holds.t
View File

@ -42,7 +42,7 @@ $schema->storage->txn_begin;
my $builder = t::lib::TestBuilder->new;
subtest 'DB constraints' => sub {
plan tests => 1;
plan tests => 2;
my $patron = $builder->build_object( { class => 'Koha::Patrons' } );
my $item = $builder->build_sample_item;
@ -58,9 +58,12 @@ subtest 'DB constraints' => sub {
my $reserve_id = C4::Reserves::AddReserve($hold_info);
my $hold = Koha::Holds->find($reserve_id);
throws_ok { $hold->priority(undef)->store }
'Koha::Exceptions::Object::NotNull',
'Storing a hold with NULL priority should throw a NotNull exception';
warning_like {
throws_ok { $hold->priority(undef)->store }
'Koha::Exceptions::Object::NotNull',
'Storing a hold with NULL priority should throw a NotNull exception';
}
qr/required column value/, 'Warning from store on NotNull exception';
};
subtest 'cancel' => sub {
@ -481,7 +484,7 @@ subtest 'cancel' => sub {
};
subtest 'rollback' => sub {
plan tests => 3;
plan tests => 4;
my $patron_category = $builder->build_object(
{
class => 'Koha::Patron::Categories',
@ -509,9 +512,12 @@ subtest 'cancel' => sub {
# Add a row with the same id to make the cancel fails
Koha::Old::Hold->new( $hold->unblessed )->store;
throws_ok { $hold->cancel( { charge_cancel_fee => 1 } ) }
'Koha::Exceptions::Object::DuplicateID',
'Cancelling a hold whose id already exists in old_reserves should throw a DuplicateID exception';
warning_like {
throws_ok { $hold->cancel( { charge_cancel_fee => 1 } ) }
'Koha::Exceptions::Object::DuplicateID',
'Cancelling a hold whose id already exists in old_reserves should throw a DuplicateID exception';
}
qr/Duplicate ID/, 'Warning from store on DuplicateID exception';
$hold = Koha::Holds->find($reserve_id);
is( ref($hold), 'Koha::Hold', 'The hold should not have been deleted' );

20
t/db_dependent/Koha/ILL/Requests.t
View File

@ -1596,7 +1596,7 @@ subtest 'Censorship' => sub {
subtest 'Checking out' => sub {
plan tests => 16;
plan tests => 17;
$schema->storage->txn_begin;
@ -1667,13 +1667,17 @@ subtest 'Checking out' => sub {
# First we pass bad parameters to the item creation to test we're
# catching the failure of item creation
my $form_stage_bad_branchcode = $request->check_out(
{
stage => 'form',
item_type => $itemtype->itemtype,
branchcode => '---'
}
);
my $form_stage_bad_branchcode;
warning_like {
$form_stage_bad_branchcode = $request->check_out(
{
stage => 'form',
item_type => $itemtype->itemtype,
branchcode => '---'
}
);
}
qr/Broken FK constraint/, 'Warning from store on FKConstraint exception';
is_deeply(
$form_stage_bad_branchcode->{value}->{errors}, {

13
t/db_dependent/Koha/Patron.t
View File

@ -377,7 +377,7 @@ subtest 'is_active' => sub {
subtest 'add_guarantor() tests' => sub {
plan tests => 6;
plan tests => 7;
$schema->storage->txn_begin;
@ -404,11 +404,14 @@ subtest 'add_guarantor() tests' => sub {
is( $guarantors->count, 1, 'No guarantors added' );
throws_ok {
$patron_1->add_guarantor( { guarantor_id => $patron_2->borrowernumber, relationship => 'father2' } );
warning_like {
throws_ok {
$patron_1->add_guarantor( { guarantor_id => $patron_2->borrowernumber, relationship => 'father2' } );
}
'Koha::Exceptions::Patron::Relationship::DuplicateRelationship',
'Exception is thrown for duplicated relationship';
}
'Koha::Exceptions::Patron::Relationship::DuplicateRelationship',
'Exception is thrown for duplicated relationship';
qr/Duplicate ID/, 'Warning from store on DuplicateID exception';
$schema->storage->txn_rollback;
};

11
t/db_dependent/Koha/Patron/Relationship.t
View File

@ -37,7 +37,7 @@ my $builder = t::lib::TestBuilder->new;
subtest 'store() tests' => sub {
plan tests => 13;
plan tests => 14;
$schema->storage->txn_begin;
@ -111,9 +111,12 @@ subtest 'store() tests' => sub {
}
);
throws_ok { $relationship_2->store; }
'Koha::Exceptions::Patron::Relationship::DuplicateRelationship',
'Exception is thrown for duplicated relationship';
warning_like {
throws_ok { $relationship_2->store; }
'Koha::Exceptions::Patron::Relationship::DuplicateRelationship',
'Exception is thrown for duplicated relationship';
}
qr/Duplicate ID/, 'Warning from store on DuplicateID exception';
is(
"$@",

4
t/db_dependent/SIP/Message.t
View File

@ -1577,8 +1577,8 @@ sub test_checkin_v2 {
Koha::Old::Checkout->new( { issue_id => $issue->issue_id } )->store;
undef $response;
$msg = C4::SIP::Sip::MsgType->new( $siprequest, 0 );
warning_like { $msg->handle_checkin($server); }
qr/data issues/,
warnings_like { $msg->handle_checkin($server); }
[ qr/Duplicate ID/, qr/data issues/ ],
'data issues warning is emitted on duplicate issue_id in old_issues';
is( substr( $response, 2, 1 ), '0', 'OK flag is false when we encounter data corruption in old_issues' );
is( substr( $response, 5, 1 ), 'Y', 'Alert flag is set' );

13
t/db_dependent/api/v1/acquisitions_orders.t
View File

@ -292,7 +292,7 @@ subtest 'get() tests' => sub {
subtest 'add() tests' => sub {
plan tests => 16;
plan tests => 17;
$schema->storage->txn_begin;
@ -371,10 +371,13 @@ subtest 'add() tests' => sub {
# Authorized attempt to create with existing id
$order->{order_id} = $order_id;
$t->post_ok( "//$auth_userid:$password@/api/v1/acquisitions/orders" => json => $order )
->status_is(409)
->json_has( '/error' => "Fails when trying to add an existing order_id" )
->json_like( '/conflict' => qr/(aqorders\.)?PRIMARY/ );
warning_like {
$t->post_ok( "//$auth_userid:$password@/api/v1/acquisitions/orders" => json => $order )
->status_is(409)
->json_has( '/error' => "Fails when trying to add an existing order_id" )
->json_like( '/conflict' => qr/(aqorders\.)?PRIMARY/ );
}
qr/Duplicate ID/, 'Warning from store on DuplicateID exception';
$schema->storage->txn_rollback;
};

30
t/db_dependent/api/v1/biblios.t
View File

@ -2169,7 +2169,7 @@ subtest 'list() tests' => sub {
subtest 'add_item() tests' => sub {
plan tests => 9;
plan tests => 10;
$schema->storage->txn_begin;
@ -2219,17 +2219,20 @@ subtest 'add_item() tests' => sub {
my $item = $builder->build_sample_item();
$t->post_ok(
"//$userid:$password@/api/v1/biblios/$biblio_id/items" => json => {
external_id => $item->barcode,
}
)->status_is( 409, 'Duplicate barcode' )->json_is( "/error" => "Duplicate barcode." );
warning_like {
$t->post_ok(
"//$userid:$password@/api/v1/biblios/$biblio_id/items" => json => {
external_id => $item->barcode,
}
)->status_is( 409, 'Duplicate barcode' )->json_is( "/error" => "Duplicate barcode." );
}
qr/Duplicate ID/;
$schema->storage->txn_rollback;
};
subtest 'update_item() tests' => sub {
plan tests => 8;
plan tests => 9;
$schema->storage->txn_begin;
@ -2273,11 +2276,14 @@ subtest 'update_item() tests' => sub {
my $other_item = $builder->build_sample_item();
$t->put_ok(
"//$userid:$password@/api/v1/biblios/$biblio_id/items/$item_id" => json => {
external_id => $other_item->barcode,
}
)->status_is( 409, 'Barcode not unique' )->json_is( "/error" => "Duplicate barcode." );
warning_like {
$t->put_ok(
"//$userid:$password@/api/v1/biblios/$biblio_id/items/$item_id" => json => {
external_id => $other_item->barcode,
}
)->status_is( 409, 'Barcode not unique' )->json_is( "/error" => "Duplicate barcode." );
}
qr/Duplicate ID/;
$t->put_ok(
"//$userid:$password@/api/v1/biblios/$biblio_id/items/$item_id" => json => {

11
t/db_dependent/api/v1/bookings.t
View File

@ -196,7 +196,7 @@ subtest 'get() tests' => sub {
subtest 'add() tests' => sub {
plan tests => 15;
plan tests => 16;
$schema->storage->txn_begin;
@ -277,9 +277,12 @@ subtest 'add() tests' => sub {
$booking->{booking_id} = $booking_id;
$booking->{start_date} = output_pref( { dateformat => "rfc3339", dt => dt_from_string->add( days => 10 ) } );
$booking->{end_date} = output_pref( { dateformat => "rfc3339", dt => dt_from_string->add( days => 14 ) } );
$t->post_ok( "//$userid:$password@/api/v1/bookings" => json => $booking )
->status_is(409)
->json_is( "/error" => "Duplicate booking_id" );
warning_like {
$t->post_ok( "//$userid:$password@/api/v1/bookings" => json => $booking )
->status_is(409)
->json_is( "/error" => "Duplicate booking_id" );
}
qr/Duplicate ID/;
# TODO: Test bookings clashes
# TODO: Test item auto-assignment

13
t/db_dependent/api/v1/libraries.t
View File

@ -160,7 +160,7 @@ subtest 'get() tests' => sub {
subtest 'add() tests' => sub {
plan tests => 16;
plan tests => 17;
$schema->storage->txn_begin;
@ -224,10 +224,13 @@ subtest 'add() tests' => sub {
# Authorized attempt to create with existing id
$library->{library_id} = $library_id;
$t->post_ok( "//$auth_userid:$password@/api/v1/libraries" => json => $library )
->status_is(409)
->json_has( '/error' => "Fails when trying to add an existing library_id" )
->json_like( '/conflict' => qr/(branches\.)?PRIMARY/ );
warning_like {
$t->post_ok( "//$auth_userid:$password@/api/v1/libraries" => json => $library )
->status_is(409)
->json_has( '/error' => "Fails when trying to add an existing library_id" )
->json_like( '/conflict' => qr/(branches\.)?PRIMARY/ );
}
qr/Duplicate ID/;
$schema->storage->txn_rollback;
};

25
t/db_dependent/api/v1/patrons.t
View File

@ -451,7 +451,7 @@ subtest 'add() tests' => sub {
$schema->storage->txn_rollback;
subtest 'librarian access tests' => sub {
plan tests => 40;
plan tests => 41;
$schema->storage->txn_begin;
@ -511,10 +511,14 @@ subtest 'add() tests' => sub {
# Test duplicate cardnumber constraint
$newpatron->{userid} = undef; # force regeneration
$t->post_ok( "//$userid:$password@/api/v1/patrons" => { 'x-confirm-not-duplicate' => 1 } => json => $newpatron )
->status_is(409)
->json_has( '/error', 'Fails when trying to POST duplicate cardnumber' )
->json_like( '/conflict' => qr/(borrowers\.)?cardnumber/ );
warning_like {
$t->post_ok(
"//$userid:$password@/api/v1/patrons" => { 'x-confirm-not-duplicate' => 1 } => json => $newpatron )
->status_is(409)
->json_has( '/error', 'Fails when trying to POST duplicate cardnumber' )
->json_like( '/conflict' => qr/(borrowers\.)?cardnumber/ );
}
qr/Duplicate ID/;
# Create a library just to make sure its ID doesn't exist on the DB
my $category_to_delete = $builder->build_object( { class => 'Koha::Patron::Categories' } );
@ -1004,7 +1008,7 @@ subtest 'update() tests' => sub {
subtest 'librarian access tests' => sub {
plan tests => 43;
plan tests => 44;
$schema->storage->txn_begin;
@ -1092,9 +1096,12 @@ subtest 'update() tests' => sub {
# Use an invalid library_id
$newpatron->{library_id} = $deleted_library_id;
$t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron )
->status_is(400)
->json_is( '/error' => "Given library_id does not exist" );
warning_like {
$t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron )
->status_is(400)
->json_is( '/error' => "Given library_id does not exist" );
}
qr/Broken FK constraint/, 'DB exception logged for invalid library_id';
# Restore the valid library_id
$newpatron->{library_id} = $patron_2->branchcode;

23
t/db_dependent/api/v1/return_claims.t
View File

@ -39,7 +39,7 @@ my $t = Test::Mojo->new('Koha::REST::V1');
subtest 'claim_returned() tests' => sub {
plan tests => 8;
plan tests => 9;
$schema->storage->txn_begin;
@ -85,15 +85,18 @@ subtest 'claim_returned() tests' => sub {
my $claim_id = $t->tx->res->json->{claim_id};
## Duplicate id
$t->post_ok(
"//$userid:$password@/api/v1/return_claims" => json => {
item_id => $item->itemnumber,
charge_lost_fee => Mojo::JSON->false,
refund_lost_fee => Mojo::JSON->false,
created_by => $librarian->id,
notes => "This is a test note."
}
)->status_is(409);
warning_like {
$t->post_ok(
"//$userid:$password@/api/v1/return_claims" => json => {
item_id => $item->itemnumber,
charge_lost_fee => Mojo::JSON->false,
refund_lost_fee => Mojo::JSON->false,
created_by => $librarian->id,
notes => "This is a test note."
}
)->status_is(409);
}
qr/Duplicate ID/;
$issue->delete;