Bug 25513

Master_U18 #788

14:48:26 koha_1       |     #   Failed test 'SWAGGER3.3.2'
14:48:26 koha_1       |     #   at t/db_dependent/api/v1/acquisitions_orders.t line 129.
14:48:26 koha_1       |     #     Structures begin differing at:
14:48:26 koha_1       |     #          $got->{unit_price} = '10177559957753600000'
14:48:26 koha_1       |     #     $expected->{unit_price} = '1.01775599577536e+19'
14:48:26 koha_1       |     # Looks like you failed 1 test of 6.
14:48:26 koha_1       | 
14:48:26 koha_1       | #   Failed test 'get() tests'
14:48:26 koha_1       | #   at t/db_dependent/api/v1/acquisitions_orders.t line 141.

Those two runs were on Docker_2

Master_U18 #772

01:45:29 koha_1       |     #   Failed test 'SWAGGER3.3.2'
01:45:29 koha_1       |     #   at t/db_dependent/api/v1/acquisitions_orders.t line 129.
01:45:29 koha_1       |     #     Structures begin differing at:
01:45:29 koha_1       |     #          $got->{unit_price_tax_included} = '15313858494394601472'
01:45:29 koha_1       |     #     $expected->{unit_price_tax_included} = '1.53138584943946e+19'
01:45:29 koha_1       |     # Looks like you failed 1 test of 6.
01:45:29 koha_1       | 
01:45:29 koha_1       | #   Failed test 'get() tests'
01:45:29 koha_1       | #   at t/db_dependent/api/v1/acquisitions_orders.t 

Also on Docker_2

Hi,

Could be link to 'ecost_tax_excluded' being a decimal 28,6 in mysql and swagger type a double?

It wouldn't be the first time a buggy double to string function didn't roundtrip for some values.

It seems '$expected->{' is coming from /usr/share/perl5/Test/More.pm, but it doesn't print the full number.

It comes from a wrong formatting, and I think it depends on some locale settings (for instance it seems to only fail on Docker_2).

The thing people should try is to set 9963405519357589504 for ecost_tax_excluded (for instance).
And see if it fails.

I cannot make it fail locally.

Created attachment 105170 [details] [review]
Bug 25513: DO NOT PUSH - try recreating the failure

You could try this patch.

Hi
There's a thinko in the patch should be:
-            value => { orderstatus => 'new' }
+            value => { orderstatus => 'new', ecost_tax_excluded => 9963405519357589504, unitprice => 10177559957753600000  }

Not
-            value => { orderstatus => 'new' }
+            value => { orderstatus => 'new', value => { ecost_tax_excluded => 9963405519357589504, unitprice => 10177559957753600000 } }

And it fails for me
ok 1 - list() tests
# Subtest: get() tests
    1..6
[Thu May 21 10:08:23 2020] [debug] GET "/api/v1/acquisitions/orders/48"
[Thu May 21 10:08:23 2020] [debug] Routing to controller "Koha::REST::V1::Auth" and action "under"
[Thu May 21 10:08:23 2020] [debug] Routing to controller "Koha::REST::V1::Acquisitions::Orders" and action "get"
[Thu May 21 10:08:23 2020] [debug] 200 OK (0.039639s, 25.228/s)
    ok 1 - GET //fgMbl_VX3EQexGiV17S9yOb3KOVToMz0qyMP_TOrYUeMkzAppWplcQ5izgERU:thePassword123@/api/v1/acquisitions/orders/48
    ok 2 - SWAGGER3.2.2
    not ok 3 - SWAGGER3.3.2

    #   Failed test 'SWAGGER3.3.2'
    #   at ./t/db_dependent/api/v1/acquisitions_orders.t line 129.
    #     Structures begin differing at:
    #          $got->{ecost_tax_excluded} = '9963405519357589504'
    #     $expected->{ecost_tax_excluded} = '9.96340551935759e+18'
