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
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.
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
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?
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.
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...
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!
> 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.
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...
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.
method_missing does create the method. Here's a relevant excerpt:
# 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
# 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
This was new in rails 2.0 - 1.x always hit method_missing
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).
