Behaviour of include against a String with a Regexp matcher "feels" inconsistent

[phil-janeapp]

Subject of the issue

With an rspec matcher such as:

expect("Hello, World!").to include(/Hello/)

there are some unclear behaviours/messages

Your environment

• Ruby version: 3.3.3
• rspec-expectations version: 3.13.2

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.13.0"
  gem "rspec-expectations", "~> 3.13.0"
end

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

RSpec.describe "Include Regexp" do
  it "should work with a regexp without a count" do
    expect("Hello, World!").to include(/Hello/)
  end
  it "works with a regexp and a count" do
    expect("Hello, World!").to include(/l/).exactly(3).times
  end
  it "works with a non-matching regexp" do
    expect("Hello, World!").to_not include(/bob/)
  end
  it "should work with a non-matching regexp and count" do
    # I realize that this expectation is ambiguous, as a literal reading means that both 
    # "no matches" or "more than one match" would pass, but at the very least
    # a better error message would be helpful
    expect("Hello, World!").to_not include(/bob/).once
  end
end

Expected behavior

All specs should pass

Actual behavior

❯ ruby test.rb
Fetching gem metadata from https://rubygems.org/..
Resolving dependencies...
Ruby version is: 3.3.3
F.F.

Failures:

  1) Include Regexp should work with a regexp without a count
     Failure/Error: expect("Hello, World!").to include(/Hello/)

     TypeError:
       no implicit conversion of Regexp into String
     # test.rb:22:in `block (2 levels) in <main>'

  2) Include Regexp works with a non-matching regexp
     Failure/Error: expect("Hello, World!").to_not include(/bob/)

     TypeError:
       no implicit conversion of Regexp into String
     # test.rb:28:in `block (2 levels) in <main>'

[phil-janeapp]

I haven't had a chance to dig in to the internals so it may be expected that this is the behaviour, but it's very counter-intuitive in how it works.

If this is intended, it would probably make sense to update the docs to clarify this. Even better would be improving the message; something such as `"expect().to include()" requires a 'times' specifier' would go a long way in clarifying the problem.

[pirj]

include behaves like this because it relies on the actual’s includes? method, which only accepts a String for a string actual.
Except for the case with counts where we do it differently.
Not to say that only the first one is counted.

Indeed this is confusing and unintuitive.
Problems I see here:

1. match is a better match, but we don’t tell the user to prefer it
2. The error message makes no sense
3. We only use the first argument to include with countable qualifiers:
   3.1 there is no warning if more arguments are passed (and swallowed)
   3.2 this is not documented

I suggest:

• deprecating the regexp with include (this relieves us from improving the error message)
• add a warning if more than one argument is passed to the matcher that also has countable qualifiers

Alternatively, we could fix the matcher to respect regexps. But the matcher’s implementation already feels overloaded.

@JonRowe what is your take on this?

[phil-janeapp]

As of RSpec 3.13, it appears that match(...).once and match(...).exactly(n).times aren't implemented, so adding support for that would also be a requirement in your suggestion list.

Otherwise I agree with the assessment. It could be argued that include sometimes reads nicer than match, but I think that's a big stretch for an actual argument against this change.

[JonRowe]

I think this is a bug, the docs certainly imply an expectation that it would work without a count (all our other matchers would behave this way), so I've fixed it.