One test affects another

I have the following class:

class Brewer < ActiveRecord::Base
class EmailNotFound < StandardError; end
class << self
def find_by_email(*args)
b = super(*args)
raise EmailNotFound if b.nil?
return b
end
end
end

And the following two tests:

require File.dirname(FILE) + ‘/…/test_helper’

class BrewerTest < Test::Unit::TestCase
fixtures :brewers

def test_should_find_brewer_with_valid_email
assert_equal brewers(:active_brewer),
Brewer.find_by_email(brewers(:active_brewer).email)
end

def test_should_raise_email_not_found_exception_with_invalid_email
assert_raises Brewer::EmailNotFound do
Brewer.find_by_email(‘NotAnEmail’)
end
end
end

The tests fail like this:

Loaded suite test/unit/brewer_test
Started
.F
Finished in 0.428367 seconds.

  1. Failure:
    test_should_raise_email_not_found_exception_with_invalid_email(BrewerTest)
    [test/unit/brewer_test.rb:207]:
    Brewer::EmailNotFound exception expected but none was thrown.

2 tests, 2 assertions, 1 failures, 0 errors

If I comment out test_should_find_brewer_with_valid_email, the other
test passes.

What’s going on here?

Regards,
–Dean

Hi –

On Sun, 17 Feb 2008, Dean wrote:

end
def test_should_find_brewer_with_valid_email

Brewer::EmailNotFound exception expected but none was thrown.

2 tests, 2 assertions, 1 failures, 0 errors

If I comment out test_should_find_brewer_with_valid_email, the other
test passes.

What’s going on here?

What I think is happening is this:

When you call super on #find_by_email, ActiveRecord goes ahead and
creates its own #find_by_email dynamically. That dynamically-created
method then takes over: when the second test runs, it uses that
version, not the one you wrote.

Try changing your find_by_email to, say, get_by_email, and change
super to find_by_email. That may not be a permanent fix but it should
show what’s going on.

David


Upcoming Rails training from David A. Black and Ruby Power and Light:
ADVANCING WITH RAILS, April 14-17 2008, New York City
CORE RAILS, June 24-27 2008, London (Skills Matter)
Stay tuned for details, and dates in Berlin!

Dean wrote:

I have the following class:

class Brewer < ActiveRecord::Base
class EmailNotFound < StandardError; end
class << self
def find_by_email(*args)
b = super(*args)

I set up an experiment like yours, and couldn’t get the equivalent of
‘find_by_email’ to fail. The first test case found its model, and the
second one
took an invalid ID and raised an error.

Architecturally, I’m not sure why you’d raise, over letting
‘find_by_email’
return ‘nil’, and detecting this. Maybe you intend to use the Samurai
Principle

  • either return victorious or don’t return at all.

But I’m too superstitious to want to call ‘super’ inside a method that’s
generated automatically via ‘method_missing’. Because your find_by
disregards
the standard ActiveRecord contract for other find_by methods, why not
rename it
‘fetch_by_email’, then put the ‘find_by_email’ and the ‘if’ inside it.

So if you had a dog named Samurai, and you said “fetch”, then…


Phlip
http://assert2.rubyforge.org/

David A. Black wrote:

When you call super on #find_by_email, ActiveRecord goes ahead and
creates its own #find_by_email dynamically. That dynamically-created
method then takes over: when the second test runs, it uses that
version, not the one you wrote.

I thought about that, but the source to method_missing does not “create
a
method” - but that’s possible. It just builds the complete find() setup
for you
and then calls this.

So where would a super route out of such a concoction?

Try changing your find_by_email to, say, get_by_email, and change
super to find_by_email. That may not be a permanent fix but it should
show what’s going on.

In terms of style it’s a permanent fix, because things with the same
name should
have the same contract!


Phlip

Hi –

On Sun, 17 Feb 2008, Phlip wrote:

and then calls this.
method_missing does create the method. Here’s a relevant excerpt:

