From 3c7d4abac2e9bb34ca00da1984c6a049f1ffd954 Mon Sep 17 00:00:00 2001 From: Fridolin Somers Date: Wed, 15 Dec 2021 11:19:21 -1000 Subject: [PATCH] Bug 29705: In test suite merge IssuingRules with CirculationRules After Bug 18936, in test suite, there is still t/db_dependent/Koha/IssuingRules.t and t/db_dependent/Koha/IssuingRules/ This patch moves IssuingRules.t inside CirculationRules.t and renames dir t/db_dependent/Koha/IssuingRules. And adds DB transation in each first-level subtest. Also renames in : use constant GUESSED_ITEMTYPES_KEY => 'Koha_IssuingRules_last_guess'; Test plan : Run prove t/db_dependent/Koha/CirculationRules.t and prove t/db_dependent/Koha/CirculationRules/* Signed-off-by: Samu Heiskanen Signed-off-by: Jonathan Druart --- Koha/CirculationRules.pm | 2 +- t/db_dependent/Koha/CirculationRules.t | 375 +++++++++++++++++- .../guess_article_requestable_itemtypes.t | 0 t/db_dependent/Koha/IssuingRules.t | 343 ---------------- 4 files changed, 372 insertions(+), 348 deletions(-) rename t/db_dependent/Koha/{IssuingRules => CirculationRules}/guess_article_requestable_itemtypes.t (100%) diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index eb66f5f1a0e..29d197ed09e 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -25,7 +25,7 @@ use Koha::CirculationRule; use base qw(Koha::Objects); -use constant GUESSED_ITEMTYPES_KEY => 'Koha_IssuingRules_last_guess'; +use constant GUESSED_ITEMTYPES_KEY => 'Koha_CirculationRules_last_guess'; =head1 NAME diff --git a/t/db_dependent/Koha/CirculationRules.t b/t/db_dependent/Koha/CirculationRules.t index b621b50cfb5..7117484d20a 100755 --- a/t/db_dependent/Koha/CirculationRules.t +++ b/t/db_dependent/Koha/CirculationRules.t @@ -19,7 +19,9 @@ use Modern::Perl; -use Test::More tests => 4; +use Benchmark; +use Test::More tests => 7; +use Test::Deep qw( cmp_methods ); use Test::Exception; use Koha::CirculationRules; @@ -27,11 +29,368 @@ use Koha::Database; use t::lib::Mocks; use t::lib::TestBuilder; -use t::lib::Mocks; my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; +subtest 'get_effective_issuing_rule' => sub { + plan tests => 2; + + $schema->storage->txn_begin; + + my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; + my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; + my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; + + subtest 'Call with undefined values' => sub { + plan tests => 5; + + my $rule; + Koha::CirculationRules->delete; + + is(Koha::CirculationRules->search->count, 0, 'There are no issuing rules.'); + # undef, undef, undef => 1 + $rule = Koha::CirculationRules->get_effective_rule({ + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'fine', + rule_value => 1, + }); + is($rule, undef, 'When I attempt to get effective issuing rule by' + .' providing undefined values, then undef is returned.'); + + # undef, undef, undef => 2 + ok( + Koha::CirculationRule->new( + { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'fine', + rule_value => 2, + } + )->store, + 'Given I added an issuing rule branchcode => undef,' + .' categorycode => undef, itemtype => undef,'); + $rule = Koha::CirculationRules->get_effective_rule({ + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'fine', + }); + _is_row_match( + $rule, + { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'fine', + rule_value => 2, + }, + 'When I attempt to get effective' + .' issuing rule by providing undefined values, then the above one is' + .' returned.' + ); + }; + + subtest 'Performance' => sub { + plan tests => 4; + + my $worst_case = timethis(500, + sub { Koha::CirculationRules->get_effective_rule({ + branchcode => 'nonexistent', + categorycode => 'nonexistent', + itemtype => 'nonexistent', + rule_name => 'nonexistent', + }); + } + ); + my $mid_case = timethis(500, + sub { Koha::CirculationRules->get_effective_rule({ + branchcode => $branchcode, + categorycode => 'nonexistent', + itemtype => 'nonexistent', + rule_name => 'nonexistent', + }); + } + ); + my $sec_best_case = timethis(500, + sub { Koha::CirculationRules->get_effective_rule({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => 'nonexistent', + rule_name => 'nonexistent', + }); + } + ); + my $best_case = timethis(500, + sub { Koha::CirculationRules->get_effective_rule({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'nonexistent', + }); + } + ); + ok($worst_case, 'In worst case, get_effective_issuing_rule finds matching' + .' rule '.sprintf('%.2f', $worst_case->iters/$worst_case->cpu_a) + .' times per second.'); + ok($mid_case, 'In mid case, get_effective_issuing_rule finds matching' + .' rule '.sprintf('%.2f', $mid_case->iters/$mid_case->cpu_a) + .' times per second.'); + ok($sec_best_case, 'In second best case, get_effective_issuing_rule finds matching' + .' rule '.sprintf('%.2f', $sec_best_case->iters/$sec_best_case->cpu_a) + .' times per second.'); + ok($best_case, 'In best case, get_effective_issuing_rule finds matching' + .' rule '.sprintf('%.2f', $best_case->iters/$best_case->cpu_a) + .' times per second.'); + }; + + $schema->storage->txn_rollback; + +}; + +subtest 'set_rule' => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; + my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; + my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; + + subtest 'Correct call' => sub { + plan tests => 4; + + Koha::CirculationRules->delete; + + lives_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + rule_name => 'lostreturn', + rule_value => '', + } ); + }, 'setting lostreturn with branch' ); + + lives_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + categorycode => $categorycode, + rule_name => 'patron_maxissueqty', + rule_value => '', + } ); + }, 'setting patron_maxissueqty with branch/category succeeds' ); + + lives_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + itemtype => $itemtype, + rule_name => 'holdallowed', + rule_value => '', + } ); + }, 'setting holdallowed with branch/itemtype succeeds' ); + + lives_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'fine', + rule_value => '', + } ); + }, 'setting fine with branch/category/itemtype succeeds' ); + }; + + subtest 'Call with missing params' => sub { + plan tests => 4; + + Koha::CirculationRules->delete; + + throws_ok( sub { + Koha::CirculationRules->set_rule( { + rule_name => 'lostreturn', + rule_value => '', + } ); + }, qr/branchcode/, 'setting lostreturn without branch fails' ); + + throws_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + rule_name => 'patron_maxissueqty', + rule_value => '', + } ); + }, qr/categorycode/, 'setting patron_maxissueqty without categorycode fails' ); + + throws_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + rule_name => 'holdallowed', + rule_value => '', + } ); + }, qr/itemtype/, 'setting holdallowed without itemtype fails' ); + + throws_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + categorycode => $categorycode, + rule_name => 'fine', + rule_value => '', + } ); + }, qr/itemtype/, 'setting fine without itemtype fails' ); + }; + + subtest 'Call with extra params' => sub { + plan tests => 3; + + Koha::CirculationRules->delete; + + throws_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + categorycode => $categorycode, + rule_name => 'lostreturn', + rule_value => '', + } ); + }, qr/categorycode/, 'setting lostreturn with categorycode fails' ); + + throws_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'patron_maxissueqty', + rule_value => '', + } ); + }, qr/itemtype/, 'setting patron_maxissueqty with itemtype fails' ); + + throws_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + rule_name => 'holdallowed', + categorycode => $categorycode, + itemtype => $itemtype, + rule_value => '', + } ); + }, qr/categorycode/, 'setting holdallowed with categorycode fails' ); + }; + + $schema->storage->txn_rollback; +}; + +subtest 'clone' => sub { + plan tests => 2; + + $schema->storage->txn_begin; + + my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; + my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; + my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; + + subtest 'Clone multiple rules' => sub { + plan tests => 4; + + Koha::CirculationRules->delete; + + Koha::CirculationRule->new({ + branchcode => undef, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'fine', + rule_value => 5, + })->store; + + Koha::CirculationRule->new({ + branchcode => undef, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'lengthunit', + rule_value => 'days', + })->store; + + Koha::CirculationRules->search({ branchcode => undef })->clone($branchcode); + + my $rule_fine = Koha::CirculationRules->get_effective_rule({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'fine', + }); + my $rule_lengthunit = Koha::CirculationRules->get_effective_rule({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'lengthunit', + }); + + _is_row_match( + $rule_fine, + { + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'fine', + rule_value => 5, + }, + 'When I attempt to get cloned fine rule,' + .' then the above one is returned.' + ); + _is_row_match( + $rule_lengthunit, + { + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'lengthunit', + rule_value => 'days', + }, + 'When I attempt to get cloned lengthunit rule,' + .' then the above one is returned.' + ); + + }; + + subtest 'Clone one rule' => sub { + plan tests => 2; + + Koha::CirculationRules->delete; + + Koha::CirculationRule->new({ + branchcode => undef, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'fine', + rule_value => 5, + })->store; + + my $rule = Koha::CirculationRules->search({ branchcode => undef })->next; + $rule->clone($branchcode); + + my $cloned_rule = Koha::CirculationRules->get_effective_rule({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'fine', + }); + + _is_row_match( + $cloned_rule, + { + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => 'fine', + rule_value => '5', + }, + 'When I attempt to get cloned fine rule,' + .' then the above one is returned.' + ); + + }; + + $schema->storage->txn_rollback; +}; + subtest 'set_rule + get_effective_rule' => sub { plan tests => 9; @@ -128,7 +487,7 @@ subtest 'set_rule + get_effective_rule' => sub { subtest 'test rule matching with different combinations of rule scopes' => sub { - my ( $tests, $order ) = prepare_tests_for_rule_scope_combinations( + my ( $tests, $order ) = _prepare_tests_for_rule_scope_combinations( { branchcode => $branchcode, categorycode => $categorycode, @@ -442,7 +801,15 @@ subtest 'get_lostreturn_policy() tests' => sub { $schema->storage->txn_rollback; }; -sub prepare_tests_for_rule_scope_combinations { +sub _is_row_match { + my ( $rule, $expected, $message ) = @_; + + ok( $rule, $message ) ? + cmp_methods( $rule, [ %$expected ], $message ) : + fail( $message ); +} + +sub _prepare_tests_for_rule_scope_combinations { my ( $scope, $rule_name ) = @_; # Here we create a combinations of 1s and 0s the following way diff --git a/t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t b/t/db_dependent/Koha/CirculationRules/guess_article_requestable_itemtypes.t similarity index 100% rename from t/db_dependent/Koha/IssuingRules/guess_article_requestable_itemtypes.t rename to t/db_dependent/Koha/CirculationRules/guess_article_requestable_itemtypes.t diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t index afcb2867b7c..4d773075178 100755 --- a/t/db_dependent/Koha/IssuingRules.t +++ b/t/db_dependent/Koha/IssuingRules.t @@ -35,350 +35,7 @@ $schema->storage->txn_begin; my $builder = t::lib::TestBuilder->new; -subtest 'get_effective_issuing_rule' => sub { - plan tests => 2; - my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; - my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; - my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; - - subtest 'Call with undefined values' => sub { - plan tests => 5; - - my $rule; - Koha::CirculationRules->delete; - - is(Koha::CirculationRules->search->count, 0, 'There are no issuing rules.'); - # undef, undef, undef => 1 - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => undef, - categorycode => undef, - itemtype => undef, - rule_name => 'fine', - rule_value => 1, - }); - is($rule, undef, 'When I attempt to get effective issuing rule by' - .' providing undefined values, then undef is returned.'); - - # undef, undef, undef => 2 - ok( - Koha::CirculationRule->new( - { - branchcode => undef, - categorycode => undef, - itemtype => undef, - rule_name => 'fine', - rule_value => 2, - } - )->store, - 'Given I added an issuing rule branchcode => undef,' - .' categorycode => undef, itemtype => undef,'); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => undef, - categorycode => undef, - itemtype => undef, - rule_name => 'fine', - }); - _is_row_match( - $rule, - { - branchcode => undef, - categorycode => undef, - itemtype => undef, - rule_name => 'fine', - rule_value => 2, - }, - 'When I attempt to get effective' - .' issuing rule by providing undefined values, then the above one is' - .' returned.' - ); - }; - - subtest 'Performance' => sub { - plan tests => 4; - - my $worst_case = timethis(500, - sub { Koha::CirculationRules->get_effective_rule({ - branchcode => 'nonexistent', - categorycode => 'nonexistent', - itemtype => 'nonexistent', - rule_name => 'nonexistent', - }); - } - ); - my $mid_case = timethis(500, - sub { Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => 'nonexistent', - itemtype => 'nonexistent', - rule_name => 'nonexistent', - }); - } - ); - my $sec_best_case = timethis(500, - sub { Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => 'nonexistent', - rule_name => 'nonexistent', - }); - } - ); - my $best_case = timethis(500, - sub { Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'nonexistent', - }); - } - ); - ok($worst_case, 'In worst case, get_effective_issuing_rule finds matching' - .' rule '.sprintf('%.2f', $worst_case->iters/$worst_case->cpu_a) - .' times per second.'); - ok($mid_case, 'In mid case, get_effective_issuing_rule finds matching' - .' rule '.sprintf('%.2f', $mid_case->iters/$mid_case->cpu_a) - .' times per second.'); - ok($sec_best_case, 'In second best case, get_effective_issuing_rule finds matching' - .' rule '.sprintf('%.2f', $sec_best_case->iters/$sec_best_case->cpu_a) - .' times per second.'); - ok($best_case, 'In best case, get_effective_issuing_rule finds matching' - .' rule '.sprintf('%.2f', $best_case->iters/$best_case->cpu_a) - .' times per second.'); - }; -}; - -subtest 'set_rule' => sub { - plan tests => 3; - - my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; - my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; - my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; - - subtest 'Correct call' => sub { - plan tests => 4; - - Koha::CirculationRules->delete; - - lives_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - rule_name => 'lostreturn', - rule_value => '', - } ); - }, 'setting lostreturn with branch' ); - - lives_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - categorycode => $categorycode, - rule_name => 'patron_maxissueqty', - rule_value => '', - } ); - }, 'setting patron_maxissueqty with branch/category succeeds' ); - - lives_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - itemtype => $itemtype, - rule_name => 'holdallowed', - rule_value => '', - } ); - }, 'setting holdallowed with branch/itemtype succeeds' ); - - lives_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => '', - } ); - }, 'setting fine with branch/category/itemtype succeeds' ); - }; - - subtest 'Call with missing params' => sub { - plan tests => 4; - - Koha::CirculationRules->delete; - - throws_ok( sub { - Koha::CirculationRules->set_rule( { - rule_name => 'lostreturn', - rule_value => '', - } ); - }, qr/branchcode/, 'setting lostreturn without branch fails' ); - - throws_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - rule_name => 'patron_maxissueqty', - rule_value => '', - } ); - }, qr/categorycode/, 'setting patron_maxissueqty without categorycode fails' ); - - throws_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - rule_name => 'holdallowed', - rule_value => '', - } ); - }, qr/itemtype/, 'setting holdallowed without itemtype fails' ); - - throws_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - categorycode => $categorycode, - rule_name => 'fine', - rule_value => '', - } ); - }, qr/itemtype/, 'setting fine without itemtype fails' ); - }; - - subtest 'Call with extra params' => sub { - plan tests => 3; - - Koha::CirculationRules->delete; - - throws_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - categorycode => $categorycode, - rule_name => 'lostreturn', - rule_value => '', - } ); - }, qr/categorycode/, 'setting lostreturn with categorycode fails' ); - - throws_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'patron_maxissueqty', - rule_value => '', - } ); - }, qr/itemtype/, 'setting patron_maxissueqty with itemtype fails' ); - - throws_ok( sub { - Koha::CirculationRules->set_rule( { - branchcode => $branchcode, - rule_name => 'holdallowed', - categorycode => $categorycode, - itemtype => $itemtype, - rule_value => '', - } ); - }, qr/categorycode/, 'setting holdallowed with categorycode fails' ); - }; -}; - -subtest 'clone' => sub { - plan tests => 2; - - my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; - my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; - my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; - - subtest 'Clone multiple rules' => sub { - plan tests => 4; - - Koha::CirculationRules->delete; - - Koha::CirculationRule->new({ - branchcode => undef, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 5, - })->store; - - Koha::CirculationRule->new({ - branchcode => undef, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'lengthunit', - rule_value => 'days', - })->store; - - Koha::CirculationRules->search({ branchcode => undef })->clone($branchcode); - - my $rule_fine = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - my $rule_lengthunit = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'lengthunit', - }); - - _is_row_match( - $rule_fine, - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 5, - }, - 'When I attempt to get cloned fine rule,' - .' then the above one is returned.' - ); - _is_row_match( - $rule_lengthunit, - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'lengthunit', - rule_value => 'days', - }, - 'When I attempt to get cloned lengthunit rule,' - .' then the above one is returned.' - ); - - }; - - subtest 'Clone one rule' => sub { - plan tests => 2; - - Koha::CirculationRules->delete; - - Koha::CirculationRule->new({ - branchcode => undef, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 5, - })->store; - - my $rule = Koha::CirculationRules->search({ branchcode => undef })->next; - $rule->clone($branchcode); - - my $cloned_rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - - _is_row_match( - $cloned_rule, - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => '5', - }, - 'When I attempt to get cloned fine rule,' - .' then the above one is returned.' - ); - - }; -}; sub _is_row_match { my ( $rule, $expected, $message ) = @_; -- 2.25.1