rspec-expectations icon indicating copy to clipboard operation
rspec-expectations copied to clipboard

Update StartWith, EndWith to use start_with?, end_with? if available

Open bclayman-sq opened this issue 4 years ago • 2 comments

This addresses issue #1025.

With this change, the StartWith matcher will rely on an object's start_with? method if available. Similarly, the EndWith matcher will rely on an object's end_with? method if available.

This is especially useful when a class implements start_with? but not the indexing operator, or end_with? but not the indexing operator.

bclayman-sq avatar Oct 13 '21 15:10 bclayman-sq

Looks great! Wondering why it wasn't done like that from the very beginning.

Wondering if we need to check if there are multiple args:

"abc".start_with?('a', 'c') # => true

https://ruby-doc.org/core-2.7.1/String.html#method-i-start_with-3F seems to allow all prefixes, while the matcher's semantic is different.

Yeah, that's a great point. If we do this:

expect("abc").to start_with("a", "c")

Then expected is ["a", "c"] and my implementation doesn't handle this correctly: "abc".start_with?(["a", "c"]) fails with an error.

I think it's OK to assume that any class that implements start_with? allows you to pass multiple args (like in your string example). In light of that, I think I can splat expected like this:

return actual.send(method_name, *expected) if actual.respond_to?(method_name)

As you and @JonRowe pointed out, this is a breaking change, so totally get that this would have to be in the 4.0 release 👍

bclayman-sq avatar Oct 14 '21 14:10 bclayman-sq

I'm not convinced to make a breaking change. There is be_start_with dynamic matcher that works similarly to start_with?. And we can keep the known behaviour for start_with intact.

To still be able to benefit from this change, I'd suggest to only delegate to start_with? if the matcher gets a single argument. For multiple - fallback to the previous behaviour.

On a side note - adding deprecation messages is not much fun.

pirj avatar Oct 15 '21 12:10 pirj

Migrated to the monorepo.

JonRowe avatar Nov 30 '24 12:11 JonRowe