Add JUnit5 implementation of CompilationRule #155
Conversation
for a compilation that never finishes.
throw an exception, not return null.
… to run sequentially.
JUnit5 provides a specific exception for parameter resolution failures
If the compilation fails before the EvaluatingProcessor blocks then the extension would be permanently waiting. Forcing termination on completion of the compilation will ensure it can never be stuck waiting.
- Refactor so in absence of (before/after)All lifecycle events, it falls back on the (before/after)Each events. This supports @RegisterExtension and per-method @ExtendWith usage scenarios. - Slim down EvaluatingProcessor so it can either fail to the CompletableFuture or block on the Phaser, simplifying the exception path. - Extract all state in a dedicated object which can do checks on the state of the phaser before actions are performed. - Add tests for the compilation state and the various operations that synchronize it with the tests. - Test each of the different extension usage scenarios. - @ExtendWith on class - @ExtendWith on method - @RegisterExtension with Lifecycle.PER_CLASS - @RegisterExtension with Lifecycle.PER_METHOD
|
As mentioned in bcf955a, I've reworked the extension for two reasons:
|
|
+1 on this PR. I would like to see it merged so we can finally start using JUnit5. |
|
Google doesn't use JUnit 5 at all, and so we wouldn't have any way of testing/maintaining this. This may be worthwhile as a standalone library instead. |
|
I've moved the extension to its own repository at https://github.com/Kiskae/compile-testing-extension and submitted a request to include it in jcenter. I'll leave it to the repo owners to close this PR or change it to just link to the third-party resource as a reference. |
|
Several things
|
|
If we have no way to test, or validate the implementation, or have no experience with JUnit 5, it doesn't seem like us being the owners of the code seems to make sense. There can be many libraries that work well together without being one conglomerate. One exception to this, I would think, is if the internals of the Compile Testing are critical to the implementation of the code, which this isn't. |
raghsriniv
added
P4
type=other
This commit adds a JUnit5 extension based on the existing
CompilationRuleBecause of the difference between JUnit4
Ruleand JUnit5Extensionit is not a straightforward conversion, but the tests are just a straight copy since the primary difficulty exists in the extension itself due to the asynchronous nature of JUnit5:ElementsandTypesare only available inside of aProcessor. Because JUnit5 does not allow extensions to take direct control of the execution of tests this required a workaround where the compiler is executed on a separate thread and then blocked while the tests are running, passing over theProcessingEnvironmentduring that span of time.ParameterResolverto injectElementsandTypesdirectly as parameters to the method when they are required. This should also be compatible with other parameter injecting extensions like@ParameterizedTest. Accessing them as methods likeCompilationRuledoes would not work since JUnit5 strongly recommends that all extensions store their state inside ofExtensionContext.Store, which the tests themselves cannot directly access.CompilationRulethe extension runs a single compilation for the entire set of tests. Since the input of the compiler does not change I believe this would not lead to invalid results. If running a compilation for each separate test is required then the only change that needs to be made is replacing(Before/After)Allwith(Before/After)Each.(Before/After)Allare not being used by the user. This happens if@ExtendWithis applied to a method directly or when@RegisterExtensionis used.The only questionable part is the finalization of the compilation which currently relies on a call to
CompletableFuture.get(long,TimeUnit)with a timeout of 1 second. I'm not entirely sure if the timeout could potentially be reached during a normal execution or if there is a better way to handle that part of the synchronization.