search

Zulu-Inuoe / jzon

A correct and safe(er) JSON RFC 8259 reader/writer with sane defaults.
MIT License
151 stars 14 forks source link

Tests failed on ECL for v1.0.0 #36

Closed Hellseher closed 1 year ago

Hellseher commented 1 year ago

Hi,

While packing JZON for Guix I've faced with an issue where tests failed on ECL implementation:

  Running test STRINGIFY-COERCE-KEY-WRITES-DOUBLE-FLOATS-WITHOUT-D0 An error occurred during initialization:
Cannot print object Implementation not supported. readably..
error: in phase 'check': uncaught exception:
%exception #<&invoke-error program: "/gnu/store/cawsqjfnnv7qd0qdjq7jmlqp440jqx8m-ecl-21.2.1/bin/ecl" arguments: ("--eval" "(require :asdf)" "--eval" "(asdf:initialize-source-registry (list :source-registry (list :tree (uiop:ensure-pathname \"/gnu/store/a0f8grgfa341lpz0n3xqkv1kvssasv0m-ecl-jzon-1.0.0/share/common-lisp/ecl/jzon\" :truenamize t :ensure-directory t)) :inherit-configuration))" "--eval" "(asdf:test-system \"com.inuoe.jzon-tests\")" "--eval" "(quit)") exit-status: 1 term-signal: #f stop-signal: #f>

Thanks, Oleg

Zulu-Inuoe commented 1 year ago

I see. Do the other tests pass? I've not tested jzon on ECL - I could see the number parsing might have non-portable issues there.

Hellseher commented 1 year ago

Hi,

Thanks for a quick responce. If you have an access to Guix you may reproduce it:

guix time-machine --commit=d37b467631d5b0e965ea933b8bda8448993580e9 -- build ecl-jzon

Failed on check phase:

phase `build' succeeded after 6.9 seconds
starting phase `check'
Invoking ecl: "/gnu/store/cawsqjfnnv7qd0qdjq7jmlqp440jqx8m-ecl-21.2.1/bin/ecl" "--eval" "(require :asdf)" "--eval" "(asdf:initialize-source-registry (list :source-registry (list :tree (uiop:ensure-pathname \"/gnu/store/a0f8grgfa341lpz0n3xqkv1kvssasv0m-ecl-jzon-1.0.0/share/c
ommon-lisp/ecl/jzon\" :truenamize t :ensure-directory t)) :inherit-configuration))" "--eval" "(asdf:test-system \"com.inuoe.jzon-tests\")" "--eval" "(quit)"
;;; Loading #P"/gnu/store/cawsqjfnnv7qd0qdjq7jmlqp440jqx8m-ecl-21.2.1/lib/ecl-21.2.1/asdf.fas"
;;; Warning: System definition file #P"/gnu/store/6y261dg8imgfhg9mb67hc15w5lmfpbc1-ecl-flexi-streams-1.0.19/share/common-lisp/ecl/flexi-streams/flexi-streams.asd" contains definition for system "flexi-streams-test". Please only define "flexi-streams" and secondary systems w
ith a name starting with "flexi-streams/" (e.g. "flexi-streams/test") in that file.
;;;
;;; Compiling /gnu/store/a0f8grgfa341lpz0n3xqkv1kvssasv0m-ecl-jzon-1.0.0/share/common-lisp/ecl/jzon/test/jzon-tests.lisp.
;;; OPTIMIZE levels: Safety=2, Space=0, Speed=3, Debug=0
;;;
;;; End of Pass 1.
;;; Finished compiling /gnu/store/a0f8grgfa341lpz0n3xqkv1kvssasv0m-ecl-jzon-1.0.0/share/common-lisp/ecl/jzon/test/jzon-tests.lisp.
;;;

