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

Diff of Range objects is less useful than default RSpec output #154

Closed
owst opened this issue Jun 21, 2022 · 4 comments
Closed

Diff of Range objects is less useful than default RSpec output #154

owst opened this issue Jun 21, 2022 · 4 comments

Comments

@owst
Copy link

owst commented Jun 21, 2022

When an expectation that two Range objects are equal is violated, the super_diff output hides the values within the Ranges:

     Failure/Error: expect(actual).to eq(expected)

       Expected #<Range:0x00007ff9338559b0> to eq #<Range:0x00007ff933855988>.

       Diff:

       ┌ (Key) ──────────────────────────┐
       │ ‹-› in expected, not in actual  │
       │ ‹+› in actual, not in expected  │
       │ ‹ › in both expected and actual │
       └─────────────────────────────────┘

         #<Range:0x00007ff9338559b0 {
         }>
     # repro.rb:20:in `block (2 levels) in <main>'

which makes it hard to diagnose the cause as you can't see the values within. The default RSpec formatter makes it clear:

     Failure/Error: expect(actual).to eq(expected)

       expected: 1..20
            got: 1..10

       (compared using ==)

       Diff:
       @@ -1 +1 @@
       -1..20
       +1..10

     # repro.rb:20:in `block (2 levels) in <main>'

would it be possible to change the super_diff output to show the values within the Ranges?

A standalone reproduction script:

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'

  gem 'rspec', "= 3.11.0"
  gem 'super_diff', "= 0.9.0", require: false
end

require "rspec/autorun"

# Uncomment the next line to see less-useful output
# require "super_diff/rspec"

RSpec.describe "Range comparisons" do
  it do
    actual = (1..10)
    expected = (1..20)

    expect(actual).to eq(expected)
  end
end

Thanks,
Owen.

@mcmire
Copy link
Collaborator

mcmire commented Jun 21, 2022

Thank you @owst, that definitely seems like a legitimate bug (I encountered a similar issue yesterday myself). I'll take a look when I get a chance!

@jas14
Copy link
Collaborator

jas14 commented Sep 16, 2024

We're falling back on the DefaultObject differ; this should just be a matter of introducing an inspection tree builder + operation tree builder that handle Range objects specifically. See #259 for a very similar fix that added handling for Data objects. This would be a good first issue!

jas14 added a commit that referenced this issue Oct 22, 2024
Fix for #154.

Adds a RangeObject inspection tree builder so Range objects are printed as strings, like this:

```
(1..2)
```

instead of as default objects:
```
#<Range:0x123456 ...
```

---------

Co-authored-by: Joe Stein <[email protected]>
owst added a commit to owst/super_diff that referenced this issue Oct 26, 2024
@owst
Copy link
Author

owst commented Oct 26, 2024

Oh, I hadn't noticed that @lucaseras had already created a very similar PR as I was about to 🙌

@jas14 should this issue be closed?

@jas14
Copy link
Collaborator

jas14 commented Oct 28, 2024

It should! I thought it had been automatically, sorry about that and thanks for highlighting.

@jas14 jas14 closed this as completed Oct 28, 2024
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