Bugzilla – Attachment 74422 Details for
Bug 18736
Problems in order calculations (rounding errors)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18736: (follow-up) Add missing test cases
Bug-18736-follow-up-Add-missing-test-cases.patch (text/plain), 8.83 KB, created by
Nick Clemens (kidclamp)
on 2018-04-18 11:13:57 UTC
(
hide
)
Description:
Bug 18736: (follow-up) Add missing test cases
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2018-04-18 11:13:57 UTC
Size:
8.83 KB
patch
obsolete
>From ebaf51a00749a86d53de7178825e8eebddf6d9ee Mon Sep 17 00:00:00 2001 >From: Mark Tompsett <mtompset@hotmail.com> >Date: Tue, 17 Apr 2018 23:39:57 +0000 >Subject: [PATCH] Bug 18736: (follow-up) Add missing test cases > >We now fully cover GetBudgetsPlanCell in these tests >--- > C4/Budgets.pm | 2 +- > t/db_dependent/Budgets.t | 106 +++++++++++++++++++++++++++++++---------------- > 2 files changed, 72 insertions(+), 36 deletions(-) > >diff --git a/C4/Budgets.pm b/C4/Budgets.pm >index d7c4c19..f10485a 100644 >--- a/C4/Budgets.pm >+++ b/C4/Budgets.pm >@@ -206,7 +206,7 @@ sub SetOwnerToFundHierarchy { > > # ------------------------------------------------------------------- > sub GetBudgetsPlanCell { >- my ( $cell, $period, $budget ) = @_; #FIXME we don't use $period or $budget >+ my ( $cell, $period, $budget ) = @_; #FIXME we don't use $period > my ($actual, $sth); > my $dbh = C4::Context->dbh; > my $roundsql = _get_rounding_sql(qq|ecost_tax_included|); >diff --git a/t/db_dependent/Budgets.t b/t/db_dependent/Budgets.t >index 7dd22d4..90e4832 100755 >--- a/t/db_dependent/Budgets.t >+++ b/t/db_dependent/Budgets.t >@@ -807,32 +807,46 @@ is( scalar @{$authCat}, 0, "GetBudgetAuthCats returns only non-empty sorting cat > > subtest 'GetBudgetSpent GetBudgetOrdered GetBudgetsPlanCell tests' => sub { > >- plan tests => 12; >+ plan tests => 16; > > #Let's build an order, we need a couple things though > t::lib::Mocks::mock_preference('OrderPriceRounding','nearest_cent'); > >- my $spent_biblio = $builder->build({ source => 'Biblio' }); >- my $spent_basket = $builder->build({ source => 'Aqbasket', value => { is_standing => 0 } }); >- my $spent_invoice = $builder->build({ source => 'Aqinvoice'}); >+ my $spent_biblio = $builder->build({ source => 'Biblio' }); >+ my $item_1 = $builder->build({ source => 'Item', value => { biblionumber => $spent_biblio->{biblionumber} } }); >+ my $biblioitem_1 = $builder->build({ source => 'Biblioitem', value => { biblionumber => $spent_biblio->{biblionumber}, itemnumber => $item_1->{itemnumber} } }); >+ my $spent_basket = $builder->build({ source => 'Aqbasket', value => { is_standing => 0 } }); >+ my $spent_invoice = $builder->build({ source => 'Aqinvoice'}); > my $spent_currency = $builder->build({ source => 'Currency', value => { active => 1, archived => 0, symbol => 'F', rate => 2, isocode => undef, currency => 'FOO' } }); >- my $spent_vendor = $builder->build({ source => 'Aqbookseller',value => { listincgst => 0, listprice => $spent_currency->{currency}, invoiceprice => $spent_currency->{currency} } }); >+ my $spent_vendor = $builder->build({ source => 'Aqbookseller',value => { listincgst => 0, listprice => $spent_currency->{currency}, invoiceprice => $spent_currency->{currency} } }); >+ my $budget_authcat = $builder->build({ source => 'AuthorisedValueCategory', value => {} }); >+ my $spent_sort1 = $builder->build({ source => 'AuthorisedValue', value => { >+ category => $budget_authcat->{category_name}, >+ authorised_value => 'PICKLE', >+ } >+ }); >+ my $spent_budget = $builder->build({ source => 'Aqbudget', value => { >+ sort1_authcat => $budget_authcat->{category_name}, >+ } >+ }); > my $spent_orderinfo = { >- basketno => $spent_basket->{basketno}, >- booksellerid => $spent_vendor->{id}, >- rrp => 16.99, >- discount => .42, >- ecost => 16.91, >- biblionumber => $spent_biblio->{biblionumber}, >- currency => $spent_currency->{currency}, >- tax_rate_on_ordering => 0, >- tax_value_on_ordering => 0, >- tax_rate_on_receiving => 0, >- tax_value_on_receiving => 0, >- quantity => 8, >- quantityreceived => 0, >+ basketno => $spent_basket->{basketno}, >+ booksellerid => $spent_vendor->{id}, >+ rrp => 16.99, >+ discount => .42, >+ ecost => 16.91, >+ biblionumber => $spent_biblio->{biblionumber}, >+ currency => $spent_currency->{currency}, >+ tax_rate_on_ordering => 0, >+ tax_value_on_ordering => 0, >+ tax_rate_on_receiving => 0, >+ tax_value_on_receiving => 0, >+ quantity => 8, >+ quantityreceived => 0, > datecancellationprinted => undef, >- datereceived => undef, >+ datereceived => undef, >+ budget_id => $spent_budget->{budget_id}, >+ sort1 => $spent_sort1->{authorised_value}, > }; > > #Okay we have basically what the user would enter, now we do some maths >@@ -860,7 +874,6 @@ subtest 'GetBudgetSpent GetBudgetOrdered GetBudgetsPlanCell tests' => sub { > #Let's test some budget planning > #Regression tests for bug 18736 > #We need an item to test by BRANCHES >- my $item_1 = $builder->build({ source => 'Item' }); > my $order_item_1 = $builder->build({ source => 'AqordersItem', value => { ordernumber => $spent_order->{ordernumber}, itemnumber => $item_1->{itemnumber} } }); > my $spent_fund = Koha::Acquisition::Funds->find( $spent_order->{budget_id} ); > my $cell = { >@@ -868,23 +881,46 @@ subtest 'GetBudgetSpent GetBudgetOrdered GetBudgetsPlanCell tests' => sub { > cell_authvalue => $spent_order->{entrydate}, #normally this is just the year/month but full won't hurt us here > budget_id => $spent_order->{budget_id}, > budget_period_id => $spent_fund->budget_period_id, >+ sort1_authcat => $spent_order->{sort1_authcat}, >+ sort2_authcat => $spent_order->{sort1_authcat}, >+ }; >+ my $test_values = { >+ 'MONTHS' => { >+ authvalue => $spent_order->{entrydate}, >+ expected_rounded => 9.85, >+ expected_exact => 9.8542, >+ }, >+ 'BRANCHES' => { >+ authvalue => $item_1->{homebranch}, >+ expected_rounded => 9.85, >+ expected_exact => 9.8542, >+ }, >+ 'ITEMTYPES' => { >+ authvalue => $biblioitem_1->{itemtype}, >+ expected_rounded => 78.80, >+ expected_exact => 78.8336, >+ }, >+ 'ELSE' => { >+ authvalue => $spent_sort1->{authorised_value}, >+ expected_rounded => 78.8, >+ expected_exact => 78.8336, >+ }, > }; >- t::lib::Mocks::mock_preference('OrderPriceRounding',''); >- my ( $actual ) = GetBudgetsPlanCell( $cell, undef, undef); #we are only testing the actual for now >- is ( $actual, '9.854200', "We expect this to be an exact order cost"); #really we should expect cost*quantity but we don't >- t::lib::Mocks::mock_preference('OrderPriceRounding','nearest_cent'); >- ( $actual ) = GetBudgetsPlanCell( $cell, undef, undef); #we are only testing the actual for now >- is ( $actual, '9.8500', "We expect this to be a rounded order cost"); #really we should expect cost*quantity but we don't >- $cell->{authcat} = 'BRANCHES'; >- $cell->{authvalue} = $item_1->{homebranch}; >- ( $actual ) = GetBudgetsPlanCell( $cell, undef, undef); #we are only testing the actual for now >- t::lib::Mocks::mock_preference('OrderPriceRounding',''); >- ( $actual ) = GetBudgetsPlanCell( $cell, undef, undef); #we are only testing the actual for now >- is ( $actual, '9.854200', "We expect this to be full cost for items from the branch"); #here we rely on items having been created for each order, again quantity should be considered >- t::lib::Mocks::mock_preference('OrderPriceRounding','nearest_cent'); >- ( $actual ) = GetBudgetsPlanCell( $cell, undef, undef); #we are only testing the actual for now >- is ( $actual, '9.8500', "We expect this to be rounded cost for items from the branch"); #here we rely on items having been created for each order, again quantity should be considered > >+ for my $authcat ( keys %$test_values ) { >+ my $test_val = $test_values->{$authcat}; >+ my $authvalue = $test_val->{authvalue}; >+ my $expected_rounded = $test_val->{expected_rounded}; >+ my $expected_exact = $test_val->{expected_exact}; >+ $cell->{authcat} = $authcat; >+ $cell->{authvalue} = $authvalue; >+ t::lib::Mocks::mock_preference('OrderPriceRounding',''); >+ my ( $actual ) = GetBudgetsPlanCell( $cell, undef, $spent_budget); #we are only testing the actual for now >+ is ( $actual+0, $expected_exact, "We expect this to be an exact order cost ($authcat)"); #really we should expect cost*quantity but we don't >+ t::lib::Mocks::mock_preference('OrderPriceRounding','nearest_cent'); >+ ( $actual ) = GetBudgetsPlanCell( $cell, undef, $spent_budget); #we are only testing the actual for now >+ is ( $actual+0, $expected_rounded, "We expect this to be a rounded order cost ($authcat)"); #really we should expect cost*quantity but we don't >+ } > > #Okay, now we can receive the order, giving the price as the user would > >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18736
:
64061
|
64065
|
70187
|
70188
|
70189
|
70768
|
71062
|
71063
|
71627
|
71628
|
71629
|
71630
|
71631
|
71632
|
74335
|
74336
|
74337
|
74338
|
74339
|
74340
|
74341
|
74342
|
74370
|
74371
|
74374
|
74397
|
74398
|
74422
|
74427
|
74438
|
76979
|
76980
|
76981
|
76982
|
76983
|
76984
|
76985
|
76986
|
76987
|
76988
|
76989
|
76990
|
76991
|
77614
|
77615
|
77616
|
77617
|
77618
|
78101
|
78120
|
78981
|
78982
|
78983
|
78984
|
78985
|
78986
|
78987
|
78988
|
79087
|
79088
|
79089
|
79090
|
79091
|
79092
|
79093
|
79094
|
79098
|
79099
|
79100
|
79101
|
79102
|
79103
|
79104
|
79105
|
79187
|
79223
|
79562
|
79563
|
82967
|
82968
|
82969
|
82970
|
82971
|
82972
|
82973
|
82974
|
82975
|
82976
|
82977
|
83212
|
83213
|
83214
|
83973
|
83974
|
83975
|
83976
|
83977
|
83978
|
83979
|
83980
|
83981
|
83982
|
83983
|
83984
|
83985
|
86646
|
86647
|
86648
|
86649
|
86650
|
86651
|
86652
|
86653
|
86654
|
86655
|
86656
|
86657
|
86658