Open mannodermaus opened 1 month ago
Thank you for the swift review! πββοΈ I will address the CI failure shortly and consider extending the unit tests.
@takahirom γεΎ γγγγΎγγγWe should be good to go with another run of the workflows. I reinstated the basic JUnit 5 detection of the stack trace strategy in the latest commit https://github.com/takahirom/roborazzi/pull/355/commits/0d8c86edad33cf61c1d832b7adf7e4cd8b1902cd, since apparently Kotlin Test defaults to using JUnit 5 behind the scenes for Compose Desktop and Multiplatform environments in some cases. With that, I was able to run all tests locally fine, including the ones in the sample projects. π€
Looks great. I should have mentioned this earlier, but I would like to add some tests and documentation.
Absolutely, sounds like a solid plan. I'll need a bit of time for this, but I'll let you know once done.
@takahirom I'm thinking about the RoborazziRule
and whether or not to make a similar functionality available with the JUnit 5 integration as well (since you wouldn't be able to use the JUnit 4-based rule with the newer API). Would this be something you are interested in?
@mannodermaus I think if we could have a mechanism similar to JUnit4 rules, that would be great, but I'm fine with the current implementation for now. We can add that mechanism to JUnit5 later.
Seems like we have a compatibility issue because the Robolectric JUnit 5 extension is built against Java 17, while Roborazzi uses the more conversative Java 11. π€ AGP has had 17 as the minimum version since 8.0, would this be something you'd be willing to raise? Roborazzi could keep its targetCompatibility at 11, but use a higher-level toolchain for compilation.
@takahirom I wrote a doc section on JUnit 5 in the latest commit (first time using Writerside; pretty neat!) and also updated the Robolectric extension to the latest version. It's now targeting Java 11, so there shouldn't be any compatibility issues - let's just hope that the integration tests finally work.
This PR introduces
roborazzi-junit5
, a newly proposed artifact for extending the capabilities of this fantastic library with the newest version of JUnit. It refactors how Roborazzi's file name generation works and adds a second detection algorithm specifically for tests using the JUnit Jupiter API from JUnit 5.Background
Historically, Robolectric-based tests have not been supported by JUnit 5 because of incompatibilities with the way it injects custom class loading into the execution environment. However, very recently there have been some fantastic achievements in this space, finally allowing JUnit 5 to deal with Robolectric tests via a third-party extension. The hope is to integrate this extension into the mainline Robolectric library in the future. With this extension in place, it's already possible to use the current Roborazzi version for ordinary test cases, but only if you specify the file path manually for every usage of
captureRoboImage
.Motivation
The default behavior of
captureRoboImage
delegates the final file name of the captured image to theDefaultFileNameGenerator
. This class can infer the file name by looking at the stacktrace of the Robolectric thread and finding the element annotated with@org.junit.Test
. There is also some basic detection for the JUnit 5 annotation (@org.junit.jupiter.api.Test
), but the check doesn't cover all of the possible cases and fails when used together with the extension I mentioned earlier. Consider the following test class as an example:Approach
This PR refactors the stacktrace detection code and makes it an implementation of the new
TestNameExtractionStrategy
interface inside ofroborazzi-core
. By default, Roborazzi will always use this implementation for generating file names, just like before. Additionally, it can detect ifroborazzi-junit5
is on the classpath, and if this is the case, it also adds the newJUnit5TestNameExtractionStrategy
to itself. You see, the issue with JUnit 5 stacktraces is that they don't always contain the exact test method in them, so you cannot find them by their annotation. Furthermore, there are a whole bunch of annotations that generate tests at runtime, and those cases don't have any annotations in the trace and therefore cannot be detected with the current way:@ParameterizedTest
@RepeatedTest
@TestFactory
@TestTemplate
The extraction strategy for JUnit 5 that I added to
roborazzi-junit5
is tailored to detecting the names of these tests. It reads the currently executed test method from a shared storage calledCurrentTestInfo
, which is updated from the other side by a JUnit 5 extension that keeps the storage up-to-date at all times. Since both sides are attached to different class loaders, there is a fair bit of ugly trickery involved to send this information across their boundaries. π΅ Essentially, the extraction strategy has to look up the sharedCurrentTestInfo
via reflection, otherwise it cannot see the other class loader's static data. I'm using lazy references as much as possible to minimize the runtime performance impact of this.The following diagram illustrates how the classes in the new artifact work together:
Setup
With this new artifact, consumers can use Roborazzi in their Robolectric tests with JUnit 5. The proposed setup would look like this:
Step 1: Add the new dependency next to the main Roborazzi library
Step 2: Enable Roborazzi's JUnit 5 extension
Any of the following options will work, and only one of them needs to be followed. I personally prefer option C.
Option A: Add the Roborazzi extension to each test class
Option B: Enable autodetection of extensions globally via properties file
Option C: Enable autodetection of extensions via android-junit5's Gradle DSL
Conclusion
Apologies for dropping this wall of text unprompted! Please let me know your thoughts. πββοΈ This PR may be incomplete, as I have not yet looked into the publishing part of this new artifact. If there are any other files that need updating, let me know.