junit4 icon indicating copy to clipboard operation
junit4 copied to clipboard

Support the selection of a default Runner in ClassRequest

Open cesar1000 opened this issue 7 years ago • 7 comments

This patch updates Request and ClassRequest to enable the configuration of a default JUnit4 Runner other than BlockJUnit4ClassRunner. The only visible change is a new static method in Request, classWithDefaultRunner, which configures the runner class to use if not configured in the test.

The context of this change is the following: when managing a large test suite that gets contributions from a large development team, it's typical to enforce certain logic across all tests for global state initialization and cleanup. In the particular case of Twitter for Android, all our tests set up global application graphs before test execution and reset static state afterwards. It is critical for correctness that every single test follows this pattern. This seems to be a particular example of the general need for a mechanism that configures the global state of the test environment.

This is typically achieved by forcing all tests to use a single shared runner or to extend a base class. However, these approaches are error prone, since they require dilligence for setting up the test correctly, or an external validation (eg. a checkstyle detector). A default test runner is a low-tech solution to this problem: a test runner extending BlockJUnit4ClassRunner implements state management logic and is configured to be used with all tests that don't explicitly say otherwise.

#1219 proposes an alternative solution to a similar problem. I feel that this approach covers the general need in a simple, non-intrusive way.

The change is incomplete and doesn't implement tests, and it's meant a small proof of concept for feedback.

cesar1000 avatar Feb 27 '17 19:02 cesar1000

Thanks for the contribution.

My concern with this approach is that if you run tests in a way that won't call this new method (ex: from an IDE) you would get different results. This could lead to confusion. Having your tests explicitly specify a runner avoids this problem.

kcooney avatar Feb 27 '17 22:02 kcooney

Thanks, Kevin!

Yeah, that's a good point. Ideally, the information should be available in code. Would you consider something like looking up a @RunWith annotation applied to a package? In the case that the test is not annotated, the annotation runner would look up the chain of parent packages for the annotation. Implementers can then either add the package info file to the codebase or generate dynamically at compilation time - in either case, the configuration would be picked up by all test execution methods.

cesar1000 avatar Feb 28 '17 19:02 cesar1000

@cesar1000 see #670 for a related discussion.

One open question is what to do if your test class extends a test class in a different package and neither test class has @RunWith? My gut tells be you should use the ((great) grand) parent test clasa's package to find the runner because you are extending the base class, so should inherit the base class's runner (because the runner is part of the behavior of the base class).

kcooney avatar Feb 28 '17 20:02 kcooney

Ah, thanks for the pointer. Yeah, I feel that the meaning of @RunWith or @Ignore when applied to a package would be well defined. I can see the point that developers may not expect annotations in packages, but I think there are strong practical reasons why teams may decide to use them, since JUnit does not offer alternative tools for applying a certain configuration to a set of tests.

In regard to selecting a runner when extending a test class: I think we should always use the test's package. On the one hand, this is a simple and consistent rule that makes it easy to find the runner. On the other hand, I think it's the responsibility of the implementer of a test to ensure that the test runner is consistent with its parent's (as you would normally do if you were to annotate your test to use a different runner). Do you have any particular case in mind?

If this sounds good, I can throw a prototype together to see how that looks and we can discuss.

cesar1000 avatar Mar 01 '17 08:03 cesar1000

I hit a snag with using package annotations: it appears that Java will only initialize Package objects when first loading a class from the package. This is ok if annotating the package containing the tests, but won't work with parent packages (unless adding placeholder classes to these packages and loading them explicitly, but that's a bad hack). Annotating packages containing tests can be useful in itself, but it feels incomplete and error prone, since people are likely to expect this feature to work on any package. Alternatively, we could make the annotation apply only to the package itself and not subpackages, but this limits its usefulness.

An alternative may be to define a named class (JUnitPackageConfiguration or similar) and load that by name. The class would define annotations or other configuration parameters. Would this be an acceptable solution? Do you have any other ideas?

cesar1000 avatar Mar 05 '17 19:03 cesar1000

@cesar1000 I think a named package would be even less discoverable to a layperson than using package-level annotations.

kcooney avatar Mar 05 '17 20:03 kcooney

@kcooney yeah, I agree. I was just hoping for a more powerful solution, but I understand that discoverability is an important goal. It's a shame that Java support for package annotations is so inconsistent. Let me give it a bit more thought.

cesar1000 avatar Mar 07 '17 03:03 cesar1000