[Thu May 21 10:08:23 2020] [debug] GET "/api/v1/acquisitions/orders/48"
[Thu May 21 10:08:23 2020] [debug] Routing to controller "Koha::REST::V1::Auth" and action "under"
[Thu May 21 10:08:23 2020] [debug] Routing to controller "Koha::REST::V1::Acquisitions::Orders" and action "get"
[Thu May 21 10:08:23 2020] [debug] 404 Not Found (0.034099s, 29.326/s)
    ok 4 - GET //fgMbl_VX3EQexGiV17S9yOb3KOVToMz0qyMP_TOrYUeMkzAppWplcQ5izgERU:thePassword123@/api/v1/acquisitions/orders/48
    ok 5 - 404 Not Found
    ok 6 - exact match for JSON Pointer "/error"
    # Looks like you failed 1 test of 6.
not ok 2 - get() tests

Indeed!
But still not fail for me (I tried yesterday and wrote the patch this morning...)

Didier, ping me on IRC when you are available :)

Created attachment 105175 [details] [review]
Bug 25513: DO NOT PUSH - try recreating the failure

This is failing in stretch

Created attachment 105177 [details] [review]
Bug 25513: DO NOT PUSH - try recreating the failure

This is failing in stretch:
Stretch with mysql 5.5
Stretch with 10.1.44-MariaDB-1~bionic (docker-compose.mariadb_d9.yml)

Not on Buster with 10.1.44-MariaDB-1~bionic (docker-compose.mariadb_d9.yml)

However, using Postman I see the same output (exponent format) with both stretch and buster:
  "unit_price": 1.01775599577536e+19,
  "ecost_tax_excluded": 9.96340551935759e+18,

buster has JSON=4.02
stretch has JSON=2.90

% more xxx.pl
use Modern::Perl;
use Mojo::JSON qw(decode_json encode_json);
my $bytes = encode_json {foo => 9963405519357589504, bar => 10177559957753600000};
say $bytes;

buster:
% perl xxx.pl
{"bar":10177559957753600000,"foo":9963405519357589504}

stretch:
% perl xxx.pl
{"bar":10177559957753600000,"foo":9963405519357589504}

I am missing something obvious...

Could be something like?
https://dba.stackexchange.com/questions/172216/why-would-mysqls-roundn-dd-1-sometimes-show-no-digits-after-the-decimal-place

