Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Diff from diffable custom matcher does not handle multiline string properly #420

Closed
jennek opened this Issue · 6 comments

3 participants

@jennek

[ I posted this message to the rspec mailing list and was requested to report here ]

I need to match long multiline strings (strings with \n embedded) while ignoring whitespace and comments and more.
Writing a custom matcher for that was not much of a problem using the Matcher DSL.
However, the Diff output I get does not work for me.

Diff treats the expected string as one long line, not splitting it at \n, or so it seems.

I stripped my code to show the issue. Like this the custom matcher does not make much sense anymore, but it demonstrates the issue in a single spec file.

require 'rspec'

RSpec::Matchers.define :custom_match do |expected|
  match do |actual|
    actual == expected
  end
  diffable
end

describe 'the difference in output between #== and custom matcher using #==' do

  before(:all) do
    @str1 = "line1\nline2\n"
    @str2 = "LINE1\nline2\n"
  end

  # fail on purpose
  it 'presents a diff per line' do
    @str1.should == @str2
  end
  # OUTPUT:
  # expected: "LINE1\nline2\n"
  # got: "line1\nline2\n" (using ==)
  # Diff:
  #     @@ -1,3 +1,3 @@
  # -LINE1
  # +line1
  # line2

  # fail on purpose
  it 'presents a diff for the whole string' do
    @str1.should custom_match(@str2)
  end
  # OUTPUT:
  # expected "line1\nline2\n" to custom match "LINE1\nline2\n"
  # Diff:
  #     @@ -1,2 +1,3 @@
  # -LINE1\nline2\n
  # +line1
  # +line2

end

The Diff output in the first example is what I want.
The Diff output of the second example shows that the expected string is not split as expected.

@jennek

I tried to write a spec for this, only to find out that the Diff output is not accessible by #failure_message_for_should

require 'rspec'

RSpec::Matchers.define :custom_match do |expected|
  match do |actual|
    actual == expected
  end
  diffable
end

describe 'output of diffable custom matcher' do

  before(:all) do
    @str1 = "line1\nline2\n"
    @str2 = "LINE1\nline2\n"
  end

  it 'presents the same diff as #==' do
    matcher = custom_match(@str2)
    matcher.matches? @str1
    expect(matcher.failure_message_for_should).to eq %|expected "line1\nline2\n" to custom match "LINE1\nline2\n"
Diff:
@@ -1,3 +1,3 @@
-LINE1
+line1
line2
|
  end
end

Running this results in

expected: "expected \"line1\nline2\n\" to custom match \"LINE1\nline2\n\"\nDiff:\n@@ -1,3 +1,3 @@\n-LINE1\n+line1\nline2\n"
     got: "expected \"line1\\nline2\\n\" to custom match \"LINE1\\nline2\\n\""
@JonRowe
Owner

The output of your spec is correct, as the diff is invoked internally as part of the output, so it won't show up in failure_message

@JonRowe
Owner

Although there is a bug with the diff here, as the first string expected is coerced into an array and the \n escaped...

@myronmarston

As @JonRowe said, it's not the responsibility of matchers to provide the diff from their failure_message method. RSpec adds the diff to the raised error message if the matcher indicates it's diffable. Instead of calling failure_message directly, I'd recommend you spec it like this:

module MatcherMethods
  def fail_with(message)
    raise_error(RSpec::Expectations::ExpectationNotMetError, message)
  end
end

RSpec.configure do |c|
  c.include MatcherMethods
end

describe "A custom matcher" do
  RSpec::Matchers.define :custom_match do |expected|
    match do |actual|
      actual == expected
    end
    diffable
  end

  let(:expected) { "LINE1\nline2\n" }
  let(:actual)   { "line1\nline2\n" }

  def eq_diff
    expect(actual).to eq(expected)
  rescue RSpec::Expectations::ExpectationNotMetError => e
    return e.message.sub(/\A.*Diff:/m, "Diff:")
  end

  it 'presents the same diff as the `eq` matcher' do
    expect {
      expect(actual).to custom_match(expected)
    }.to fail_with(a_string_including eq_diff)
  end
end

Just use the matcher as a user would, and set expectations about the resulting error message, as that includes the diff. Also, rather than hardcoding the expected diff, you can capture the diff from eq, and use it to match against.

@JonRowe
Owner

There is actually a bug here @myronmarston, custom matchers currently will always have expected in an array which breaks the diff output.

Desired:
screen shot 2014-01-12 at 18 43 13

Actual:
screen shot 2014-01-12 at 18 43 24

@myronmarston

Yep, I know there's a bug, that's why I asked @jennek to report this. My spec doesn't pass. I was demonstrating how to write a failing spec for this.

@JonRowe JonRowe closed this in #423
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.