after_(build|create|stub) Callbacks

169 vistas
Ir al primer mensaje no leído

fowlduck

no leída,
10 oct 2009, 1:29:38 a. m.10/10/09
para factory_girl
I've added after_build, after_create, and after_stub callbacks to
factory_girl. Give it a shot and let me know what you think.

http://github.com/nate/factory_girl/commits/callbacks

As I said on the related issue [ http://github.com/thoughtbot/factory_girl/issues#issue/5
] I didn't add documentation. If it gets accepted I'd gladly add it.

Nate aka fowlduck

Dan Croak

no leída,
10 oct 2009, 1:37:37 p. m.10/10/09
para factory_girl
Nate,

Your callbacks branch looks good to me. Well-written, well-tested.
Thank you for taking the time to write it and share it with others.

I know callbacks have been talked about for Factory Girl since it was
released and there has been resistance to including them. I can't
remember why. I want to say lack of compelling use cases?

I'll bring this up to the other thoughtbot guys on Monday.

For (mostly my) reference, the cases laid out so far are:

verified_user case:

http://groups.google.com/group/factory_girl/browse_thread/thread/f649d226bc4c0735/5ac1631fcd904081?lnk=gst&q=callback#5ac1631fcd904081

has_many while respecting build strategy case:

http://groups.google.com/group/factory_girl/browse_thread/thread/9059470e3092e2b8

I haven't personally been clamoring for FG callbacks, but thinking
about how I would alternately handle the cases, it seems like the
callbacks could be a little cleaner.

Alternate approach to the verified_user example:

Set up a Factory with the state you expect:

Factory :verified_user, :parent => :user do |user|
user.verified { true }
end

Or however the state is handled. An advantage of this approach is
allowing the programmer to see the object in the desired state. An
advantage of the after_build strategy is it better encapsulates "how"
the object gets to that state (#register and #verify methods).

Alternate approach to the has_many example:

user = Factory(:user)
items = [Factory(:item, :user => user)]

Nate,

With the after_build approach, do you want to be able to build both
user and the user's items? Is the goal to save database writes and
increase test speed?

Dan

fowlduck

no leída,
10 oct 2009, 3:52:29 p. m.10/10/09
para factory_girl
Hi Dan,

Thanks for taking a look, I know many people, including myself, really
want callbacks in FG.

The goal of callbacks is two-fold:

(1) make your factories more expressive and therefore make your tests
easier to read
(2) treat your models like they would be treated in the application

(1): Currently there is no way to do has_many in factory_girl. Instead
of using `Factory(:user_with_expensive_items)` you must construct the
relations in the setup portion of your test or create a helper to do
it. This puts logic, that you shouldn't need to grok to understand the
test, into the setup block or sticks that logic in yet another
location. I believe callbacks help encapsulate the purpose of the
object graph you're constructing better and thereby simplify your
tests and make them more readable.

We already do this with other attributes giving us `Factory
(:administrative_user)`, `Factory(:delinquent_account)`, etc, but are
currently unable to do this with has_many relationships. Callbacks
give us this. Heck, we can even do this with belongs_to right now. Why
not with has_many?

(2): By adjusting the state (as in state-machine) of your model the
way your application would you arrive at the same state (as in
attributes) your application would, no surprises. This may make your
factories less brittle, allowing you to change what adjusting the
state (as in state machine) does to your model state (as in
attributes) without having to fix factories, but instead fix just
tests.

I'm more certain of the first point, less of the second. The first
point is the entire reason I wrote this extension, the second is one
that I've heard from others.

Thanks for your time and consideration.

Nate aka fowlduck

On Oct 10, 12:37 pm, Dan Croak <dcr...@thoughtbot.com> wrote:
> Nate,
>
> Your callbacks branch looks good to me. Well-written, well-tested.
> Thank you for taking the time to write it and share it with others.
>
> I know callbacks have been talked about for Factory Girl since it was
> released and there has been resistance to including them. I can't
> remember why. I want to say lack of compelling use cases?
>
> I'll bring this up to the other thoughtbot guys on Monday.
>
> For (mostly my) reference, the cases laid out so far are:
>
> verified_user case:
>
> http://groups.google.com/group/factory_girl/browse_thread/thread/f649...
>
> has_many while respecting build strategy case:
>
> http://groups.google.com/group/factory_girl/browse_thread/thread/9059...

Max

no leída,
10 oct 2009, 5:14:37 p. m.10/10/09
para factory_girl
+1 to fowlduck's patch

I'd like to support model state argument a bit. All proper non-
association-related cases can be covered by setting attributes
manually. The problem is — sometimes you end up breaking single-
authority. For example method activate! is supposed to do whatever to
activate the user. Currently you can do it by:

1) Setting activated_at to certain time in factory. This creates
second authority. Now you have two places to maintain what "activate"
means.
2) Creating a helper method which calls activate!. Now you're
splitting responsibility of defining model state into two locations.
IMO, this is unnecessary.