def method_missing(method_id, *arguments)
# …
self.class_eval %{
def self.#{method_id}(*args)

And a little demo:

Loading development environment (Rails 2.0.2)

Auction.method(:find_by_title)
NameError: undefined method find_by_title' for class Class’
Auction.find_by_title(“blah”)
=> nil
Auction.method(:find_by_title)
=> #<Method: Auction(id: integer, title: string, start_date: datetime,
…)

And the RDoc comments in base.rb:

     # Each dynamic finder or initializer/creator is also defined
     # in the class after it is first invoked, so that future
     # attempts to use it do not run through method_missing.

So I’m going to stick with my original interpretation of the OP’s
result :slight_smile:

David


Upcoming Rails training from David A. Black and Ruby Power and Light:
ADVANCING WITH RAILS, April 14-17 2008, New York City
CORE RAILS, June 24-27 2008, London (Skills Matter)
See http://www.rubypal.com for details, and stay
tuned for dates in Berlin!

On 18 Feb 2008, at 13:00, David A. Black wrote:

=> #<Method: Auction(id: integer, title: string, start_date: datetime,

This was new in rails 2.0 - 1.x always hit method_missing

Fred

On Feb 17, 9:04 pm, Phlip [email protected] wrote:

‘find_by_email’ to fail. The first test case found its model, and the second one
took an invalid ID and raised an error.

Architecturally, I’m not sure why you’d raise, over letting ‘find_by_email’
return ‘nil’, and detecting this. Maybe you intend to use the Samurai Principle

  • either return victorious or don’t return at all.

It’s for concise controllers:

8 def activate
9 self.current_brewer = Brewer.activate_with_activation_code
params[:activation_code]
10 flash[:notice] = “Signup complete!”
11 redirect_to brewer_path(current_brewer)
12 rescue Brewer::ActivationCodeNotFound
13 flash[:error] = “Could not find that activation code”
14 redirect_to(activate_path)
15 end

Rather than using an if brewer.nil? branch.

But I’m too superstitious to want to call ‘super’ inside a method that’s
generated automatically via ‘method_missing’. Because your find_by disregards
the standard ActiveRecord contract for other find_by methods, why not rename it
‘fetch_by_email’, then put the ‘find_by_email’ and the ‘if’ inside it.

I had forgot that find_by_email was created by method_missing. I’ve
discarded the idea for an if; then; else thing.

So if you had a dog named Samurai, and you said “fetch”, then…

(-:

Thanks for the replies everyone.
–Dean

Hi –

On Mon, 18 Feb 2008, Frederick C. wrote:

On 18 Feb 2008, at 13:00, David A. Black wrote:

So I’m going to stick with my original interpretation of the OP’s
result :slight_smile:

This was new in rails 2.0 - 1.x always hit method_missing

Yes – my interpretation of his result includes the inference that
he’s using 2.0 :slight_smile:

David


Upcoming Rails training from David A. Black and Ruby Power and Light:
ADVANCING WITH RAILS, April 14-17 2008, New York City
CORE RAILS, June 24-27 2008, London (Skills Matter)
See http://www.rubypal.com for details, and stay
tuned for dates in Berlin!

A better choice might be find_by_email! (the “dangerous” twin of
find_by_email).

D’oh!

Hi –

On Sun, 17 Feb 2008, Phlip wrote:

David A. Black wrote:

Try changing your find_by_email to, say, get_by_email, and change
super to find_by_email. That may not be a permanent fix but it should
show what’s going on.

In terms of style it’s a permanent fix, because things with the same name should
have the same contract!

Maybe, though I don’t like the idea of having get_ and find_ as
markers of difference, since they’re sort of arbitrary. It’s the same
problem as instance_eval and instance_exec; they indicate slightly
different methods, but there’s nothing in the method names that tells
you which is which.

A better choice might be find_by_email! (the “dangerous” twin of
find_by_email).

David


Upcoming Rails training from David A. Black and Ruby Power and Light:
ADVANCING WITH RAILS, April 14-17 2008, New York City
CORE RAILS, June 24-27 2008, London (Skills Matter)
See http://www.rubypal.com for details, and stay
tuned for dates in Berlin!