From 5b61408eb736d8a795eaeb9f7b44583d79e8b66a Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Sat, 4 Apr 2020 15:01:02 +0000 Subject: [PATCH] Bug 25113: Refactor CirculationRules.t when testing scope combinations MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit We used to test rule scopes by explicitly defining each combination. When adding new scopes, it is much easier if these tests are auto- generated for you so that you don't have to repeat similar code. This patch removes those "duplicates" and adds a method that returns test cases for each scope as follows: branchcode categorycode itemtype __________ ____________ ________ branchcode categorycode itemtype branchcode categorycode * branchcode * itemtype branchcode * * * categorycode itemtype * categorycode * * * itemtype * * * And automatically extends the test when new scopes are added. This also obsoletes the "Get effective issuing rule in correct order" test in t/db_dependent/Koha/IssuingRules.t To test: 1. prove t/db_dependent/Koha/CirculationRules.t Sponsored-by: The National Library of Finland Signed-off-by: Joonas Kylmälä Signed-off-by: Katrin Fischer JD amended patch: perl tidy Signed-off-by: Jonathan Druart --- t/db_dependent/Koha/CirculationRules.t | 224 +++++++------------ t/db_dependent/Koha/IssuingRules.t | 297 +------------------------ 2 files changed, 82 insertions(+), 439 deletions(-) diff --git a/t/db_dependent/Koha/CirculationRules.t b/t/db_dependent/Koha/CirculationRules.t index 9e3fb4439f..708168a048 100644 --- a/t/db_dependent/Koha/CirculationRules.t +++ b/t/db_dependent/Koha/CirculationRules.t @@ -33,7 +33,7 @@ my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; subtest 'set_rule + get_effective_rule' => sub { - plan tests => 14; + plan tests => 8; $schema->storage->txn_begin; @@ -91,27 +91,6 @@ subtest 'set_rule + get_effective_rule' => sub { is( $rule->rule_value, $default_rule_value, '* means default' ); - Koha::CirculationRules->set_rule( - { - branchcode => '*', - categorycode => '*', - itemtype => $itemtype, - rule_name => $rule_name, - rule_value => 2, - } - ); - - $rule = Koha::CirculationRules->get_effective_rule( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => $rule_name, - } - ); - is( $rule->rule_value, 2, - 'More specific rule is returned when itemtype is given' ); - $rule = Koha::CirculationRules->get_effective_rule( { branchcode => $branchcode_2, @@ -123,129 +102,44 @@ subtest 'set_rule + get_effective_rule' => sub { is( $rule->rule_value, 1, 'Default rule is returned if there is no rule for this branchcode' ); - Koha::CirculationRules->set_rule( - { - branchcode => '*', - categorycode => $categorycode, - itemtype => '*', - rule_name => $rule_name, - rule_value => 3, - } - ); - - $rule = Koha::CirculationRules->get_effective_rule( - { - - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => $rule_name, - } - ); - is( $rule->rule_value, 3, - 'More specific rule is returned when categorycode exists' ); - - Koha::CirculationRules->set_rule( - { - branchcode => '*', - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => $rule_name, - rule_value => 4, - } - ); - $rule = Koha::CirculationRules->get_effective_rule( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => $rule_name, - } - ); - is( $rule->rule_value, 4, - 'More specific rule is returned when categorycode and itemtype exist' ); - - Koha::CirculationRules->set_rule( - { - branchcode => $branchcode, - categorycode => '*', - itemtype => '*', - rule_name => $rule_name, - rule_value => 5, - } - ); - $rule = Koha::CirculationRules->get_effective_rule( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => $rule_name, - } - ); - is( $rule->rule_value, 5, - 'More specific rule is returned when branchcode exists' ); - - Koha::CirculationRules->set_rule( - { - branchcode => $branchcode, - categorycode => '*', - itemtype => $itemtype, - rule_name => $rule_name, - rule_value => 6, - } - ); - $rule = Koha::CirculationRules->get_effective_rule( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => $rule_name, - } - ); - is( $rule->rule_value, 6, - 'More specific rule is returned when branchcode and itemtype exists' ); - - Koha::CirculationRules->set_rule( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => '*', - rule_name => $rule_name, - rule_value => 7, - } - ); - $rule = Koha::CirculationRules->get_effective_rule( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => $rule_name, - } - ); - is( $rule->rule_value, 7, - 'More specific rule is returned when branchcode and categorycode exist' - ); + subtest 'test rule matching with different combinations of rule scopes' => sub { + my ( $tests, $order ) = prepare_tests_for_rule_scope_combinations( + { + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + }, + 'maxissueqty' + ); + + plan tests => 2**scalar @$order; + + foreach my $test (@$tests) { + my $rule_params = {%$test}; + $rule_params->{rule_name} = $rule_name; + my $rule_value = $rule_params->{rule_value} = int( rand(10) ); + + Koha::CirculationRules->set_rule($rule_params); + + my $rule = Koha::CirculationRules->get_effective_rule( + { + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + rule_name => $rule_name, + } + ); + + my $scope_output = ''; + foreach my $key ( values @$order ) { + $scope_output .= " $key" if $test->{$key} ne '*'; + } - Koha::CirculationRules->set_rule( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => $rule_name, - rule_value => 8, - } - ); - $rule = Koha::CirculationRules->get_effective_rule( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => $rule_name, + is( $rule->rule_value, $rule_value, + 'Explicitly scoped' + . ( $scope_output ? $scope_output : ' nothing' ) ); } - ); - is( $rule->rule_value, 8, - 'More specific rule is returned when branchcode, categorycode and itemtype exist' - ); + }; my $our_branch_rules = Koha::CirculationRules->search({branchcode => $branchcode}); is( $our_branch_rules->count, 4, "We added 8 rules"); @@ -506,3 +400,47 @@ subtest 'get_lostreturn_policy() tests' => sub { $schema->storage->txn_rollback; }; + +sub prepare_tests_for_rule_scope_combinations { + my ( $scope, $rule_name ) = @_; + + # Here we create a combinations of 1s and 0s the following way + # + # 000... + # 001... + # 010... + # 011... + # 100... + # 101... + # 110... + # 111... + # + # (the number of columns equals to the amount of rule scopes) + # The ... symbolizes possible future scopes. + # + # - 0 equals to circulation rule scope with any value (aka. *) + # - 1 equals to circulation rule scope exact value, e.g. + # "CPL" (for branchcode). + # + # The order is the same as the weight of scopes when sorting circulation + # rules. So the first column of numbers is the scope with most weight. + # This is defined by C<$order> which will be assigned next. + # + # We must maintain the order in order to keep the test valid. This should be + # equal to Koha/CirculationRules.pm "order_by" of C sub. + # Let's explicitly define the order and fail test if we are missing a scope: + my $order = [ 'branchcode', 'categorycode', 'itemtype' ]; + is( join(", ", sort keys %$scope), + join(", ", sort @$order), 'Missing a scope!' ) if keys %$scope ne scalar @$order; + + my @tests = (); + foreach my $value ( glob( "{0,1}" x keys %$scope || 1 ) ) { + my $test = { %$scope }; + for ( my $i=0; $i < keys %$scope; $i++ ) { + $test->{$order->[$i]} = '*' unless substr( $value, $i, 1 ); + } + push @tests, $test; + } + + return \@tests, $order; +} diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t index 4d37fcfebc..31abe27c4c 100644 --- a/t/db_dependent/Koha/IssuingRules.t +++ b/t/db_dependent/Koha/IssuingRules.t @@ -36,7 +36,7 @@ $schema->storage->txn_begin; my $builder = t::lib::TestBuilder->new; subtest 'get_effective_issuing_rule' => sub { - plan tests => 3; + plan tests => 2; my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; @@ -94,301 +94,6 @@ subtest 'get_effective_issuing_rule' => sub { ); }; - subtest 'Get effective issuing rule in correct order' => sub { - plan tests => 26; - - my $rule; - Koha::CirculationRules->delete; - is(Koha::CirculationRules->search->count, 0, 'There are no issuing rules.'); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - is($rule, undef, 'When I attempt to get effective issuing rule, then undef' - .' is returned.'); - - # undef, undef, undef => 5 - ok(Koha::CirculationRule->new({ - branchcode => undef, - categorycode => undef, - itemtype => undef, - rule_name => 'fine', - rule_value => 5, - })->store, 'Given I added an issuing rule branchcode => undef, categorycode => undef, itemtype => undef,'); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - _is_row_match( - $rule, - { - branchcode => undef, - categorycode => undef, - itemtype => undef, - rule_name => 'fine', - rule_value => 5, - }, - 'When I attempt to get effective issuing rule,' - .' then the above one is returned.' - ); - - # undef, undef, undef => 5 - # undef, undef, $itemtype => 7 - ok( - Koha::CirculationRule->new( - { - branchcode => undef, - categorycode => undef, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 7, - } - )->store, - "Given I added an issuing rule branchcode => undef, categorycode => undef, itemtype => $itemtype," - ); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - _is_row_match( - $rule, - { - branchcode => undef, - categorycode => undef, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 7, - }, - 'When I attempt to get effective issuing rule,' - .' then the above one is returned.' - ); - - # undef, undef, undef => 5 - # undef, undef, $itemtype => 7 - # undef, $categorycode, undef => 9 - ok( - Koha::CirculationRule->new( - { - branchcode => undef, - categorycode => $categorycode, - itemtype => undef, - rule_name => 'fine', - rule_value => 9, - } - )->store, - "Given I added an issuing rule branchcode => undef, categorycode => $categorycode, itemtype => undef," - ); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - _is_row_match( - $rule, - { - branchcode => undef, - categorycode => $categorycode, - itemtype => undef, - rule_name => 'fine', - rule_value => 9, - }, - 'When I attempt to get effective issuing rule,' - .' then the above one is returned.' - ); - - # undef, undef, undef => 5 - # undef, undef, $itemtype => 7 - # undef, $categorycode, undef => 9 - # undef, $categorycode, $itemtype => 11 - ok( - Koha::CirculationRule->new( - { - branchcode => undef, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 11, - } - )->store, - "Given I added an issuing rule branchcode => undef, categorycode => $categorycode, itemtype => $itemtype," - ); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - _is_row_match( - $rule, - { - branchcode => undef, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 11, - }, - 'When I attempt to get effective issuing rule,' - .' then the above one is returned.' - ); - - # undef, undef, undef => 5 - # undef, undef, $itemtype => 7 - # undef, $categorycode, undef => 9 - # undef, $categorycode, $itemtype => 11 - # $branchcode, undef, undef => 13 - ok( - Koha::CirculationRule->new( - { - branchcode => $branchcode, - categorycode => undef, - itemtype => undef, - rule_name => 'fine', - rule_value => 13, - } - )->store, - "Given I added an issuing rule branchcode => $branchcode, categorycode => undef, itemtype => undef," - ); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - _is_row_match( - $rule, - { - branchcode => $branchcode, - categorycode => undef, - itemtype => undef, - rule_name => 'fine', - rule_value => 13, - }, - 'When I attempt to get effective issuing rule,' - .' then the above one is returned.' - ); - - # undef, undef, undef => 5 - # undef, undef, $itemtype => 7 - # undef, $categorycode, undef => 9 - # undef, $categorycode, $itemtype => 11 - # $branchcode, undef, undef => 13 - # $branchcode, undef, $itemtype => 15 - ok( - Koha::CirculationRule->new( - { - branchcode => $branchcode, - categorycode => undef, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 15, - } - )->store, - "Given I added an issuing rule branchcode => $branchcode, categorycode => undef, itemtype => $itemtype," - ); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - _is_row_match( - $rule, - { - branchcode => $branchcode, - categorycode => undef, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 15, - }, - 'When I attempt to get effective issuing rule,' - .' then the above one is returned.' - ); - - # undef, undef, undef => 5 - # undef, undef, $itemtype => 7 - # undef, $categorycode, undef => 9 - # undef, $categorycode, $itemtype => 11 - # $branchcode, undef, undef => 13 - # $branchcode, undef, $itemtype => 15 - # $branchcode, $categorycode, undef => 17 - ok( - Koha::CirculationRule->new( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => undef, - rule_name => 'fine', - rule_value => 17, - } - )->store, - "Given I added an issuing rule branchcode => $branchcode, categorycode => $categorycode, itemtype => undef," - ); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - _is_row_match( - $rule, - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => undef, - rule_name => 'fine', - rule_value => 17, - }, - 'When I attempt to get effective issuing rule,' - .' then the above one is returned.' - ); - - # undef, undef, undef => 5 - # undef, undef, $itemtype => 7 - # undef, $categorycode, undef => 9 - # undef, $categorycode, $itemtype => 11 - # $branchcode, undef, undef => 13 - # $branchcode, undef, $itemtype => 15 - # $branchcode, $categorycode, undef => 17 - # $branchcode, $categorycode, $itemtype => 19 - ok( - Koha::CirculationRule->new( - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 19, - } - )->store, - "Given I added an issuing rule branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype," - ); - $rule = Koha::CirculationRules->get_effective_rule({ - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - }); - _is_row_match( - $rule, - { - branchcode => $branchcode, - categorycode => $categorycode, - itemtype => $itemtype, - rule_name => 'fine', - rule_value => 19, - }, - 'When I attempt to get effective issuing rule,' - .' then the above one is returned.' - ); - }; - subtest 'Performance' => sub { plan tests => 4; -- 2.39.5