Running test suite JZON
 Running test suite PARSING
  Running test PARSES-INTEGERS ..
  Running test PARSES-NEGATIVE-ZERO.0 X
  Running test PARSES-ARRAYS-DISALLOWS-EMPTY-WITH-COMMA ..
  Running test PARSE-ALLOWS-STRINGS-AT-MAX-STRING-LENGTH .
  Running test PARSE-4.9E-324 X
  Running test PARSES-ATOMS ......
  Running test PARSE-LIMITS-MAX-STRING-LENGTH .
  Running test PARSE-DISALLOWS-BLOCK-COMMENTS .
  Running test UNICODE-CHARS ...
  Running test PARSES-OBJECT-DISALLOWS-TRAILING-COMMA-WITH-EOF ..
  Running test PARSE-NESTED ...X
  Running test PARSE-RETURNS-BASE-STRINGS ..
  Running test PARSE-COMMENTS-DELIMIT-ATOMS .....
  Running test PARSE-ACCEPTS-STREAM ....X
  Running test PARSE-97924.49742786969 X
  Running test PARSE-LIMITS-MAX-STRING-LENGTH-WITH-ESCAPE-CODES .
  Running test EMPTY-ARRAY-INSIDE-ARRAY .
  Running test PARSES-ATOMS-ERROR-ON-INCOMPLETE ....................
  Running test PARSES-OBJECTS-EOF ..
  Running test PARSE-ACCEPTS-NON-SIMPLE-STRING ....X
  Running test DISALLOWS-TRAILING-EXPONENT-MARKER ...
  Running test PARSE-LINE-COMMENTS-DO-NOT-END-ON-CR-ONLY-LF ..
  Running test PARSE-5E-324 X
  Running test PARSES-ARRAYS-DISALLOWS-TRAILING-COMMA-WITH-EOF ..
  Running test PARSES-DECIMALS ..
  Running test PARSES-OBJECT-DISALLOWS-EMPTY-WITH-COMMA ..
  Running test PARSE--1.31300000121E8 X
  Running test MISCELLANEOUS-BLOCK-COMMENT-TESTS ...........
  Running test PARSE-ALLOWS-STRINGS-BELOW-MAX-STRING-LENGTH .
  Running test PARSE-IGNORES-PRE-POST-WHITESPACE ....X
  Running test UNTERMINATED-BLOCK-COMMENT-ERRORS ..
  Running test PARSE-ALLOWS-COMMENTS-WHEN-ASKED .
  Running test EMPTY-OBJECT-INSIDE-ARRAY .
  Running test PARSE-MULTIPLE X
  Running test PARSES-ARRAYS ..
  Running test PARSE-ALLOWS-BLOCK-COMMENTS-WHEN-ASKED .
  Running test PARSE-22057.311791265754 X
  Running test PARSE-4.8E-324 X
  Running test PARSES-OBJECTS ..
  Running test PARSES-EXPONENT X
  Running test PARSE-USES-CUSTOM-KEY-FN .
  Running test PARSES-OBJECTS-ALLOWS-TRAILING-COMMA-WHEN-ASKED .
  Running test PARSES-ARRAYS-DISALLOWS-SEVERAL-TRAILING-COMMAS ..
  Running test PARSE-ACCEPTS-OCTET-VECTOR ....X
  Running test PARSE-23456789012E66 X
  Running test PARSE-POOLS-KEYS ..
  Running test PARSE-0.000000000000000000000034567890120102012 X
  Running test PARSES-ARRAYS-DISALLOWS-TRAILING-COMMA .
  Running test PARSE-NEGATIVE-DECIMAL X
  Running test suite INCREMENTAL
   Running test MULTI-CLOSE-OK
   Running test PARSE-NEXT-AFTER-CLOSE-ERRORS .
   Running test PARSE-NEXT-BASICS ................
   Running test PARSE-NEXT-AFTER-COMPLETE-RETURNS-NIL .......
   Running test PARSE-NEXT-AFTER-TOPLEVEL-CONTINUES-FAILING ....
   Running test PARSE-NEXT-ERRORS-AFTER-TOPLEVEL ..
  Running test PARSE-DISALLOWS-COMMENTS .
  Running test DISALLOWS-TRAILING-DECIMAL-POINT ..
  Running test PARSES-OBJECTS-DISALLOWS-TRAILING-COMMA .
  Running test PARSE-0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000000000005 X
  Running test PARSE-SINGULAR ..X
  Running test PARSES-NEGATIVE-ZERO X
  Running test PARSE-1.31300000121E8 X
  Running test PARSE-ACCEPTS-BINARY-STREAM ....X
  Running test PARSES-ARRAYS-ALLOWS-TRAILING-COMMA-WHEN-ASKED .
  Running test PARSE-DOES-NOT-NEST-BLOCK-COMMENTS .
  Running test PARSE-MAX-STRING-LENGTH-RESPECTS-ESCAPE-CODES-2 .
  Running test PARSES-ARRAYS-SIGNALS-EOF .
  Running test PARSES-OBJECTS-DISALLOWS-SEVERAL-TRAILING-COMMAS ..
  Running test PARSE-MAX-STRING-LENGTH-RESPECTS-ESCAPE-CODES-1 .
  Running test PARSE-ACCEPTS-PATHNAME ....X
  Running test PARSES-ZERO .
  Running test DISALLOWS-LEADING-ZEROS ...
  Running test PARSES-DECIMALS-WITH-EXPONENT X
  Running test PARSE-ERRORS-ON-TOO-LARGE-STRING-LENGTH .
 Running test suite STRINGIFY
  Running test STRINGIFY-REPLACER-REPLACES-VALUES-USING-MULTIPLE-VALUES .
  Running test STRINGIFY-COERCE-KEY-CALLS-FN .
  Running test STRINGIFY-PRETTY-ARRAY-NEWLINES-IF-NESTED-OBJECT f
  Running test STRINGIFY-REPLACER-IS-CALLED-RECURSIVELY .
  Running test STRINGIFY-CLASS-USES-TYPE-FOR-NIL .
  Running test STRINGIFY-NESTED-ARRAY-PRETTY .
  Running test STRINGIFY-SIGNALS-TYPE-ERROR-ON-IMPROPER-SEQUENCE .
  Running test STRINGIFY-INTEGERS ..
  Running test STRINGIFY-WORKS-ON-BINARY-STREAMS .
  Running test STRINGIFY-COERCE-KEY-WRITES-SINGLE-FLOATS-WITHOUT-S0 An error occurred during initialization:
