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

include matcher is not diffable enough #1321

Closed
ojab opened this issue Sep 9, 2021 · 5 comments
Closed

include matcher is not diffable enough #1321

ojab opened this issue Sep 9, 2021 · 5 comments

Comments

@ojab
Copy link
Contributor

ojab commented Sep 9, 2021

Subject of the issue

Consider:

RSpec.describe 'foo' do
  RSpec::Matchers.define :my_include do |expected|
    match do |actual|
      @expected_as_array = [[expected]]
      expect(actual).to include(expected)
    end

    diffable
  end

  let(:expected) { {a: 'a' * 100, b: 2} }
  let(:actual) { [{a: 'a' * 100  , b: 3}] }

  it { expect(actual).to include(expected) }
  it { expect(actual).to my_include(expected) }
end

image

diff for my_includes shows exact lines where the issue occurred, because include matcher tries to compare single object with the whole array.
It's especially useful when working with real/more complicated data structures, for me it's unfortunately spend 3 minutes reading output or try to diff by hand usually on similar failures.

Your environment

  • Ruby version: 3.0.2
  • rspec-expectations version: current mastermain
@pirj
Copy link
Member

pirj commented Sep 10, 2021

A PR is welcome. You can start with a pending spec.
If you're up to improve this, check rspec/rspec-support#365 (and listed issues) and rspec/rspec-support#516 for details.

@ojab
Copy link
Contributor Author

ojab commented Sep 10, 2021

I don't quite understand the expected solution here, while the single expected hash is quite easy, for multiple hashes we probably should find the closest objects in the actual enumerable and show diffs for them, but I guess it will lead to #1161.

And even if we knew what should be done, not sure if I'd have the capacity to do it.
So only see something say something issue for now and workaround in a private matcher 😅

@JonRowe
Copy link
Member

JonRowe commented Oct 1, 2021

Our current differ is limited in that it has to diff everything as strings, so it "prints" objects to strings then diffs those strings. As the include matcher is diffing an array vs a single element, it's always going to be "wrong" when it comes to the diff, and we don't currently have an api to provide the differ with a more intelligent solution.

Mutating the expected is a work around, but it might cause problems for re-use (which we support) and other downstream complications, for now I'm going to close this as a "can't fix" as its essentially a duplicate of out "we'd like to improve the differ" issues.

@JonRowe JonRowe closed this as completed Oct 1, 2021
@ghost
Copy link

ghost commented Oct 3, 2023

@JonRowe What Issue should I subscribe to and read to help with the Differ limitations?

@JonRowe
Copy link
Member

JonRowe commented Oct 3, 2023

Theres a couple over on rspec/rspec-support, contributions for improving the differ or making it configurable with say super_diff would be welcome

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

No branches or pull requests

3 participants