Skip to content

Fix be_routable description -- Renames BeRoutableMatcher to BeRoutable #1307

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

Closed
wants to merge 2 commits into from

Conversation

tonyta
Copy link
Contributor

@tonyta tonyta commented Feb 13, 2015

Problem
Previously, using be_routable in a one-liner...

RSpec.describe 'a not-routable route', type: :routing do
  let(:sad_route) { '/this/route/makes/me/sad' }
  specify { expect(get: sad_route).not_to be_routable }
end

...will output the following:

a not-routable route
  should not be routable matcher

BaseMatcher#description turns BeRoutableMatcher into "be routable matcher".

Solution
The solution implemented here is to rename BeRoutableMatcher to BeRoutable in order to rely on BaseMatcher#description to correctly return the following description:

a not-routable route
  should not be routable

This looks like the precedence in other matchers like BeValid.

Alternate Solution
Alternatively, #description could be overridden in BeRoutableMatcher:

def description
  "be routable"
end

I'd be happy to change PR if that's preferred 😸

RSpec::Matchers::BuiltIn::BaseMatcher#description now takes care
of correctly returning the description "be routable" instead of
"be routable matcher"
@JonRowe
Copy link
Member

JonRowe commented Feb 13, 2015

I'm not sure what @cupakromer will think but personally I'd rather see:

Alternate Solution
Alternatively, #description could be overridden in BeRoutableMatcher:

def description
  "be routable"
end

As this matcher is named inline with the rest of them in this file, and potentially people are using the constant name (although it is marked private).

@cupakromer
Copy link
Member

I agree with @JonRowe. Let's define description.

@tonyta
Copy link
Contributor Author

tonyta commented Feb 13, 2015

Thanks @JonRowe @cupakromer. I'm closing this PR and redefining #description in #1310

@tonyta tonyta closed this Feb 13, 2015
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 this pull request may close these issues.

3 participants