google / auto

A collection of source code generators for Java.
Apache License 2.0
10.42k stars 1.2k forks source link

AutoFactory: factories are final, prevent use of Mockito #99

Closed cgrushko closed 5 years ago

cgrushko commented 10 years ago

Generated factories are final, which means they cannot be mocked using Mockito. Even though they're designed for injection, passing a mock of such factory is so much easier than providing a test module and messing around with the DI's graph.

gk5885 commented 10 years ago

The factories are intended to be final and if mocking is desired, an interface should be used.

cgrushko commented 10 years ago

I actually like the way interfaces make the factory explicit, but using them means writing a module (or is it generated? I couldn't figure it out. At any rate, I still have to register it). I might end up with more boilerplate than I tried to avoid :)

cgruber commented 10 years ago

I had a whole plan for auto-generating a dagger module you could include that was in the auto factory prototype I worked up way back. I could probably resuscitate it, but I'm not sure we want to have it in Auto, since it's integration with a specific DI container, and AutoFactory can be used wherever. At any rate, it's a three liner:

@Provided FooFactory provideFooFactory(FooFactoryImpl impl) { return impl; }

alicederyn commented 8 years ago

What was the result of this discussion at Google?

If this issue is still open for discussion, I'd like to note that declaring classes final was only listed as the easier approach to prohibiting extension in Effective Java; making constructors private is also valid. In this case, the unfortunate side-effect of declaring this generated API final is that you discourage people from using AutoFactory when they find it's final. (Real-life examples are available on request.) The work-around is not obvious, and adds many lines of unnecessary boiler-plate that needs to be kept in-sync with other code. Unfortunately it seems like Daggers † and ‡ don't support @Inject-annotated static factory methods, which presumably would mean they would need changes, too (and I have no idea how controversial that would be).

If someone's able to clarify the state of all this, that'd be fab.

alicederyn commented 8 years ago

Oh, and I've just realised there's an allowSubclasses property on @AutoFactory that presumably removes the final keyword. So, I guess this can be closed? Unless there was a plan to use @Inject-annotated static factory methods even when allowSubclasses was false?

tbroyer commented 8 years ago

This was fixed in e0e2394ec33cb3d58e39fa142f138fefd517b1a3 BTW; and it's properly documented in the javadoc https://static.javadoc.io/com.google.auto.factory/auto-factory/1.0-beta3/com/google/auto/factory/AutoFactory.html#allowSubclasses-- ;-)

emartynov commented 8 years ago

Do I understand correctly, that I want to mock factories I should use allowSubclasses = true?