feat: support junit5 in Java Protobuf SDK #1876

octonato · 2023-11-21T12:56:17Z

Experiment to support JUnit5

octonato

@ennru, here is the results of first experiments.

I modified only one sample. The integration tests are using junit5, but the unit tests are still on junit4. It works with both if we add the vintage engine.

We can add support for junit5 and still support junit4 at the same time.

Newly generated test classes will be on junit5 though. Users really willing to stick with junit4 will have to change the generated tests.

samples/java-protobuf-customer-registry-kafka-quickstart/pom.xml

octonato · 2023-11-21T13:11:50Z

...ava-sdk-protobuf-testkit/src/main/java/kalix/javasdk/testkit/junit/KalixTestKitResource.java

@@ -60,7 +63,7 @@
 * }
 * </pre>
 */
-public final class KalixTestKitResource extends ExternalResource {
+public final class KalixTestKitResource extends ExternalResource implements BeforeAllCallback, AfterAllCallback {


When implementing this two interfaces (BeforeAllCallback and AfterAllCallback), we can use this class as a Registered Extension. Junit will take care of the lifecycle for us. This is the equivalent of ClassRule in junit4.

KalixTestKitResource has already a hard dependency on junit4. In the sbt project, junit4 is marked as provided and then we add it to the pom explicitly.

With this change, the KalixTestKitResource will also depend on junit5 and we will need both in the pom. That's problematic for existing projects. If a user update the SDK version, but doesn't touch the pom, they will get an error.

Therefore, as mentioned above, we can solve it by added junit libraries as transitive dependencies.

Would it be an option to have a the JUnit 5 variant of this in the jupiter package?

Hmm, that could be an option.

That will make it easier for us to deprecate junit4 support.

I will PR this.

octonato · 2023-11-21T13:14:54Z

...gistry-kafka-quickstart/src/it/java/customer/api/CustomerActionWithKafkaIntegrationTest.java

@@ -38,7 +38,7 @@ public class CustomerActionWithKafkaIntegrationTest {
  /**
   * The test kit starts both the service container and the Kalix proxy.
   */
-  @ClassRule
+  @RegisterExtension


This will behave as the ClassRule annotation if the class (ie KalixTestKitResource) implements BeforeAllCallback and AfterAllCallback.

ennru · 2023-11-21T16:07:23Z

Possibly interesting changes in Testcontainers v1.18.0:

Move junit-jupiter-api's dependency configuration to implementation testcontainers/testcontainers-java#5985

See even Testcontainers container lifecycle management using JUnit 5

octonato · 2023-11-21T16:31:11Z

project/Dependencies.scala

-    junit4 % Provided,
-    junit5 % Provided,


This is the most impactful change.

So far, our testkit was depending directly on a class from junit4. It has junit5 as dependency, but it was not used.

Users were required to have junit4 in their pom because it was Provided.

It's annoying now to keep them as Provided because the testkit now requires both libraries. That means that users will need to add both to their pom.xml.

By making them transitive through the testkit, we ensure that users will have both and can pick the one they prefer.

On our side, new projects (created from mvn archetype) and new generated code will use junit5.

I don't see any point in supporting both junit versions. JUnit 5 was released in 2017. We could support both for a short transition time, but we should be very clear that migration is indispensable.

Even the current version of Testcontainers 1.19.3 depends on JUnit 4.13.2.

sdk/java-sdk-spring/src/it/java/com/example/wiring/tracing/TracingIntegratonTest.java

aludwiko

LGTM

ennru · 2023-11-22T14:17:53Z

project/Dependencies.scala

-    junit4 % Provided,
-    junit5 % Provided,


Even the current version of Testcontainers 1.19.3 depends on JUnit 4.13.2.

ennru · 2023-11-22T14:19:17Z

samples/java-protobuf-doc-snippets/src/main/java/com/example/Main.java

@@ -0,0 +1,33 @@
+package com.example;


Did you intend to check in this and the others generated files under java-protobuf-doc-snippets?

No, that was a mistake. And I'm surprised that they are not checked in. But I get why. We only build it to verify correctness and then the generated classed are each time recreated.

At some point, when chasing for compilation errors, I wrote a script to compile all projects under samples. That's how those got generated on my machine.

I will revert it.

ennru · 2023-11-22T14:25:41Z

...ava-sdk-protobuf-testkit/src/main/java/kalix/javasdk/testkit/junit/KalixTestKitResource.java

@@ -60,7 +63,7 @@
 * }
 * </pre>
 */
-public final class KalixTestKitResource extends ExternalResource {
+public final class KalixTestKitResource extends ExternalResource implements BeforeAllCallback, AfterAllCallback {


Would it be an option to have a the JUnit 5 variant of this in the jupiter package?

octonato · 2023-11-22T15:19:20Z

...rotobuf-valueentity-counter/src/test/java/com/example/actions/ExternalCounterActionTest.java

  public void setUp() {
    closeable = MockitoAnnotations.openMocks(this);
  }

-  @Before


For the record, that was my WTF moment of the day.

I don't know how this was working before, but the tearDown method should be annotated with After, not Before.

What I'm guessing is that Junit4 will only run the first method it finds annotated with Before. And the tearDown where never called.

github-actions bot added java-sdk-protobuf kalix-runtime Runtime and SDKs sub-team labels Nov 21, 2023

octonato changed the title ~~Octonato/move to junit5~~ feat: support junit5 in Java Protobuf SDK Nov 21, 2023

octonato commented Nov 21, 2023

View reviewed changes

octonato force-pushed the octonato/move-to-junit5 branch from f1a6284 to 5862de3 Compare November 21, 2023 13:19

ennru mentioned this pull request Nov 21, 2023

Create a Kalix Java/protobuf BOM #1878

Open

github-actions bot added the java-sdk label Nov 21, 2023

octonato added 6 commits November 21, 2023 17:21

chore: add support for junit5 in java/proto sdk

bc761aa

adapt quickstart sample

5c729b9

remove explicit junit dependency from samples

0a937c8

make junit dependency transitive

343aa89

port java samples to junit5

9bc6943

adapt codegen for junit5

64cc959

octonato force-pushed the octonato/move-to-junit5 branch from 6df437b to 64cc959 Compare November 21, 2023 16:22

octonato marked this pull request as ready for review November 21, 2023 16:22

update junit-jupite to 5.10.1

6ce83cb

octonato commented Nov 21, 2023

View reviewed changes

octonato added 3 commits November 21, 2023 17:53

fix codegen imports

d0887e0

some more import refactorings

88d7a04

fix compilation

ae8fd80

franciscolopezsancho reviewed Nov 22, 2023

View reviewed changes

sdk/java-sdk-spring/src/it/java/com/example/wiring/tracing/TracingIntegratonTest.java Show resolved Hide resolved

aludwiko approved these changes Nov 22, 2023

View reviewed changes

octonato added 3 commits November 22, 2023 10:57

fix annotation in spring test

2ab0732

fixed samples

42aadd2

fix replicated entity sample

569bce2

octonato merged commit 321435e into main Nov 22, 2023
65 checks passed

octonato deleted the octonato/move-to-junit5 branch November 22, 2023 14:25

ennru reviewed Nov 22, 2023

View reviewed changes

octonato commented Nov 22, 2023

View reviewed changes

octonato mentioned this pull request Nov 22, 2023

chore: split JUnit4/Jupiter testkit classes #1881

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support junit5 in Java Protobuf SDK #1876

feat: support junit5 in Java Protobuf SDK #1876

octonato commented Nov 21, 2023

octonato left a comment

octonato Nov 21, 2023

ennru Nov 22, 2023

octonato Nov 22, 2023

octonato Nov 21, 2023

ennru commented Nov 21, 2023 •

edited

Loading

octonato Nov 21, 2023

aludwiko Nov 22, 2023

ennru Nov 22, 2023

aludwiko left a comment

ennru Nov 22, 2023

ennru Nov 22, 2023

octonato Nov 22, 2023

ennru Nov 22, 2023

octonato Nov 22, 2023

feat: support junit5 in Java Protobuf SDK #1876

feat: support junit5 in Java Protobuf SDK #1876

Conversation

octonato commented Nov 21, 2023

octonato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ennru commented Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aludwiko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ennru commented Nov 21, 2023 •

edited

Loading