Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Change failure message for BeComparedTo#failure_message_when_negated #417

Merged
merged 1 commit into from Jan 9, 2014

Conversation

Projects
None yet
3 participants
Contributor

prathamesh-sonpatki commented Jan 8, 2014

  • Normal error message will be given for all operator except comparison
    operators
  • For comparison operators such as gt/lt/gte/lte, with normal message a
    warning saying that the usage is confusing is given.
  • "It might be more clearly expressed without the" is dropped completely
    as it doesn't sound correct with except
  • Fixes #386

Personally think the variable don't help. Would be clearer as:

def failure_message_when_negated
  "`expect(#{@actual}).not_to be #{@operator} #{@expected}`".tap do |message|
    if [:<, :>, :<=, :>=].include?(@operator)
      message << " not only FAILED,\nit is a bit confusing."
    end
  end
end
Owner

JonRowe commented Jan 8, 2014

I'd prefer to see this without the tap, we don't use it that commonly in rspec.

Perhaps:

def failure_message_when_negated
  message = "`expect(#{@actual}).not_to be #{@operator} #{@expected}`"
  if [:<, :>, :<=, :>=].include?(@operator)
      message << " not only FAILED,\nit is a bit confusing."
  end
  message
end
Owner

JonRowe commented Jan 9, 2014

or

def failure_message_when_negated
  message = "`expect(#{@actual}).not_to be #{@operator} #{@expected}`"

  if [:<, :>, :<=, :>=].include?(@operator)
    message +  " not only FAILED,\nit is a bit confusing."
  else
    message
  end
end

Yeah, that's nicer than the explicit return.

Contributor

prathamesh-sonpatki commented Jan 9, 2014

Thanks. I will update soon
On Jan 9, 2014 7:53 AM, "Thomas Drake-Brockman" notifications@github.com
wrote:

Yeah, that's nicer than the explicit return.


Reply to this email directly or view it on GitHubhttps://github.com/rspec/rspec-expectations/pull/417#issuecomment-31897279
.

@prathamesh-sonpatki prathamesh-sonpatki Change failure message for BeComparedTo#failure_message_when_negated
- Normal error message will be given for all operator except comparison
  operators
- For comparison operators such as gt/lt/gte/lte, with normal message a
  warning saying that the usage is confusing is given.
- "It might be more clearly expressed without the" is dropped completely
  as it doesn't sound correct with except
- Fixes #386
046e45c
Contributor

prathamesh-sonpatki commented Jan 9, 2014

@JonRowe Done

@JonRowe JonRowe added a commit that referenced this pull request Jan 9, 2014

@JonRowe JonRowe Merge pull request #417 from prathamesh-sonpatki/issue-386
Change failure message for BeComparedTo#failure_message_when_negated
4356321

@JonRowe JonRowe merged commit 4356321 into rspec:master Jan 9, 2014

1 check passed

default The Travis CI build passed
Details

@prathamesh-sonpatki prathamesh-sonpatki deleted the prathamesh-sonpatki:issue-386 branch Jan 9, 2014

@JonRowe JonRowe added a commit that referenced this pull request Jan 9, 2014

@JonRowe JonRowe changelog for #417
[skip ci]
7fcce6a

@JonRowe JonRowe added a commit that referenced this pull request Jan 9, 2014

@JonRowe JonRowe cleanup after #417 3f025fe

@myronmarston myronmarston added a commit that referenced this pull request Jan 9, 2014

@myronmarston myronmarston A bit more cleanup for #417.
- Inspect actual and expected in the message - this
  is important so `"Foo"` is clearly a string and not
  a class.
- No need for the line break.
- Use `fail_with` matcher, as it specifies the exception
  class and is more consistent with other specs.
59d1f38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment