Problem with should_not when passing a multiple args to a matcher

killswitchcollectiv · January 7, 2010, 2:28am

We seem to be hitting some undesirable behavior with should_not in
combination with matchers that accept collections. Let me use the
“include”
matcher for example (a co-worker reported similar problems using
“be_any” so
I don’t believe this is limited to “include”). The RDoc states:

Passes if actual includes expected. This works for collections and
Strings.
You can also pass in multiple args and it will only pass if all args are
found in collection.

Examples

[1,2,3].should include(3)

[1,2,3].should include(2,3) #would pass

[1,2,3].should include(2,3,4) #would fail

[1,2,3].should_not include(4)

“spread”.should include(“read”)

“spread”.should_not include(“red”)

The RDoc doesn’t give any examples of using a “should_not” along with
multiple args being passed to “include”, but since I couldn’t find any
documentation on why it would not work I assume that it should work.
Here is
a list of tests I came up with to find out what works and what doesn’t.
Note: All of these tests SHOULD fail; the problem is with the tests that
do
NOT fail.

it "does properly fail on a collection when using \"should\" with

multiple args" do

  [1,2,3].should include(2,3,4)

end



it "does NOT properly fail on a collection when using \"should_not\"

with multiple args when at least one value in the expected args is in
the
actual collection" do

  [1,2,3].should_not include(3,4,5)

end



it "does properly fail on a collection when using \"should_not\"

with
multiple args when all of the values in the expected args are in the
actual
collection" do

  [1,2,3].should_not include(1,2,3)

end



it "does properly fail on a collection when using \"should_not\"

with
multiple args when all of the values in the expected args are in the
actual
collection and when the expected args don’t cover all of the values in
the
actual collection" do

  [1,2,3].should_not include(1,2)

end



it "does properly fail on a collection when using \"should_not\"

with a
single arg" do

  [1,2,3].should_not include(3)

end



it "does properly fail on a string when using \"should_not\" with

multiple args when all of the values in the expected args are in the
actual
string" do

  "abc".should_not include("a", "b", "c")

end



it "does NOT properly fail when using \"should_not\" with multiple

args
when at least one of the values in the expected args is in the actual
string" do

  "abc".should_not include("a", "b", "e")

end



it "does properly fail on a string when using \"should_not\" with

multiple args when all of the values in the expected args are in the
actual
collection and when the expected args don’t cover all of the values in
the
actual string" do

  "abc".should_not include("a","b")

end



it "does properly fail on a string when using \"should_not\" with a

single arg" do

  "abc".should_not include("a")

end

We are using the following rspec gems:

Rspec-1.2.9

Rspec-rails-1.2.7.1

So I think one of two things needs to happen here dependent on whether
this
is desired behavior or not.

If this is desired behavior then an exception really should be raised
if
you try to use “should_not” in combination with passing a collection to
a
matcher. At the very least there needs to be some documentation to
inform
RSpec users that tests will not always fail as they might expect when
using
“should_not” in combination with a matcher that accepts multiple args.
If this is not desired behavior then of course we need a fix.

I apologize if this has been brought up previously; I did as much
searching
on this issue as possible.

Thanks in advance!

Ben F.

killswitchcollectiv · January 7, 2010, 8:21am

On Wed, Jan 6, 2010 at 7:15 PM, Ben F.
[email protected]wrote:

“spread”.should_not include(“red”)
it “does properly fail on a collection when using "should" with
actual collection” do
end
Thanks in advance!

Ben F.

Hi Ben,

First, thank you for such a detailed, yet grok’able report.

Second - I don’t agree with your assessment that this is working
incorrectly

Unless I’m misreading your assessment, I’m reading that you expect
“should
include(*args)” to mean “should include ALL of these args” whereas
“should_not include(*args)” means “should not include ANY of these
args.”

Make sense?

IMO, I think this is a documentation problem and that it is neither in
need
of an exception, nor in need of a fix.

That said, perhaps we need a couple of matchers: include_all and
include_any, with include being an alias for include_all. That would
certainly clear up the confusion. WDYT?

Cheers,
David

killswitchcollectiv · January 7, 2010, 8:17pm

On Thu, Jan 7, 2010 at 12:32 PM, Ben F.
[email protected]wrote:

Yep.

If this is indeed what you are suggesting, I think this would be great! Not
only does it add the functionality we are looking for but the method names
are self documenting and much easier to understand what functionality to
expect from each one.

Cool. I’m not sure when I’ll get to this, but I’m pretty sure this would
work for you for now (untested):

Spec::Matchers.define :include_all do |*expected|
match do |actual|
expected.all? {|e| actual.include?(e)}
end
end

Spec::Matchers.define :include_any do |*expected|
match do |actual|
expected.any? {|e| actual.include?(e)}
end
end

Cheers,
David

killswitchcollectiv · January 7, 2010, 8:25pm

On 7 Jan 2010, at 19:15, David C. wrote:

expected.any? {|e| actual.include?(e)}

end
end

Hi Ben/David

If it’s any help, I’ve been using three extra matchers for a while now:

include_all_once
include_all_at_least_times
include_any

I’ve put them in a gist[1], as the code to produce the error messages
makes them a bit verbose.

HTH
Ashley

[1] include_all_at_least_times.rb · GitHub

–
http://www.patchspace.co.uk/
http://www.linkedin.com/in/ashleymoran

killswitchcollectiv · January 7, 2010, 7:41pm

Thanks for looking at this David,

Just so I am clear on what you are suggesting:

renaming the “include” matcher to “include_all”

a. create an alias method for “include” for backwards compatibility

create a new matcher called “include_any”

If this is indeed what you are suggesting, I think this would be great!
Not
only does it add the functionality we are looking for but the method
names
are self documenting and much easier to understand what functionality to
expect from each one.

Ben F.

From: [email protected]
[mailto:[email protected]] On Behalf Of David C.
Sent: Thursday, January 07, 2010 1:19 AM
To: rspec-users
Subject: Re: [rspec-users] Problem with should_not when passing a
multipleargs to a matcher

On Wed, Jan 6, 2010 at 7:15 PM, Ben F. [email protected]
wrote:

We seem to be hitting some undesirable behavior with should_not in
combination with matchers that accept collections. Let me use the
“include”
matcher for example (a co-worker reported similar problems using
“be_any” so
I don’t believe this is limited to “include”). The RDoc states:

Passes if actual includes expected. This works for collections and
Strings.
You can also pass in multiple args and it will only pass if all args are
found in collection.

Examples

[1,2,3].should include(3)

[1,2,3].should include(2,3) #would pass

[1,2,3].should include(2,3,4) #would fail

[1,2,3].should_not include(4)

“spread”.should include(“read”)

“spread”.should_not include(“red”)

The RDoc doesn’t give any examples of using a “should_not” along with
multiple args being passed to “include”, but since I couldn’t find any
documentation on why it would not work I assume that it should work.
Here is
a list of tests I came up with to find out what works and what doesn’t.
Note: All of these tests SHOULD fail; the problem is with the tests that
do
NOT fail.

it "does properly fail on a collection when using \"should\" with

multiple args" do

  [1,2,3].should include(2,3,4)

end



it "does NOT properly fail on a collection when using \"should_not\"

with multiple args when at least one value in the expected args is in
the
actual collection" do

  [1,2,3].should_not include(3,4,5)

end



it "does properly fail on a collection when using \"should_not\"

with
multiple args when all of the values in the expected args are in the
actual
collection" do

  [1,2,3].should_not include(1,2,3)

end



it "does properly fail on a collection when using \"should_not\"

with
multiple args when all of the values in the expected args are in the
actual
collection and when the expected args don’t cover all of the values in
the
actual collection" do

  [1,2,3].should_not include(1,2)

end



it "does properly fail on a collection when using \"should_not\"

with a
single arg" do

  [1,2,3].should_not include(3)

end



it "does properly fail on a string when using \"should_not\" with

multiple args when all of the values in the expected args are in the
actual
string" do

  "abc".should_not include("a", "b", "c")

end



it "does NOT properly fail when using \"should_not\" with multiple

args
when at least one of the values in the expected args is in the actual
string" do

  "abc".should_not include("a", "b", "e")

end



it "does properly fail on a string when using \"should_not\" with

multiple args when all of the values in the expected args are in the
actual
collection and when the expected args don’t cover all of the values in
the
actual string" do

  "abc".should_not include("a","b")

end



it "does properly fail on a string when using \"should_not\" with a

single arg" do

  "abc".should_not include("a")

end

We are using the following rspec gems:

Rspec-1.2.9

Rspec-rails-1.2.7.1

So I think one of two things needs to happen here dependent on whether
this
is desired behavior or not.

If this is desired behavior then an exception really should be raised
if
you try to use “should_not” in combination with passing a collection to
a
matcher. At the very least there needs to be some documentation to
inform
RSpec users that tests will not always fail as they might expect when
using
“should_not” in combination with a matcher that accepts multiple args.
If this is not desired behavior then of course we need a fix.

I apologize if this has been brought up previously; I did as much
searching
on this issue as possible.

Thanks in advance!

Ben F.

Hi Ben,

First, thank you for such a detailed, yet grok’able report.

Second - I don’t agree with your assessment that this is working
incorrectly

Unless I’m misreading your assessment, I’m reading that you expect
“should
include(*args)” to mean “should include ALL of these args” whereas
“should_not include(*args)” means “should not include ANY of these
args.”

Make sense?

IMO, I think this is a documentation problem and that it is neither in
need
of an exception, nor in need of a fix.

That said, perhaps we need a couple of matchers: include_all and
include_any, with include being an alias for include_all. That would
certainly clear up the confusion. WDYT?

Cheers,

David