splitwise / super_diff

A more helpful way to view differences between complex data structures in RSpec.
https://splitwise.github.io/super_diff/
MIT License
982 stars 50 forks source link

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

Open maxnotarangelo opened 1 year ago

maxnotarangelo commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

maxnotarangelo commented 3 weeks ago

This is still happening for me on 0.12.1

jas14 commented 8 hours ago

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 address[^1], 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.

[^1]: allegedly; I haven't confirmed this myself