Bugzilla – Attachment 106904 Details for
Bug 25113
Make CirculationRules.t flexible for new scope combinations
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25113: Refactor CirculationRules.t when testing scope combinations
Bug-25113-Refactor-CirculationRulest-when-testing-.patch (text/plain), 21.29 KB, created by
Joonas Kylmälä
on 2020-07-15 07:24:22 UTC
(
hide
)
Description:
Bug 25113: Refactor CirculationRules.t when testing scope combinations
Filename:
MIME Type:
Creator:
Joonas Kylmälä
Created:
2020-07-15 07:24:22 UTC
Size:
21.29 KB
patch
obsolete
>From 03d9123d3431581730898f994eaf99d3a554cc55 Mon Sep 17 00:00:00 2001 >From: Lari Taskula <lari.taskula@hypernova.fi> >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=UTF-8 >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ä <joonas.kylmala@helsinki.fi> >--- > t/db_dependent/Koha/CirculationRules.t | 208 ++++++++--------------- > t/db_dependent/Koha/IssuingRules.t | 297 +-------------------------------- > 2 files changed, 72 insertions(+), 433 deletions(-) > >diff --git a/t/db_dependent/Koha/CirculationRules.t b/t/db_dependent/Koha/CirculationRules.t >index f24542e2b6..8be962cae7 100644 >--- a/t/db_dependent/Koha/CirculationRules.t >+++ b/t/db_dependent/Koha/CirculationRules.t >@@ -32,7 +32,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; > >@@ -90,27 +90,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, >@@ -122,129 +101,40 @@ 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, >- } >- ); >+ subtest 'test rule matching with different combinations of rule scopes' => sub { > >- $rule = Koha::CirculationRules->get_effective_rule( >- { >- >- branchcode => $branchcode, >+ my ( $tests, $order ) = prepare_tests_for_rule_scope_combinations( { >+ branchcode => $branchcode, > categorycode => $categorycode, >- itemtype => $itemtype, >- rule_name => $rule_name, >- } >- ); >- is( $rule->rule_value, 3, >- 'More specific rule is returned when categorycode exists' ); >+ itemtype => $itemtype, >+ }, 'maxissueqty' ); > >- 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' ); >+ plan tests => 2**scalar @$order; > >- 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' ); >+ 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( >- { >- 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( $rule_params ); > >- 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' >- ); >+ my $rule = Koha::CirculationRules->get_effective_rule( { >+ branchcode => $branchcode, >+ categorycode => $categorycode, >+ itemtype => $itemtype, >+ rule_name => $rule_name, >+ } ); > >- 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, >+ my $scope_output = ''; >+ foreach my $key ( values @$order ) { >+ $scope_output .= " $key" if $test->{$key} ne '*'; >+ } >+ >+ 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"); >@@ -376,3 +266,47 @@ subtest 'get_effective_daysmode' => 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<get_effective_rule> 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.11.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=106904">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 25113
:
102765
|
103350
|
106904
|
108963
|
108978