Bug 21717 - TestBuilder.t is failing randomly
Summary: TestBuilder.t is failing randomly
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 21613
Blocks:
  Show dependency treegraph
 
Reported: 2018-10-29 20:33 UTC by Jonathan Druart
Modified: 2018-11-28 14:04 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 21717: Fix generation of real's values (2.29 KB, patch)
2018-10-29 21:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21717: Fix generation of real's values (2.39 KB, patch)
2018-10-30 00:56 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21717: Fix generation of real's values (2.45 KB, patch)
2018-11-01 18:39 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2018-10-29 20:33:59 UTC
This has been highlighted by the strict SQL modes (bug 21613), TestBuilder is not generating correctly values for decimals:

Out of range value for column 'tax_rate_on_receiving'
Out of range value for column 'discount'

I am expecting other tests to fail randomly.
Comment 1 Jonathan Druart 2018-10-29 21:25:10 UTC
Created attachment 81582 [details] [review]
Bug 21717: Fix generation of real's values

With strict SQL modes we now have TestBuilder tests that fail randomly with:
  Out of range value for column 'tax_rate_on_receiving'
  Out of range value for column 'discount'

We are trying to insert a value that is too high for the type defined at DB level.

It happens for discount when a value > 100 is generated. The datatype is float(6,4).
From the MySQL Doc:
"""
MySQL permits a nonstandard syntax: FLOAT(M,D) or REAL(M,D) or DOUBLE PRECISION(M,D).
Here, (M,D) means than values can be stored with up to M digits in total, of which D digits may be after the decimal point.
"""

So float(6,4) does not allow 100.

To recreate easily the issue:
  use t::lib::TestBuilder;
  my $builder = t::lib::TestBuilder->new;
  $builder->build( { source => 'Aqorder', value => { discount => 100 } } )->{discount};

To make sure this patch fixes the issue you can use this script:
  use Modern::Perl;
  use t::lib::TestBuilder;
  my $builder = t::lib::TestBuilder->new;
  my ( $real, $i ) = ( 0, 0 );
  while ( $real < 100 ) {
      $real = $builder->_gen_real( { info => { size => [ 6, 4 ] } } );
      warn $i++;
  }
  warn "$i - $real";

Test plan:
0/ Does not apply this patch
1/ Run the snippet above
2/ Confirm that it fails quite quickly (constantly before 500 for me)
3/ Apply this patch
4/ Run again the script
5/ Confirm that it is stuck in an infinite loop
6/ CTRL-C :D

QA Note:
Other _gen_* subroutine may need a fix, but we will wait for them to fail
TODO:
1. Add JS check to the interface to prevent the use to enter a value > 100
2. Allow '100' as a valid value for discount (?)
Comment 2 Alex Buckley 2018-10-30 00:56:30 UTC
Created attachment 81607 [details] [review]
Bug 21717: Fix generation of real's values

With strict SQL modes we now have TestBuilder tests that fail randomly with:
  Out of range value for column 'tax_rate_on_receiving'
  Out of range value for column 'discount'

We are trying to insert a value that is too high for the type defined at DB level.

It happens for discount when a value > 100 is generated. The datatype is float(6,4).
From the MySQL Doc:
"""
MySQL permits a nonstandard syntax: FLOAT(M,D) or REAL(M,D) or DOUBLE PRECISION(M,D).
Here, (M,D) means than values can be stored with up to M digits in total, of which D digits may be after the decimal point.
"""

So float(6,4) does not allow 100.

To recreate easily the issue:
  use t::lib::TestBuilder;
  my $builder = t::lib::TestBuilder->new;
  $builder->build( { source => 'Aqorder', value => { discount => 100 } } )->{discount};

To make sure this patch fixes the issue you can use this script:
  use Modern::Perl;
  use t::lib::TestBuilder;
  my $builder = t::lib::TestBuilder->new;
  my ( $real, $i ) = ( 0, 0 );
  while ( $real < 100 ) {
      $real = $builder->_gen_real( { info => { size => [ 6, 4 ] } } );
      warn $i++;
  }
  warn "$i - $real";

Test plan:
0/ Does not apply this patch
1/ Run the snippet above
2/ Confirm that it fails quite quickly (constantly before 500 for me)
3/ Apply this patch
4/ Run again the script
5/ Confirm that it is stuck in an infinite loop
6/ CTRL-C :D

QA Note:
Other _gen_* subroutine may need a fix, but we will wait for them to fail
TODO:
1. Add JS check to the interface to prevent the use to enter a value > 100
2. Allow '100' as a valid value for discount (?)

Followed test plan, patch worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 3 Tomás Cohen Arazi 2018-11-01 18:39:24 UTC
Created attachment 81834 [details] [review]
Bug 21717: Fix generation of real's values

With strict SQL modes we now have TestBuilder tests that fail randomly with:
  Out of range value for column 'tax_rate_on_receiving'
  Out of range value for column 'discount'

We are trying to insert a value that is too high for the type defined at DB level.

It happens for discount when a value > 100 is generated. The datatype is float(6,4).
From the MySQL Doc:
"""
MySQL permits a nonstandard syntax: FLOAT(M,D) or REAL(M,D) or DOUBLE PRECISION(M,D).
Here, (M,D) means than values can be stored with up to M digits in total, of which D digits may be after the decimal point.
"""

So float(6,4) does not allow 100.

To recreate easily the issue:
  use t::lib::TestBuilder;
  my $builder = t::lib::TestBuilder->new;
  $builder->build( { source => 'Aqorder', value => { discount => 100 } } )->{discount};

To make sure this patch fixes the issue you can use this script:
  use Modern::Perl;
  use t::lib::TestBuilder;
  my $builder = t::lib::TestBuilder->new;
  my ( $real, $i ) = ( 0, 0 );
  while ( $real < 100 ) {
      $real = $builder->_gen_real( { info => { size => [ 6, 4 ] } } );
      warn $i++;
  }
  warn "$i - $real";

Test plan:
0/ Does not apply this patch
1/ Run the snippet above
2/ Confirm that it fails quite quickly (constantly before 500 for me)
3/ Apply this patch
4/ Run again the script
5/ Confirm that it is stuck in an infinite loop
6/ CTRL-C :D

QA Note:
Other _gen_* subroutine may need a fix, but we will wait for them to fail
TODO:
1. Add JS check to the interface to prevent the use to enter a value > 100
2. Allow '100' as a valid value for discount (?)

Followed test plan, patch worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Nick Clemens 2018-11-01 19:10:10 UTC
Awesome work all!

Pushed to master for 18.11
Comment 5 Martin Renvoize 2018-11-09 16:33:06 UTC
Pushed to 18.05.x for 18.05.06
Comment 6 Fridolin SOMERS 2018-11-28 14:04:21 UTC
Depends on Bug 21613 not in 17.11.x