Cannot print object Implementation not supported. readably..
error: in phase 'check': uncaught exception:
%exception #<&invoke-error program: "/gnu/store/cawsqjfnnv7qd0qdjq7jmlqp440jqx8m-ecl-21.2.1/bin/ecl" arguments: ("--eval" "(require :asdf)" "--eval" "(asdf:initialize-source-registry (list :source-registry (list :tree (uiop:ensure-pathname \"/gnu/store/a0f8grgfa341lpz0n3xqk
v1kvssasv0m-ecl-jzon-1.0.0/share/common-lisp/ecl/jzon\" :truenamize t :ensure-directory t)) :inherit-configuration))" "--eval" "(asdf:test-system \"com.inuoe.jzon-tests\")" "--eval" "(quit)") exit-status: 1 term-signal: #f stop-signal: #f>
phase `check' failed after 1.8 seconds
command "/gnu/store/cawsqjfnnv7qd0qdjq7jmlqp440jqx8m-ecl-21.2.1/bin/ecl" "--eval" "(require :asdf)" "--eval" "(asdf:initialize-source-registry (list :source-registry (list :tree (uiop:ensure-pathname \"/gnu/store/a0f8grgfa341lpz0n3xqkv1kvssasv0m-ecl-jzon-1.0.0/share/common-
lisp/ecl/jzon\" :truenamize t :ensure-directory t)) :inherit-configuration))" "--eval" "(asdf:test-system \"com.inuoe.jzon-tests\")" "--eval" "(quit)" failed with status 1
builder for `/gnu/store/f65dr545dlxsq5ilnrrcwqbkiwcxzmw8-ecl-jzon-1.0.0.drv' failed with exit code 1
build of /gnu/store/f65dr545dlxsq5ilnrrcwqbkiwcxzmw8-ecl-jzon-1.0.0.drv failed
View build log at '/var/log/guix/drvs/f6/5dr545dlxsq5ilnrrcwqbkiwcxzmw8-ecl-jzon-1.0.0.drv.gz'.
guix build: error: build of `/gnu/store/f65dr545dlxsq5ilnrrcwqbkiwcxzmw8-ecl-jzon-1.0.0.drv' failed
Zulu-Inuoe commented 1 year ago

Hey no problem. Unfortunately I don't think I've got access to guix. I'm on a Windows machine, and I'm having trouble getting ECL to run on there, too, due to some locale settings it doesn't seem to like.

Thanks for the log, it's helpful in at least isolating that parsing seems to be OK, but printing might not be. I'm going to take a couple of blind stabs and see later about getting ECL running locally

Zulu-Inuoe commented 1 year ago

Sorry @Hellseher could you confirm what commit you're loading in? The order of the tests seems like an older version than I was expecting

Hellseher commented 1 year ago

Hi,

It's versioned tag of jzon - v1.0.0

The commit I've posted above belongs to Guix (sorry for confusion) it's for reproducible build.

Thanks, Oleg

Zulu-Inuoe commented 1 year ago

No worries. Thanks for the confirmation.

What's strange to me is that if you compare your output to the tests in that tag

https://github.com/Zulu-Inuoe/jzon/blob/v1.0.0/test/jzon-tests.lisp

You'll see that it is not including several tests before that one. That's weird, but hopefully an unrelated issue. I'll have to get ECL running in order to get to the actual issue that caused the crash, though.

I'll keep you posted, thanks

Zulu-Inuoe commented 1 year ago

So, among other things, it looks like the functions I need from float-features aren't supported on ECL at this time. Specifically for writing I make use of float-features:single-float-bits and float-features:double-float-bits But it also looks like those functions are not available for parsing either which explains why some of the parsing tests failed.

For jzon:parse I believe I can make due on ECL without float-features by #-ecl around my calls to the eisel-lemire code, but for WRITING would require more work to get ECL to spit out values in the correct formats. In addition, the tests themselves would need to be revised as right now I have tests written such as

(= (ff:bits-double-float #x419F4DEA807BE76D) (jzon:parse "1.31300000121E8"))

in which I expect a certain binary pattern from the result of parsing.

All-in-all, I would rather if we could just get those functions working on ECL, instead. Though seeing as it's not already been done, it's not likely to be easy..

Zulu-Inuoe commented 1 year ago

An extremely hacky thought occurred to me - We can abuse ECL's FFI to alloc a ffi:float or ffi:double with the bit pattern we want, then ffi:deref-pointer. There's got to be some way to replicate whatever the ffi:deref-pointer does for those types, though, without going to those extents.

Zulu-Inuoe commented 1 year ago

As hacky as it is, I did implement it this way in #39 and confirm all tests pass.

I'm going to let the PR sit unmerged for at least a week in case somebody has better ideas

The good news is that besides the float reader/writer issues, all other tests pass on ECL no problem.

Zulu-Inuoe commented 1 year ago

More info from -https://github.com/Shinmera/float-features/pull/22 IEEE-Floats has an impl in portable CL, which would be an OK backup. However, as hacky as the ffi solution is, I prefer it if it's reliable and more performant. I have not run benchmarks on ECL, yet. The only use-case I could see for not having it, is some target where ffi is not supported.

Per this commit - https://gitlab.com/embeddable-common-lisp/ecl/-/commit/1ce4713804e704dd29a1c56bfabe3ea5a129a0f5

It looks like there'll be 'native' support for this in ECL in the next release. What I'll do is implement the some code via #. to conditionalize on if that symbol is present or not. A pain

Zulu-Inuoe commented 1 year ago

Closing this issue now that I've merged the changes to main. It'll be fixed next Quicklisp release, or you can use the latest release on Ultralisp