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

Output chained matchers description #1439

Open
inkstak opened this issue Dec 13, 2023 · 6 comments
Open

Output chained matchers description #1439

inkstak opened this issue Dec 13, 2023 · 6 comments

Comments

@inkstak
Copy link

inkstak commented Dec 13, 2023

Subject of the issue

I've created a custom matcher which allows to chain other matchers against a computed value :

matcher :have_html_body do
  chain :to, :other_matcher
  
  match do |actual|
    @actual = actual.parsed_body

    html_body_valid?(@actual) && other_matcher_matches?(@actual)
  end
  
  def other_matcher_matches?(actual)
    other_matcher.nil? || other_matcher.matches?(actual)
  end
end

I can then chain any matchers to match the parsed body :

expect(response).to have_html_body.to include("Hello World")
expect(response).to have_html_body.to match(/hello/)
expect(response).to have_html_body.to have_selector("#test", text: "Hello World")

It works fine, but descriptions are cumbersome :

is expected to have html body to #<RSpec::Matchers::BuiltIn::Include:0x0000000108887c10 [...long output...]>
is expected to have html body to #<RSpec::Matchers::BuiltIn::Match:0x000000010f476228 [...long output...]>
is expected to have html body to #<Capybara::RSpecMatchers::Matchers::HaveText:0x0000000140b35fd8 [...long output...]>

Expected behavior

It should generate a readable description :

is expected to have html body to include "Hello World"
is expected to have html body to match /hello/
is expected to have html body to have visible css "#test" with text "Hello World"

Your environment

  • Ruby 3.2.2
  • rspec 3.12.0
  • rspec-expectations 3.12.3
  • rspec-support 3.12.1

config.include_chain_clauses_in_custom_matcher_descriptions is already set to true

Investigation

There are 3 methods involved to generate chained description :

I'm not sure which one is the most relevant to fix the issue but my guess is EnglishPhrasing.
Something like :

   def self.list(obj)
     return " #{RSpec::Support::ObjectFormatter.format(obj)}" if !obj || Struct === obj || Hash === obj
-    items = Array(obj).map { |w| RSpec::Support::ObjectFormatter.format(w) }
+    items = Array(obj).map { |w| w.is_a?(RSpec::Matchers::Composable) ? w.description : RSpec::Support::ObjectFormatter.format(w) }
@pirj
Copy link
Member

pirj commented Dec 13, 2023

This is not how you chain matchers.
There’s the and/or mechanism built-in http://rspec.info/features/3-12/rspec-expectations/compound-expectations/

@inkstak
Copy link
Author

inkstak commented Dec 13, 2023

Thanks, but I dont want to chain matcher with the same subject.
Matchers chained to have_html_body.to receive a transformed subject.

@pirj
Copy link
Member

pirj commented Dec 13, 2023

Your example is not indicative of that: you pass @actual to the other matcher. Sorry, my bad, I’ve missed the parse_body part.

I understand what you are trying to work around. This will be solved in this pr #1319, but it’s a breaking change, so only in RSpec 4.

@inkstak
Copy link
Author

inkstak commented Dec 13, 2023

Sounds nice but it seems to me that the PR you've linked will only change the output diff on test failures. Am I wrong ?
Maybe this is part of larger changes for RSpec 4 ?

In the matcher above, I have no problem to output a good failure message of chained matchers ... except for the description part.

matcher :have_html_body do
  chain :to,     :other_matcher
  chain :not_to, :other_negative_matcher
  
  failure_message do
    message  = "expected to #{description}"
    message += "\n#{other_matcher&.failure_message}" if other_matcher
    message += "\n#{other_negative_matcher&.failure_message_when_negated}" if other_negative_matcher
    message
  end
end

@pirj
Copy link
Member

pirj commented Dec 13, 2023

Do you have any issue with printed example descriptions of chained matchers built using the built-in compound mechanism?

@pirj
Copy link
Member

pirj commented Dec 13, 2023

Confusingly, chain is not to chain matchers, but rather to add qualifiers to the same matcher. Think be_within/of. See http://rspec.info/documentation/3.12/rspec-expectations/RSpec/Matchers/DSL/Macros.html#chain-instance_method

i can’t tell if dsl-defined matchers are compoundable by default.

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

2 participants