(In reply to didier from comment #15)
> Could be something like?
> https://dba.stackexchange.com/questions/172216/why-would-mysqls-roundn-dd-1-
> sometimes-show-no-digits-after-the-decimal-place

Yes, that seems related.
Stretch has 4.041-2 and Buster has 4.050-2

(In reply to Jonathan Druart from comment #14)
> buster has JSON=4.02
> stretch has JSON=2.90
> 
> % more xxx.pl
> use Modern::Perl;
> use Mojo::JSON qw(decode_json encode_json);
> my $bytes = encode_json {foo => 9963405519357589504, bar =>
> 10177559957753600000};
> say $bytes;
> 
> buster:
> % perl xxx.pl
> {"bar":10177559957753600000,"foo":9963405519357589504}
> 
> stretch:
> % perl xxx.pl
> {"bar":10177559957753600000,"foo":9963405519357589504}
> 
> I am missing something obvious...

Note that this was meaningless, we wanted to test:

use Mojo::JSON qw(decode_json encode_json);
my $bytes = encode_json {foo => 9963405519357589504.0000, bar => 10177559957753600000.0000};
say $bytes;

Both return 
{"bar":1.01775599577536e+19,"foo":9.96340551935759e+18}

Created attachment 105265 [details] [review]
Bug 25513: Fix random failure from acquisitions_orders.t

No idea if this is correct, but the tests pass without that condition

See bug 17932

Additional notes:
1. This may break Jessie, so we should have a patch for stables that will "fix" the tests by adding specific value to the order we create
2. There is a "format" parameter in the API spec, but we don't use it
https://swagger.io/docs/specification/data-models/data-types/
Maybe we should.

Another fix would be to extend the test in that area (TO_JSON) and add  _float_column_type. We could then use Math::BigFloat:
  use Math::BigFloat;
  my $x = Math::BigFloat->new(10177559957753600000);
  my $y = Math::BigFloat->new(1.01775599577536e+19);
  say $x;say $y;
will display twice 10177559957753600000

(In reply to Jonathan Druart from comment #19)
> Additional notes:
> 1. This may break Jessie, so we should have a patch for stables that will
> "fix" the tests by adding specific value to the order we create
> 2. There is a "format" parameter in the API spec, but we don't use it
> https://swagger.io/docs/specification/data-models/data-types/
> Maybe we should.
> 
> Another fix would be to extend the test in that area (TO_JSON) and add 
> _float_column_type. We could then use Math::BigFloat:
>   use Math::BigFloat;
>   my $x = Math::BigFloat->new(10177559957753600000);
>   my $y = Math::BigFloat->new(1.01775599577536e+19);
>   say $x;say $y;
> will display twice 10177559957753600000

I agree we should be identifying floats and using:

"type:" "number",
"format": "float"

I would like to mention that the like you provided is for OAS3 and we are using OAS2
https://swagger.io/specification/v2/

Created attachment 105317 [details] [review]
Bug 25513: Fix random failure from acquisitions_orders.t

No idea if this is correct, but the tests pass without that condition

See bug 17932

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Tested on koha-testing-docker on Ubuntu 18.04

By running the test in a loop (on master) I could get 2 failures in less that 20 runs.
But after that it took 150 runs to get a failure.

Thanks to the patch "Bug 25513: DO NOT PUSH - try recreating the failure" , now it fails every time.
And the 2nd patch (fix) works! :D

On master + only the fix: the tests passed 1350 times and counting. Looks pretty solid :)

In total, overnight, the tests passed 4649 times. Unconceivable to reach without the patch.

This bug is related to an original bug we reported to DBD::mysql RT:
https://rt.cpan.org/Public/Bug/Display.html?id=119904
which is now migrated to:
https://github.com/perl5-dbi/DBD-mysql/issues/212

The basic idea is that under some circumstances DBD::mysql output failed the looks_like_number test because of wrong flags set on the scalar variable.

This bug is removing our workaround under the assumption current DBD::mysql versions on our supported OS are patched. This is the case in Stretch, Buster and Focal. But as the Github link suggests, there's still questions about this being really fixed for good.

I'm adding a regression test based on Jonathan's DO NOT PUSH patch, so our CI can catch any issues now and in the future.

Created attachment 105393 [details] [review]
Bug 25513: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Created attachment 105395 [details] [review]
Bug 25513: Remove unneeded integer casting in Koha::Object->TO_JSON

This patch removes a problematic integer casting happening in TO_JSON.
The original need for this cast (a DBD::mysql bug) is now fixed. We
remove the cast as it is causing random failures.

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/acquisitions_orders.t
=> FAIL: Tests fail with select high values (i.e. casting gives wrong
        results)
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests now pass!
5. Sign off :-D

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Pushed to master for 20.05

Hm, bug still happened on Ubuntu 18.04 for last 3 builds.

https://jenkins.koha-community.org/job/Koha_Master_U18/813/
https://jenkins.koha-community.org/job/Koha_Master_U18/812/
https://jenkins.koha-community.org/job/Koha_Master_U18/811/

I confirm I can make it fail 100% of the time on U18, but only occasionally on D10 (hum?)

On U18 it's failing on the tests we wrote for this specific problem ("Regression tests for bug 25513"), but on D10 it fails on another one:

t/db_dependent/api/v1/acquisitions_orders.t .. 1/5 
    #   Failed test 'SWAGGER3.3.2'
    #   at t/db_dependent/api/v1/acquisitions_orders.t line 129.
    #     Structures begin differing at:
    #          $got->{ecost_tax_included} = '2.7621662673219e+21'
    #     $expected->{ecost_tax_included} = '2762166267321895092224.000000'

missing dependencies, not backported to 19.11.x