Testability of field injections

[tortmayr]

Currently almost all our default implementations use protected fields that are set from Guice via property injection. While filed injection is awesome for writing clean & simple code it makes it difficult to properly unit test the parent class.

To enable/improve testability I could think of the following approaches:

• Use constructor/method injection instead: While this makes the class easy to test it also exposes unnecessary public API (setters/constructors) which increases the risk of API breaks. (As soon as new property is added or an existing one is removed)

or stick with field inection and

• Provide empty constuctor & constructor to set the field injected properties: Basically the same as constructor/method injection. Exposes more public API and increases the potential for API breaks.
• Use reflection API for unit tests: Technically works and requires no changes to the class under test. However, reflection should only be used as a last resort and should be avoided if possible
• Use a dervied class from the class under test with the corresponding constructor for unit testing: Should work just fine for our uses cases because we always opt for protected over private fields. To me it feels like this is the best approach because we can use clean property injection in the tested class and only have to do one minor adpation when in the testing scope.

@planger @martin-fleck-at WDYT?

[planger]

Maybe package-private constructors with @inject or field injection with package-private constructors that also set the fields? This way we can simply unit test the classes and provide mocks as constructor arguments, but imho package private constructors aren't officially API?

[tortmayr]

Does package private work in this case? Since they tests are in a separate project I'd assume that the package private constructor is not visible

[planger]

Usually you put unit tests in a different project / folder (in m2), but in the same package. Then package private would work.

[tortmayr]

Sounds good to me

[CamilleLetavernier]

IMO, injected fields should be considered "Public API" anyway, because adding new properties would already break existing modules (at least when the property uses a new type).

In theory, dependency injection is similar to a constructor API, so it should be treated in the same way (And using public constructor injection makes the most sense in the case. It also allows supporting final fields, which is good). However, constructor injection makes it unnecessarily difficult to add sub-classes, as any property added to the parent constructor also needs to be added to all sub-class constructors.

So, in this case, setter methods are more flexible (Either protected or package-private, if you don't want to expose them in your "Java API", but still use them in subclasses and/or tests).

It's also relatively easy to setup a small Guice-injector in the tests (If the test is really a unit-test for a single class), and it can be combined with mocks.

I don't really have a strong preference either way. I would only advise against field-injection and always prefer either constructor- or setter-injection instead. While field-injection is more concise, it's also the least flexible option.