I think it's very beneficial to provide this "window" from factory
declaration to model logic to preserve single authority and maintain
SRP.

Murray Steele

no leída,
12 oct 2009, 6:39:10 a. m.12/10/09
para factor...@googlegroups.com
Good work fowlduck!

The one thing I'd add is to put in a before_create as well. It's been
a while since I did this (and I now work somewhere else so don't have
the factories / tests that used it to hand) but it was definitely
useful. There was stuff we wanted to do to an object and it's
associations before we created it, that we didn't neccessarily care /
need to do to it when we were just building it. Probably to do with
stubbing.

Cheers,

Murray

2009/10/10 Max <m...@bitsonnet.com>:

Nathan Sutton

no leída,
12 oct 2009, 9:33:19 a. m.12/10/09
para factor...@googlegroups.com
after_build happens before create :)

Nate Sutton

On Oct 12, 2009, at 5:39 AM, Murray Steele <murray...@gmail.com>
wrote:

Murray Steele

no leída,
12 oct 2009, 12:31:30 p. m.12/10/09
para factor...@googlegroups.com
True, but there was definitely something we needed to do before we
created an object, that we didn't *always* need to do after we built
it. Like how there are after_validate and before_save in AR, yes,
after_validate happens before you save, but it's not the same thing as
before_save because they happen at a different points in the object
save cycle (and one can be invoked without the other).

That said, it's definitely not a biggie, and if it's going to go in,
the lack of a before_create isn't something that should hold it up
this time.

2009/10/12 Nathan Sutton <nathan...@gmail.com>:

Nathan Sutton

no leída,
12 oct 2009, 1:11:28 p. m.12/10/09
para factor...@googlegroups.com
That's an excellent point. It's easy enough to add more callbacks with
this framework for callbacks in place.

Nate aka fowlduck

> --~--~---------~--~----~------------~-------~--~----~
> Individuals over processes. Interactions over tools. Agile Rails
> training from thoughtbot, the makers of Clearance, Shoulda, &
> Factory Girl:
> http://thoughtbot.com/services/training
>
> You received this message because you are subscribed to the
> "factory_girl" mailing list.
> To post to this group, send email to factor...@googlegroups.com
> To unsubscribe from this group, send email to
> factory_girl...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/factory_girl?hl=en
> -~----------~----~----~----~------~----~------~--~---
>

Chad Pytel

no leída,
16 oct 2009, 4:40:10 p. m.16/10/09
para factory_girl
I just merged this and bumped the gem version to 1.2.3. We should
work on updating the docs now. Would nice to do it before, but I
didn't want to wait to push this out because we needed it.

Thanks for your contribution, Nate.
-Chad

On Oct 12, 1:11 pm, Nathan Sutton <nathan.sut...@gmail.com> wrote:
> That's an excellent point. It's easy enough to add more callbacks with  
> this framework for callbacks in place.
>
> Nate aka fowlduck
>
> On Oct 12, 2009, at 11:31 AM, Murray Steele wrote:
>
>
>
>
>
> > True, but there was definitely something we needed to do before we
> > created an object, that we didn't *always* need to do after we built
> > it.  Like how there are after_validate and before_save in AR, yes,
> > after_validate happens before you save, but it's not the same thing as
> > before_save because they happen at a different points in the object
> > save cycle (and one can be invoked without the other).
>
> > That said, it's definitely not a biggie, and if it's going to go in,
> > the lack of a before_create isn't something that should hold it up
> > this time.
>
> > 2009/10/12 Nathan Sutton <nathan.sut...@gmail.com>:
>
> >> after_build happens before create :)
>
> >> Nate Sutton
>
> >> On Oct 12, 2009, at 5:39 AM, Murray Steele <murray.ste...@gmail.com>
>  smime.p7s
> 5KViewDownload

Nathan Sutton

no leída,
16 oct 2009, 4:42:38 p. m.16/10/09
para factor...@googlegroups.com
Not a problem, glad to help out a great project. I can take a look at
the docs tonight and make a first effort.

Nate

max

no leída,
30 oct 2009, 6:15:22 p. m.30/10/09
para factory_girl
I am just starting with factory girl and would like to use these
callbacks. Do you know if they will be available using from
gems.github.com? Does the fact that they are not published there
indicate that they are not part of the official release?

Thanks,
Max
>  smime.p7s
> 5KViewDownload

Nathan Sutton

no leída,
30 oct 2009, 6:37:54 p. m.30/10/09
para factor...@googlegroups.com
Gem publishing on github has ceased, use gemcutter instead:

http://github.com/blog/515-gem-building-is-defunct

http://gemcutter.org/

This patch has been accepted and is now in factory_girl 1.2.3

Nate

Responder a todos
Responder al autor
Reenviar
0 mensajes nuevos