aeson icon indicating copy to clipboard operation
aeson copied to clipboard

aeson-2.1.2.1 test suite fails on 32bit/i686

Open sternenseemann opened this issue 2 years ago • 2 comments

The offending tests seem to be:

  • /i_number_huge_exp.json.tokens bs/
  • /i_number_huge_exp.json.tokens lbs/
  • /i_number_huge_exp.json.decode via tokens/

Full log

Using Nix/nixpkgs, i686-linux, GHC 9.4.6

sternenseemann avatar Aug 14 '23 13:08 sternenseemann

We are seeing this in Debian as well, the error is:

  i_number_huge_exp.json
      decode:                                      FAIL
        Test output was different from 'tests/JSONTestSuite/results/i_number_huge_exp.txt'. Output of ["diff","-u","tests/JSONTestSuite/results/i_number_huge_exp.txt","/tmp/i_number_huge_exp3240953-4.actual"]:
        --- tests/JSONTestSuite/results/i_number_huge_exp.txt	2001-09-09 01:46:40.000000000 +0000
        +++ /tmp/i_number_huge_exp3240953-4.actual	2023-10-21 06:05:02.580198038 +0000
        @@ -1 +1 @@
        -Right (Array [Number 4.0e-30000000995])
        +Right (Array [Number 4.0e64770077])
        
        Use -p '$0=="tests.JSONTestSuite.i_number_huge_exp.json.decode"' to rerun this test only.
      decode via tokens:                           FAIL
        Test output was different from 'tests/JSONTestSuite/results/i_number_huge_exp.txt'. Output of ["diff","-u","tests/JSONTestSuite/results/i_number_huge_exp.txt","/tmp/i_number_huge_exp3240953-5.actual"]:
        --- tests/JSONTestSuite/results/i_number_huge_exp.txt	2001-09-09 01:46:40.000000000 +0000
        +++ /tmp/i_number_huge_exp3240953-5.actual	2023-10-21 06:05:02.588198009 +0000
        @@ -1 +1 @@
        -Right (Array [Number 4.0e-30000000995])
        +Right (Array [Number 4.0e64770077])
        
        Use -p '/i_number_huge_exp.json.decode via tokens/' to rerun this test only.
      tokens bs:                                   FAIL (0.01s)
        Test output was different from 'tests/JSONTestSuite/results/i_number_huge_exp.tok'. Output of ["diff","-u","tests/JSONTestSuite/results/i_number_huge_exp.tok","/tmp/i_number_huge_exp3240953-6.actual"]:
        --- tests/JSONTestSuite/results/i_number_huge_exp.tok	2001-09-09 01:46:40.000000000 +0000
        +++ /tmp/i_number_huge_exp3240953-6.actual	2023-10-21 06:05:02.596197979 +0000
        @@ -1,4 +1,4 @@
         TkArrayOpen
         TkItem
        -TkNumber NumScientific 4.0e-30000000995
        +TkNumber NumScientific 4.0e64770077
         TkArrayEnd
        
        Use -p '/i_number_huge_exp.json.tokens bs/' to rerun this test only.
      tokens lbs:                                  FAIL
        Test output was different from 'tests/JSONTestSuite/results/i_number_huge_exp.tok'. Output of ["diff","-u","tests/JSONTestSuite/results/i_number_huge_exp.tok","/tmp/i_number_huge_exp3240953-7.actual"]:
        --- tests/JSONTestSuite/results/i_number_huge_exp.tok	2001-09-09 01:46:40.000000000 +0000
        +++ /tmp/i_number_huge_exp3240953-7.actual	2023-10-21 06:05:02.604197950 +0000
        @@ -1,4 +1,4 @@
         TkArrayOpen
         TkItem
        -TkNumber NumScientific 4.0e-30000000995
        +TkNumber NumScientific 4.0e64770077
         TkArrayEnd
        
        Use -p '/i_number_huge_exp.json.tokens lbs/' to rerun this test only.

I bisected this and found the commit that introduced this regression is f089cdbf5dc (CC @phadej ). I believe this commit is wrong because the cached results are only valid for 64-bit systems and not for 32-bit ones. In Debian, I have reverted this commit in Debian for now.

iliastsi avatar Oct 21 '23 08:10 iliastsi

The real issue here is that on both systems the exponent in numbers may overflow (and on 32bit system JSONTestSuite happened to have tests where it does), but the parse is still successful. So that is clearly wrong.

There are two options:

  • change Scientific exponent to be an Integer, so overflow won't happen. Then the behavior will be the same.
  • Catch overflows and report parse errors. That will result in different behaviors on different systems.

phadej avatar Oct 21 '23 09:10 phadej