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

Hash include with composable matcher gives misleading diff #1120

Open
owst opened this issue Jun 19, 2019 · 3 comments
Open

Hash include with composable matcher gives misleading diff #1120

owst opened this issue Jun 19, 2019 · 3 comments

Comments

@owst
Copy link

owst commented Jun 19, 2019

Expecting a hash to include another when using composable matchers gives a misleading diff for the subset of the hash that did match:

  1) includes should include {:a => /2/, :b => "bar"}
     Failure/Error:
       expect(
         {
           a: '123',
           b: 'foo',
         }
       ).to include(a: /2/, b: 'bar')
     
       expected {:a => "123", :b => "foo"} to include {:b => "bar"}
       Diff:
       @@ -1,3 +1,3 @@
       -:a => /2/,
       -:b => "bar",
       +:a => "123",
       +:b => "foo",

Notice that

-:a => /2/,
+:a => "123",

is in the diff, despite "123" being matched by /2/ - this can make it harder to determine what the problem is, with a larger hash.

N.b. that the expected {:a => "123", :b => "foo"} to include {:b => "bar"} part of the failure message is good, and does indicate the problem - this issue is only about the "Diff:" part.

Your environment

Resolving dependencies...
Using bundler 1.17.3
Using diff-lcs 1.3
Using rspec-support 3.7.1
Using rspec-core 3.7.1
Using rspec-expectations 3.7.0
Using rspec-mocks 3.7.0
Using rspec 3.7.0
Ruby version is: 2.5.3

Steps to reproduce

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  gem "rspec", "3.7.0" # Activate the gem and version you are reporting the issue against.
end

puts "Ruby version is: #{RUBY_VERSION}"
require 'rspec/autorun'

RSpec.describe 'includes' do
  it do
    expect(
      {
        a: '123',
        b: 'foo',
      }
    ).to include(a: /2/, b: 'bar')
  end
end

Expected behavior

The diff does not highlight matched entries as different:

@@ -1,3 +1,3 @@
 :a => "123",
-:b => "bar",
+:b => "foo",

Actual behavior

The diff does highlight matched entries as different:

@@ -1,3 +1,3 @@
-:a => /2/,
-:b => "bar",
+:a => "123",
+:b => "foo",
@benoittgt
Copy link
Member

benoittgt commented Jun 20, 2019

Hello

match should work.

You can try:

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  gem "rspec"
end

require 'rspec/autorun'

RSpec.describe 'includes' do
  it do
    expect(
      {
        a: '123',
        b: 'foo',
      }
    ).to match(a: /2/, b: 'foo')
  end
end

@benoittgt benoittgt reopened this Jun 20, 2019
@benoittgt
Copy link
Member

Sorry to close the issue. My answer is not the one you expect. Let me have a further look.

@JonRowe
Copy link
Member

JonRowe commented Jun 21, 2019

The issue here is that the differ diff-lcs diffs objects as strings, and the strings are different, thus this is the output we get. Improving this is something we'd very much like but theres no immediate bug fix here.

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