t::lib::TestBuilder::_gen_real() always returns 2 decimal places no matter the column's size definition. > TestBuilder.pm _gen_real() > printf("%.2f", rand($max-0.1)); Our database has decimal type columns with higher than 2 decimal places. This makes comparing structures with Test::More::is_deeply impossible when the other object is built by TestBuilder, and the other fetched from database, as seen here: # Structures begin differing at: # $got->[0]{amount} = '90203.710000' # $expected->[0]{amount} = '90203.71'
Created attachment 116336 [details] [review] Bug 27630: Add tests To test: 1. prove t/db_dependent/TestBuilder.t 2. Observe failing test t/db_dependent/TestBuilder.t .. 3/13 Failed test 'There can be more decimals when the column size allows it.' at t/db_dependent/TestBuilder.t line 101. got: '67631.42' expected: '67631.420000' Looks like you failed 1 test of 4. Sponsored-by: The National Library of Finland
Created attachment 116337 [details] [review] Bug 27630: Allow more TestBuilder decimal places than 2 To test: 1. prove t/db_dependent/TestBuilder.t 2. Observe success Sponsored-by: The National Library of Finland
There's something I'm missing. When column type is "decimal", DBIx returns the trailing decimal zeroes, but when it's "float", it returns only the numbers until the significant precision. So if column size for decimals is 4, and the number being 1.0100, I get the following results from DBIx: decimal: 1.0100 float: 1.01
Created attachment 116340 [details] [review] Bug 27630: Add test to prove different behavior with floats To test: 1. prove t/db_dependent/TestBuilder.t 2. It might or might not pass all tests - if it passes, run it again until you get the following error (always with the "float" step): ok 5 - decimal - Builder builds same precision as DBIx returns. not ok 6 - float - Builder builds same precision as DBIx returns. Failed test 'float - Builder builds same precision as DBIx returns.' at t/db_dependent/TestBuilder.t line 112. got: '51.7080' expected: '51.708' Looks like you failed 1 test of 6. Sponsored-by: The National Library of Finland
Created attachment 116342 [details] [review] Bug 27630: Add test to prove different behavior with floats To test: 1. prove t/db_dependent/TestBuilder.t 2. It might or might not pass all tests - if it passes, run it again until you get the following error (always with either the "float" or "double" steps): ok 5 - decimal - Builder builds same precision as DBIx returns. not ok 6 - float - Builder builds same precision as DBIx returns. Failed test 'float - Builder builds same precision as DBIx returns.' at t/db_dependent/TestBuilder.t line 112. got: '51.7080' expected: '51.708' Looks like you failed 1 test of 7. Sponsored-by: The National Library of Finland
Could we re-fetch the generated TestBuilder object at the end of TestBuilder::_storeColumnValues() to make it identical with what Koha::Object / DBIx / DBI returns?
(In reply to Lari Taskula from comment #6) > Could we re-fetch the generated TestBuilder object at the end of > TestBuilder::_storeColumnValues() to make it identical with what > Koha::Object / DBIx / DBI returns? Related to Bug 20621
Created attachment 116426 [details] [review] Bug 27630: Add tests This proves different behavior between floats and decimals for values built by TestBuilder vs values returned by DBI/DBIx. To test: 1. prove t/db_dependent/TestBuilder.t 2. Observe failure Subtest: _gen_real formatting 1..4 not ok 1 - There can be more decimals when the column size allows it. Failed test 'There can be more decimals when the column size allows it.' at t/db_dependent/TestBuilder.t line 112. got: '10.10' expected: '10.100000' not ok 2 - decimal - Builder builds same format as DBIx returns. Failed test 'decimal - Builder builds same format as DBIx returns.' at t/db_dependent/TestBuilder.t line 118. got: '10.10' expected: '10.100000' not ok 3 - float - Builder builds same precision as DBIx returns. Failed test 'float - Builder builds same precision as DBIx returns.' at t/db_dependent/TestBuilder.t line 124. got: '10.10' expected: '10.1' not ok 4 - double - Builder builds same precision as DBIx returns. Failed test 'double - Builder builds same precision as DBIx returns.' at t/db_dependent/TestBuilder.t line 131. got: '10.10' expected: '10.1' Looks like you failed 4 tests of 4. not ok 4 - _gen_real formatting Failed test '_gen_real formatting' Sponsored-by: The National Library of Finland
Created attachment 116427 [details] [review] Bug 27630: Update database generated values and format in TestBuilder To test: 1. prove t/db_dependent/TestBuilder.t 2. Observe success TODO: Enable the commented-out test when statistics table gets a primary key Sponsored-by: The National Library of Finland
Created attachment 116440 [details] [review] Bug 27630: Add tests This proves different behavior between floats and decimals for values built by TestBuilder vs values returned by DBI/DBIx. To test: 1. prove t/db_dependent/TestBuilder.t 2. Observe failure Subtest: _gen_real formatting 1..4 not ok 1 - There can be more decimals when the column size allows it. Failed test 'There can be more decimals when the column size allows it.' at t/db_dependent/TestBuilder.t line 112. got: '10.10' expected: '10.100000' not ok 2 - decimal - Builder builds same format as DBIx returns. Failed test 'decimal - Builder builds same format as DBIx returns.' at t/db_dependent/TestBuilder.t line 118. got: '10.10' expected: '10.100000' not ok 3 - float - Builder builds same precision as DBIx returns. Failed test 'float - Builder builds same precision as DBIx returns.' at t/db_dependent/TestBuilder.t line 124. got: '10.10' expected: '10.1' not ok 4 - double - Builder builds same precision as DBIx returns. Failed test 'double - Builder builds same precision as DBIx returns.' at t/db_dependent/TestBuilder.t line 131. got: '10.10' expected: '10.1' Looks like you failed 4 tests of 4. not ok 4 - _gen_real formatting Failed test '_gen_real formatting' Sponsored-by: The National Library of Finland Signed-off-by: David Nind <david@davidnind.com>
Created attachment 116441 [details] [review] Bug 27630: Update database generated values and format in TestBuilder To test: 1. prove t/db_dependent/TestBuilder.t 2. Observe success TODO: Enable the commented-out test when statistics table gets a primary key Sponsored-by: The National Library of Finland Signed-off-by: David Nind <david@davidnind.com>
Testing notes: - Applied first patch and ran tests, then applied second patch and ran tests - Tests pass as per the test plan
Jonathan suggested using build_object() instead of build(). It works in most cases. build_object() already refreshes the object because it executes a search on the given Koha-objects class, thus returning values in the same format that Koha::Objets->search/find would have. I found only two DBIx schema files that have decimal fields with higher number of allowed decimal places than two, and no corresponding Koha::Object. These are AqbudgetsPlanning and PendingOfflineOperation. So there are very few cases where build_object would not be sufficient to solve the decimal formatting issues. Additionally, as pointed out by Jonathan, we should be first study the side effects this patch would have on the total execution time of Koha test suite. I'm gonna mark this as Failed QA until we have those numbers.