Provide a JUnit 5 equivalent of ModifiedClassPathRunner #17491
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
|
There is a test failing that doesn't seem to have anything to do with my changes as it's not using the new extension. Seems unrelated to my changes but changes in spring-projects/spring-data-relational@b7ee1bb. I might be wrong. |
|
Thanks very much for taking a look at this, @dreis2211. While we certainly dislike having a mixture of JUnit 4 and JUnit 5-based tests, I'm afraid we dislike the tests being run twice a little bit more. We're concerned that it'll cause confusion in the future as log output will be generated twice and debugging will be harder as well due to everything being invoked twice. That said, there's a lot of great stuff here even without the move to the new extension. Would you be interested in splitting this into two? We'd quite like to merge the introduction of the |
|
@wilkinsona That's unfortunate, but understandable. I have one more thing I'd like to try out in order to avoid the duplicated execution. Would this change your mind if that works? |
|
@wilkinsona I indeed found a way to avoid the duplicated test execution. Hope that changes your mind about the PR :) |
wilkinsona
added
type: task
Migrate a new tests to use the `ApplicationContextRunner`. See gh-17491
Migrate `RestClientTestWithoutJacksonIntegrationTests` to use Spring's `MockRestServiceServer`. See gh-17491
Extract classes from `ModifiedClassPathRunner` so that they can be reused. See gh-17491
Add a JUnit 5 extension that allows tests to be run with a modified classpath. Since JUnit 5 does not currently offer a way to run tests with a different classpath, we instead fake the original invocation and launch an entirely new run for each method. See gh-17491
Migrate the remaining JUnit 4 tests to JUnit 5, making use of the new `ModifiedClassPathExtension`. See gh-17491
Delete `ModifiedClassPathRunner` since we no longer have any tests that use it. See gh-17491
|
@dreis2211 This was an absolute monster effort!! Thanks so much. It's now been merged to master but I've chopped the PR up into a number of smaller commits. The reflection calls are a little brittle, but hopefully those JUnit internals won't change too much. Thanks again! |
Remove the `ModifiedClassPathClassLoaderFactory` in favor of factory methods on `ModifiedClassPathClassLoader`. See gh-17491
Meta-annotate `ClassPathExclusions` and `ClassPathOverrides` with so that the `ModifiedClassPathExtension` no longer needs to be used directly. See gh-17491
Remove `runner` since we're no longer tied to JUnit 4. See gh-17491
Hi,
the last two days I worked on an alternative for the
ModifiedClassPathRunnerin order to complete the migration to JUnit 5 and workaround the missing classloader support described in #16957 and junit-team/junit5#201.I've implemented an
ModifiedClassPathExtensionthat works similar to the oldModifiedClassPathRunnerand extracted the functionality that is shared between both. I did this because I deprecatedModifiedClassPathRunnerinstead of directly removing it, but I have no strong feelings for keeping the old stuff personally.The new extension makes use of JUnit 5's Launcher API and executes the tests with the modified classloader while swallowing the original test results. Yet, it still executes the original tests due to the fact that JUnit 5 unfortunately requires the underlying
Invocationto be processed. This leads obviously to slightly higher test execution times and makes proper test setup and cleaning (e.g. through@BeforeAlland@AfterAll) more important, but I found this to be reasonable still.With the new extension I was able to move all tests to JUnit 5. The extension is obviously not perfect and is a workaround until native classloader support in JUnit 5 arrives, but having no JUnit 4 tests anymore is hopefully a convincing argument for this PR.
Let me know what you think.
Cheers,
Christoph