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

Differ does not report differences in string arrays #518

Open
dmolesUC opened this issue Oct 18, 2021 · 5 comments · May be fixed by #519
Open

Differ does not report differences in string arrays #518

dmolesUC opened this issue Oct 18, 2021 · 5 comments · May be fixed by #519

Comments

@dmolesUC
Copy link

Subject of the issue

When comparing two arrays of objects, Differ provides a useful report on which expected objects are missing and which unexpected objects are present. When comparing two arrays of strings, however, the array diff is omitted and only the truncated inspect diff is present.

Looking at the source code, it appears this was sort of intentional; but on the other hand it looks as though the original change back in 2010 was only intended to make sure the multiline differ wasn't applied to single-line strings. Ignoring all single-line string arrays just seems to have been a side effect.

Your environment

  • Ruby version: 3.0.2
  • rspec-support version: 3.10.2

Steps to reproduce

Run the following (failing) spec:

module ModuleWithALongName
  class ClassWithALongName
    include Comparable

    attr_reader :v

    def initialize(v)
      @v = v
    end

    def to_s
      "Corge<#{v}>"
    end

    def <=>(o)
      return nil unless o.is_a?(ClassWithALongName)
      self.v <=> o.v
    end

    def inspect
      "#{self.class.name}<value: #{v}>"
    end
  end

  describe ClassWithALongName do
    let(:d1) { Array.new(5) { |i| ClassWithALongName.new(i) } }
    let(:d2) { Array.new(5) { |i| ClassWithALongName.new((i / 2).odd? ? i : i * 2) } }

    context 'as objects' do
      it 'is equal' do
        expect(d1).to eq(d2)
      end
    end

    context 'as strings' do
      it 'is equal' do
        d1s = d1.map(&:inspect)
        d2s = d2.map(&:inspect)
        expect(d1s).to eq(d2s)
      end
    end
  end
end

Expected behavior

Failure messages for both tests should show the array diff.

Actual behavior

Only the failure message for as objects shows the array diff.

Failures:

  1) ModuleWithALongName::ClassWithALongName as objects is equal
     Failure/Error: expect(d1).to eq(d2)
     
       expected: [ModuleWithALongName::ClassWithALongName<value: 0>, ModuleWithALongName::ClassWithALongName<value: 2>...oduleWithALongName::ClassWithALongName<value: 3>, ModuleWithALongName::ClassWithALongName<value: 8>]
            got: [ModuleWithALongName::ClassWithALongName<value: 0>, ModuleWithALongName::ClassWithALongName<value: 1>...oduleWithALongName::ClassWithALongName<value: 3>, ModuleWithALongName::ClassWithALongName<value: 4>]
     
       (compared using ==)
     
       Diff:
       
       
       @@ -1,6 +1,6 @@
        [ModuleWithALongName::ClassWithALongName<value: 0>,
       + ModuleWithALongName::ClassWithALongName<value: 1>,
         ModuleWithALongName::ClassWithALongName<value: 2>,
       - ModuleWithALongName::ClassWithALongName<value: 2>,
         ModuleWithALongName::ClassWithALongName<value: 3>,
       - ModuleWithALongName::ClassWithALongName<value: 8>]
       + ModuleWithALongName::ClassWithALongName<value: 4>]
       
     # ./spec/lib/module_with_a_long_name_spec.rb:33:in `block (3 levels) in <module:ModuleWithALongName>'
     # /Users/david/.rvm/gems/ruby-3.0.2/gems/webmock-3.14.0/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'

  2) ModuleWithALongName::ClassWithALongName as strings is equal
     Failure/Error: expect(d1s).to eq(d2s)
     
       expected: ["ModuleWithALongName::ClassWithALongName<value: 0>", "ModuleWithALongName::ClassWithALongName<value:...leWithALongName::ClassWithALongName<value: 3>", "ModuleWithALongName::ClassWithALongName<value: 8>"]
            got: ["ModuleWithALongName::ClassWithALongName<value: 0>", "ModuleWithALongName::ClassWithALongName<value:...leWithALongName::ClassWithALongName<value: 3>", "ModuleWithALongName::ClassWithALongName<value: 4>"]
     
       (compared using ==)
     # ./spec/lib/module_with_a_long_name_spec.rb:41:in `block (3 levels) in <module:ModuleWithALongName>'
     # /Users/david/.rvm/gems/ruby-3.0.2/gems/webmock-3.14.0/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
2 examples, 2 failures, 0 passed
Finished in 0.037725 seconds
@dmolesUC
Copy link
Author

FWIW, the ridiculous workaround I've come up with as a temporary hack to debug failing tests is this:

    context 'as string map values' do
      it 'is equal' do
        d1s = d1.map(&:inspect)
        d2s = d2.map(&:inspect)

        expect(d1s.map { |v| { v: v } }).to eq(d2s.map { |v| { v: v } })
      end
    end

Which produces the at-least-somewhat-readable array diff:

 [{:v=>"ModuleWithALongName::ClassWithALongName<value: 0>"},
+ {:v=>"ModuleWithALongName::ClassWithALongName<value: 1>"},
  {:v=>"ModuleWithALongName::ClassWithALongName<value: 2>"},
- {:v=>"ModuleWithALongName::ClassWithALongName<value: 2>"},
  {:v=>"ModuleWithALongName::ClassWithALongName<value: 3>"},
- {:v=>"ModuleWithALongName::ClassWithALongName<value: 8>"}]
+ {:v=>"ModuleWithALongName::ClassWithALongName<value: 4>"}]

@pirj
Copy link
Member

pirj commented Oct 19, 2021

Can you dig why it was done initially? I found "Extract diffing code from expectations to support" commit, and it leads to a different repo.

@dmolesUC
Copy link
Author

@pirj See this commit in rspec-expectations (also linked above). Note that the code was in fail_with.rb at that time.

@pirj
Copy link
Member

pirj commented Oct 20, 2021

Ah, sorry, I've missed this. The original reasoning of not showing such diffs:

-that
+this

doesn't really apply to string arrays.

Would you like to submit a PR?

@dmolesUC dmolesUC linked a pull request Oct 20, 2021 that will close this issue
@dmolesUC
Copy link
Author

@pirj Done.

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

Successfully merging a pull request may close this issue.

2 participants