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

A failing spec with expect(value).to eq 1.0 raises JSON::ParserError #162

Open
maxnotarangelo opened this issue Nov 19, 2022 · 5 comments
Open

Comments

@maxnotarangelo
Copy link

maxnotarangelo commented Nov 19, 2022

I'm getting a JSON::ParserError with the following code:

it "lets you compare a big float with another float" do
  expect(100_000.1).to eq 1.0
end

It looks like it's happening only when both numbers are floats, and the first is at least 100,000. The error is happening here: https://github.com/mcmire/super_diff/blob/d0ccdf204a54da14bfecdbb306008527c52f1686/lib/super_diff/helpers.rb#L55

The proximate cause is that

ObjectSpace.dump(100_000.1)
=> "100000."

I think this is a similar issue to #144.

@mcmire
Copy link
Collaborator

mcmire commented Nov 21, 2022

Hi @maxnotarangelo, thanks for the report, makes sense to me. I'll have to think about how best to fix this as I sense it may be a tricky one although I'm not sure.

@mcmire
Copy link
Collaborator

mcmire commented Feb 11, 2023

Hi @maxnotarangelo, just re-reviewing the issues list and ran across this issue. A fix for #144 was shipped in 0.9.0. Not sure if you still use the gem, but does that fix this issue for you?

@maxnotarangelo
Copy link
Author

Hmm, I'm already on 0.9.0 and I'm still getting the same error.

@maxnotarangelo
Copy link
Author

This is still happening for me on 0.12.1

@jas14
Copy link
Collaborator

jas14 commented Oct 11, 2024

The problem is basically that for CRuby, object_address_for is trying to reimplement Object#inspect in Ruby code, without the low-level information available in the C code.

Before Ruby 2.7.0, object_id was derived directly from the memory address1, so one could reverse that to get the actual memory address. 2.7.0 introduced compaction, which can change an object's memory address, so we switched to using ObjectSpace.dump, which is sketchy:

Generally, you SHOULD NOT use this library if you do not know about the MRI implementation. Mainly, this library is for (memory) profiler developers and MRI developers who need to know about MRI memory usage.

ObjectSpace.dump happens to return JSON data. But relying on that format is quite brittle, and evidently floats aren't dumped as JSON anyway.

So, to solve this particular issue, let's just rescue JSON parse errors.

In the long term, we should either A) add a C extension to get the object's address in CRuby (seems like overkill if all we're trying to do is make things look more like the built-in #inspect output), or B) change that identifier to just be the object ID, assuming people don't care if it's the literal memory address.

Footnotes

  1. allegedly; I haven't confirmed this myself

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