should_have_named_scope rewrite

[Brian Cardarella]

Does anybody think this is worth anything?

http://github.com/thoughtbot/shoulda/issues#issue/60
(my patch is included in the issue)

Basically, my rewrite tests against an expected result for a named
scope. So instead of the now deprecated should_have_named_scope that
just matches the SQL syntax this patch will allow you to write out a
test along with the expected result and leave the implementation up to
you.

Includes both RSpec matchers and Test::Unit macros

The only thing I don't like is that you have to stringify the
expectation for Test::Unit if it is being declared in a setup block.
Kind of ugly IMO but I could not think of a another way.

[Dan Croak]

I dunno. I still want to test my named scopes in a state-based way.

Given some preconditions
When I execute this method
Then my expectations should be verified

Having a "better" named scope is, to me, a temptation to write worse tests.

How would the following named scope be tested using should_have_named_scope?

named_scope :unwatched, lambda { |user|
{ :joins => "left outer join viewings
on viewings.movie_id = movies.id
and viewings.user_id = #{user.id}",
:conditions => "users.id is null" }
}

My guess is I'd repeat the implementation in the :joins key of
should_named_scope, revealing no intention about the method.

Is there a different way to test this with your fork?

--
Dan Croak
@Croaky

[Brian Cardarella]

The state based expectations is exactly what this does.

So for RSpec:

RSpec:
describe Book
describe "Named Scope" do
before(:each) do
@expected = [Factory(:book, :name => "test")]
Factory(:book, :name => "blah")
Factory(:book, :name => "heynow")
@by_test_param = "test"
end

it { should have_named_scope(:test).to_return(@expected) }
it { should have_named_scope("by_name
(@by_test_param)").to_return(@expected) }
end
end

Or maybe I'm misunderstanding what you're getting at...

[Dan Croak]

If that's the case, it seems like a roundabout and potentially
error-prone way to do it more directly:

should "find by name" do

expected = [Factory(:book, :name => "test")]
Factory(:book, :name => "blah")
Factory(:book, :name => "heynow")

assert_equal expected, Book.test
assert_equal expected, Book.by_name("test")
end

--
Dan Croak
@Croaky

[Brian Cardarella]

If you accept having an error while you properly TDD then that is one
thing. But I believe that the test should fail first (not give an
error) if the given named scope doesn't exist. Then it should fail if
the expectation is not met.

Not to mention that the matcher and macro both give you a standardized
formatting to your failures and test names.

And I'm certain where the potential for error comes in over the 2nd
example.

[Dan Croak]

Brian,

On Tue, Sep 29, 2009 at 2:48 PM, Brian Cardarella <bcard...@gmail.com> wrote:

> If you accept having an error while you properly TDD then that is one
> thing. But I believe that the test should fail first (not give an
> error) if the given named scope doesn't exist. Then it should fail if
> the expectation is not met.

I'm not sure what you mean here. The usual flow is: error (no method
defined), error (wrong number of arguments), failure (wrong result),
some progression to green.

I don't see how it's different in this case.

> Not to mention that the matcher and macro both give you a standardized
> formatting to your failures and test names.

That's a valid point and something to strive for, but I don't like the
standard name this gives me "should have named scope" is an
implementation detail. It's like saying "should have class method" or
"should have instance method".

> And I'm certain where the potential for error comes in over the 2nd
> example.

Again, not sure I understand what you mean here.

--
Dan Croak
@Croaky

[Brian Cardarella]

TDD flow: Hmm. Yeah you're right about that. I guess testing the
existence of a named scope is too granular of a test. If it exists the
expectation test will pass anyway. Not only that but first testing the
existence of a named scope rules out the possibility of testing the
expectation of chained scopes wrapped into a single method.

I don't have an issue with the name. Back when Shoulda had macro names
that were diferent than the Rails DSL I found that confussing as I was
constantly looking up the appropriate macro name. But I suppose if it
was to be used for testing chained scopes as well that might not be
best to limit the name as 'should_have_named_scope'

On Sep 29, 3:27 pm, Dan Croak <dcr...@thoughtbot.com> wrote:
> Brian,
>

[Dan Croak]

Cool. Thanks for the conversation. I wrote up a blog post on this
because it came up on the training mailing list, too:

http://robots.thoughtbot.com/post/200254501/testing-named-scopes

On Tue, Sep 29, 2009 at 3:43 PM, Brian Cardarella <bcard...@gmail.com> wrote:
>
> TDD flow:  Hmm.  Yeah you're right about that. I guess testing the
> existence of a named scope is too granular of a test. If it exists the
> expectation test will pass anyway. Not only that but first testing the
> existence of a named scope rules out the possibility of testing the
> expectation of chained scopes wrapped into a single method.
>
> I don't have an issue with the name. Back when Shoulda had macro names
> that were diferent than the Rails DSL I found that confussing as I was
> constantly looking up the appropriate macro name. But I suppose if it
> was to be used for testing chained scopes as well that might not be
> best to limit the name as 'should_have_named_scope'

I meant naming more from the perspective of that RSpec doc output,
seeing the specification of a model in a nice format.

Side note: shoulda has a rake shoulda:list task that supposedly does
the same thing. I'd love to use it, but it doesn't appear to be
working (when Shoulda is installed as a gem in a Rails app). Not sure
the best approach around this, but might be a good patch.

--
Dan Croak
@Croaky