Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass the hexapdf test suite #2822

Open
eregon opened this issue Jan 9, 2023 · 4 comments
Open

Pass the hexapdf test suite #2822

eregon opened this issue Jan 9, 2023 · 4 comments
Assignees

Comments

@eregon
Copy link
Member

eregon commented Jan 9, 2023

This used to work in #1391.
Unfortunately there was no CI for hexapdf at the time and it regressed (e.g. #2770).
Now hexapdf has a CI: gettalong/hexapdf#199 (comment)

So we should suggest to add truffleruby there, but first of course we need to pass all tests.
Some tests fail on latest hexapdf, see https://github.com/eregon/hexapdf/actions/workflows/ci.yml

@eregon eregon assigned eregon and andrykonchin and unassigned eregon Jan 9, 2023
@andrykonchin
Copy link
Member

andrykonchin commented Jan 10, 2023

I've investigated the failed specs and found out that there are basically two issues:

  • with Array#reject
  • and with precision of floating point numbers

Array#reject

The first issue can be reproduced by modifying an array during iterations through the array with #reject/#reject!

It can be reproduced with the following code:

a = [:a]
counter = 0

a.reject do |el|
  if counter < 1
    a << :b
    counter += 1
  end
  false
end

In CRuby (3.1.3) #reject sees the added element and calls a block for it as well.

On TruffleRuby internal exception is raised.

Floating point numbers

There are several tests that fail because number.round(6).to_s returns a string representation with more than 6 digits after a point:

Test #1

# rbenv local 3.1.3
6.5358983848622465.round(6).to_s # => "6.535898"
5.7639320225002075.round(6).to_s # => "5.763932"
6.535898384862244.round(6).to_s # => "6.535898"

# rbenv local truffleruby-jvm
6.5358983848622465.round(6).to_s => "6.5358979999999995"
5.763932022500207.round(6).to_s => "5.7639320000000005"
6.535898384862244.round(6).to_s => "6.5358979999999995"

Test #2

# rbenv local 3.1.3
1.616.round(6).to_s # => "1.616"

# rbenv local truffleruby-jvm
1.616.round(6).to_s # => "1.6159999999999999"

Test #3

# rbenv local 3.1.3
23.609.round(6).to_s # => "23.609"

# rbenv local truffleruby-jvm
23.609.round(6).to_s # => "23.608999999999998"

Test #4

# rbenv local 3.1.3
3.381966011250105.round(6).to_s # => "3.381966"
8.624771282204213.round(6).to_s # => "8.624771"
7.911127102658033.round(6).to_s # => "7.911127"
7.911127102658032.round(6).to_s # => "7.911127"
8.624771282204211.round(6).to_s # => "8.624771"

# rbenv local truffleruby-jvm
3.381966011250105.round(6).to_s # => "3.3819660000000002"
8.624771282204213.round(6).to_s # => "8.624770999999999"
7.911127102658033.round(6).to_s # => "7.9111270000000005"
7.911127102658032.round(6).to_s # => "7.9111270000000005"
8.624771282204211.round(6).to_s # => "8.624770999999999"

@eregon
Copy link
Member Author

eregon commented Jan 10, 2023

Thanks, it looks like we need fix reject/reject!, and also fix Float#round.
Maybe we have an issue similar to https://bugs.ruby-lang.org/issues/19318 re Float#round.
@aardvark179 worked on Float#round in a8d4075 but it seems we still have some incompatibility there.

Note that if the to_s differs but the value would be == it wouldn't matter much and that would then be an issue of Float#to_s. But the above seems actual issues of Float#round, e.g.:

$ ruby -e 'p [5.763932022500207.round(6), 5.763932]'
[5.763932, 5.763932, true] # CRuby
[5.7639320000000005, 5.763932, false] # TruffleRuby

@andrykonchin
Copy link
Member

Array#reject is fixed in e11a36d.

@gettalong
Copy link

I just tested HexaPDF 1.0.0 with the latest TruffleRuby 24.1.1. Besides the issue with Float#round that @eregon mentions, there is only two other issues left:

  3) Error:
HexaPDF::DictionaryFields::DateConverter#test_0002_allows conversion to a Time object from a binary string:
ArgumentError: Zone offset not in valid range: -18:00 to +18:00
    <internal:core> core/truffle/time_operations.rb:92:in `compose'
    <internal:core> core/time.rb:427:in `new'
    /home/thomas/exthdsync/personal/repos/hexapdf/lib/hexapdf/dictionary_fields.rb:334:in `convert'
    /home/thomas/exthdsync/personal/repos/hexapdf/lib/hexapdf/dictionary_fields.rb:179:in `block in convert'
    /home/thomas/exthdsync/personal/repos/hexapdf/lib/hexapdf/dictionary_fields.rb:178:in `each'
    /home/thomas/exthdsync/personal/repos/hexapdf/lib/hexapdf/dictionary_fields.rb:178:in `convert'
    /home/thomas/exthdsync/personal/repos/hexapdf/test/hexapdf/test_dictionary_fields.rb:204:in `block (4 levels) in <top (required)>'
    /home/thomas/exthdsync/personal/repos/hexapdf/test/hexapdf/test_dictionary_fields.rb:179:in `each'
    /home/thomas/exthdsync/personal/repos/hexapdf/test/hexapdf/test_dictionary_fields.rb:179:in `test_0002_allows conversion to a Time object from a binary string'

  4) Failure:
HexaPDF::Layout::TableBox::fit::row spans#test_0001_works if content of a row span cell is larger than the rows [/home/thomas/exthdsync/personal/repos/hexapdf/test/hexapdf/layout/test_table_box.rb:400]:
cell 0 height.
--- expected
+++ actual
@@ -1 +1 @@
-30
+Infinity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants