[RSpec] implicit receiver for should problem when helper predicate methods are in use


#1

I just upgraded RSpec and noticed that some of my specs started to
fail and it is caused by the 1.1.2 update where implicit receiver for
should is introduced.

Here is one sample example group, which works with older versions
naturally.

describe “example group” do
it “example” do
should be_ok
end

def ok?
true
end
end

And gets NoMethodError of course. I tried different things to set as
subject, but was unlucky. For example, subject {self} caused
SystemStackError although I hoped that this does the trick, since self
has method ok? as expected.

I also noticed that there is accessor for subject, so I tried
something like this instead of def ok?:

def subject.ok?
true
end

Why doesn’t it work? It works when doing like this:
s = “str”
def s.ok?; true; end
s.ok? # true

I found only one way to fix them like this:
class String
def ok?
true
end
end

Of course I’m not happy with that solution :slight_smile:

What are my options to fix these specs? It seems to me that most
logical functionality to this problem would be to make the subject
{self} line to work as expected.

Jarmo


#2

Okay, I fiddled around a little and made this solution:
module Spec
module Example
module Subject
module ExampleMethods

    def should(matcher=nil)
      if matcher
        self == subject ?

Spec::Expectations::PositiveExpectationHandler.handle_matcher(self,
matcher) : subject.should(matcher)
else
self == subject ?
Spec::Expectations::PositiveExpectationHandler.handle_matcher(self) :
subject.should
end
end

    def should_not(matcher=nil)
      if matcher
        self == subject ?

Spec::Expectations::NegativeExpectationHandler.handle_matcher(self,
matcher) : subject.should_not(matcher)
else
self == subject ?
Spec::Expectations::NegativeExpectationHandler.handle_matcher(self) :
subject.should_not
end
end
end
end
end
end

In short, if subject is set as self then I’m calling Kernel::should
method’s body directly. I don’t know if there’s any side effects that
I’m omitting block parameter (although original should and should_not
also doesn’t send any blocks).

Any problems that might arise with this patch?

Also, if my original example wasn’t real life example enough, then I
will give another one:
require ‘watir’

describe “Google” do
subject {self}

before :all do
@b = Watir::Browser.new
@b.gotohttp://www.google.com
end

it “has google written on page” do
should have_text(“Google”)
end

def has_text? text
@b.text.include?(text)
end

end

PS! I changed my poster nick, but I’m still the same “juuser” who made
original thread.

Regards,
Jarmo


#3

On Thu, May 7, 2009 at 1:34 PM, Jarmo P. removed_email_address@domain.invalid wrote:

matcher) : subject.should(matcher)
Spec::Expectations::NegativeExpectationHandler.handle_matcher(self,
end

In short, if subject is set as self then I’m calling Kernel::should
method’s body directly. I don’t know if there’s any side effects that
I’m omitting block parameter (although original should and should_not
also doesn’t send any blocks).

Any problems that might arise with this patch?

The first problem is the dependency on subject, which is a construct
from rspec’s example groups. This would make rspec’s matchers unusable
outside rspec.

I’m also not clear on what your goal is, per my earlier response.
Please help me understand.


#4

On Thu, May 7, 2009 at 11:06 PM, Jarmo P. removed_email_address@domain.invalid
wrote:

I think that I don’t understand what you mean by that exactly. I mean,
I patched the should and should_not methods in moule
Subject::ExampleMethods themselves, which means that this changes only
behaviour of subject itself e.g. it is already part of RSpec and don’t
see how it would break matchers outside of RSpec. Please correct me if
i’m wrong.

You’re correct. I misunderstood.

end
$browser = Browser.new

Something like this. Do You see where I’m going? Or am I doing
something what I ain’t suppose to do ever? Should I just make
WatirHelperMethods as a class instead of module so it would work like
this: helper = HelperClass.new($browser); helper.login “testuser”…?
Doesn’t remove much of a verboseness also.

This is pretty helpful, thanks for a more thorough explanation. I did
add your patch locally, however, and it doesn’t seem to solve the
problem (I still get undefined method `ok?’ when I run the example in
your 2nd post).

I need to think about this some more before I agree to go in this
direction, as I want to make sure there are no negative impacts that
I’m not thinking of right now. That said, I’d need a patch w/ specs
that fail without this change and pass with this change. Please file a
ticket at http://rspec.lighthouseapp.com w/ a patch.

Cheers,
David

I have also another question: why the should and should_not methods in
Spec::Example::Subject::ExampleMethods have this if statement anyway?
I mean, the matcher argument has it’s default value as nil and it is
also as nil on Kernel#should(_not) method so this if doesn’t make
sense to me. Can’t it just be written as subject.should(matcher)? Or
is it just some code block which is written like that because of some
3rd party tools?

Good point - thanks! :
http://github.com/dchelimsky/rspec/commit/b061f9f40dc8a29d34ddb3e74427b244bbcf0aa8


#5

I think that I don’t understand what you mean by that exactly. I mean,
I patched the should and should_not methods in moule
Subject::ExampleMethods themselves, which means that this changes only
behaviour of subject itself e.g. it is already part of RSpec and don’t
see how it would break matchers outside of RSpec. Please correct me if
i’m wrong.

Anyway, wasn’t that Watir example good enough? I think that You’re
right when You think that this thing should not be needed when testing
Ruby code with RSpec, but as soon as I start using Watir or some
similar tool, then I need to write bunch of helper methods to make my
specs less verbose. Let me try to make it more clear.

module WatirHelperMethods
def has_text? text
text.is_a?(Regexp) ? $browser.text =~ text :
$browser.text.include?(text)
end

def login user_id
  # do something so user would be logged in
end

def logged_in? user_id
  # return true if user is logged in
end

def start_browser_on_url url
  $browser = Browser.new
  $browser.goto url
end

end

describe “my test” do
include WatirHelperMethods

before :all do
  start_browser_on_url "http://url"
end

it "should log user into application" do
  login "testuser"
  should be_logged_in
  should have_text("Welcome testuser")

end

after :all do
$browser.close
end
end

Something like this. Do You see where I’m going? Or am I doing
something what I ain’t suppose to do ever? Should I just make
WatirHelperMethods as a class instead of module so it would work like
this: helper = HelperClass.new($browser); helper.login “testuser”…?
Doesn’t remove much of a verboseness also.

I have also another question: why the should and should_not methods in
Spec::Example::Subject::ExampleMethods have this if statement anyway?
I mean, the matcher argument has it’s default value as nil and it is
also as nil on Kernel#should(_not) method so this if doesn’t make
sense to me. Can’t it just be written as subject.should(matcher)? Or
is it just some code block which is written like that because of some
3rd party tools?

Jarmo


#6

On May 8, 10:50 am, David C. removed_email_address@domain.invalid wrote:

This is pretty helpful, thanks for a more thorough explanation. I did
add your patch locally, however, and it doesn’t seem to solve the
problem (I still get undefined method `ok?’ when I run the example in
your 2nd post).
Sorry. It didn’t work because I didn’t have subject {self} there.

I need to think about this some more before I agree to go in this
direction, as I want to make sure there are no negative impacts that
I’m not thinking of right now. That said, I’d need a patch w/ specs
that fail without this change and pass with this change. Please file a
ticket athttp://rspec.lighthouseapp.comw/ a patch.
Ok, will do that later.

3rd party tools?

Good point - thanks! :http://github.com/dchelimsky/rspec/commit/b061f9f40dc8a29d34ddb3e7442
:slight_smile:

Cheers,
Jarmo


#7

Thanks for adding this change into repo!
I’m quite surprised that anyone else haven’t stumbled upon this
problem yet. I guess it’s because most of spec’ing is done for Ruby
projects, so this functionality is not needed.

I have been thinking a little more about this topic and asked myself:
why not make it backwards compatible without explicitly stating
subject as ‘self’? If not making it backwards compatible then all my
current specs which use Watir would have to be changed by adding
“subject {self}” to them.

Why just not use ExampleGroup as a receiver for #should when string is
used in ExampleGroup description? I cannot see any negative impacts on
that change and rake specs are also passing. Any ideas where this
might fail? There probably aren’t any specs where description is
written as a string and then you want to invoke matchers against that
string… Doesn’t seem logical to me.

So, I’ve changed locally should method in
Spec::Example::Subject::ExampleMethods from:

    def should(matcher=nil)
      self == subject ? self.__should_for_example_group__

(matcher) : subject.should(matcher)
end

to:
def should(matcher=nil)
if self == subject || (subject.is_a?(String) && subject ==
instance_eval(&self.class.subject))
self.should_for_example_group(matcher)
else
subject.should(matcher)
end
end

I’ve added that “&& subject == instance_eval(&self.class.subject)”
part as an extra precaution, but i don’t have any ideas, why it’s
needed anyway, because I cannot think of any cases where string is
used as a subject… or maybe there are some libraries, which monkey-
patch String and want to call #should implicitly?! :slight_smile:

in short, at the moment this seems to work also (rake specs are
passing again):
def should(matcher=nil)
if self == subject || subject.is_a?(String)
self.should_for_example_group(matcher)
else
subject.should(matcher)
end
end

If you think that this functionality might be reasonable for wider use
(some other Watir or similar library users for example), then I can
provide a patch with specs.

Regards,
Jarmo


#8

Added patch and specs to
https://rspec.lighthouseapp.com/projects/5645-rspec/tickets/816-possibility-to-use-self-as-a-subject-for-explicit-receiver

Jarmo


#9

On May 12, 6:21 am, David C. removed_email_address@domain.invalid wrote:

I don’t want to promote using self as subject. I think being able to
do so explicitly, as we can now with your previous patch, is perfectly
reasonable. But doing so is a bit of a trick in my view, and runs
counter to the overall intent of the structure of examples of
behaviour of an object or sub-system. “should” is about that object or
sub-system, not self.
I’m also thinking that maybe I’m doing something wrong when using this
approach, but cannot think of any better way. Do you have any
suggestions how to get rid of using self as a subject in my context?
Maybe i’m not aware of some best practices when it comes to using
helper methods. Should I monkey-patch Watir (or some other library) to
add methods to it like has_text? and so on in which case subject would
be $browser (which is actually correct)?

If your whole suite is using this, you can localize the “subject
{self}” call, however, in spec_helper, like this:

class Spec::ExampleGroup
subject {self}
end
Thank you for this great tip! I think that this solution will satisfy
me.

HTH,
David

Jarmo


#10

On Wed, May 13, 2009 at 11:02 AM, Jarmo P. removed_email_address@domain.invalid
wrote:

suggestions how to get rid of using self as a subject in my context?
Maybe i’m not aware of some best practices when it comes to using
helper methods. Should I monkey-patch Watir (or some other library) to
add methods to it like has_text? and so on in which case subject would
be $browser (which is actually correct)?

I actually think that what you’re doing is perfectly fine. Just not
the common case. That’s why I think it’s OK to support explicitly
assigning self as subject, but not making that a default behaviour.

If it were me, and it’s not, but if it were, I’d wrap those helpers
in an object and make explicit calls.


#11

On Mon, May 11, 2009 at 4:30 PM, Jarmo P. removed_email_address@domain.invalid
wrote:

   def should(matcher=nil)
       subject.should(matcher)

passing again):
provide a patch with specs.
I don’t want to promote using self as subject. I think being able to
do so explicitly, as we can now with your previous patch, is perfectly
reasonable. But doing so is a bit of a trick in my view, and runs
counter to the overall intent of the structure of examples of
behaviour of an object or sub-system. “should” is about that object or
sub-system, not self.

If your whole suite is using this, you can localize the “subject
{self}” call, however, in spec_helper, like this:

class Spec::ExampleGroup
subject {self}
end

HTH,
David


#12

On May 13, 7:08 pm, David C. removed_email_address@domain.invalid wrote:

I actually think that what you’re doing is perfectly fine. Just not
the common case. That’s why I think it’s OK to support explicitly
assigning self as subject, but not making that a default behaviour.

If it were me, and it’s not, but if it were, I’d wrap those helpers
in an object and make explicit calls.

you mean something like this?

class PageHelper
def has_text?

blah

end
end

describe “cool page” do
subject {PageHelper.new}

before :all do
PageHelper.new
end

it “has something” do
should have_text(“blah”)
end

end

Doesn’t seem too bad. I will give it a thought!

Jarmo


#13

On Wed, May 13, 2009 at 11:02 AM, Jarmo P. removed_email_address@domain.invalid
wrote:

suggestions how to get rid of using self as a subject in my context?

class Spec::ExampleGroup
subject {self}
end
Thank you for this great tip! I think that this solution will satisfy
me.

Great!

Cheers,
David