Skip to content

Conversation

@martin-g
Copy link
Member

Use JUnit 5.x Vintage engine instead of JUnit 4.x

What is the purpose of the change

  • Fix the execution of JUnit 4.x unit tests by using JUnit 5.x Vintage engine. Fixes AVRO-3628

Verifying this change

Check the stdout of the Maven build and make sure that JUnit 4.x tests are executed, e.g. avro-maven-plugin module should execute 11 tests.

Documentation

  • Does this pull request introduce a new feature? no

Use JUnit 5.x Vintage engine instead of JUnit 4.x

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@github-actions github-actions bot added build Java Pull Requests for Java binding labels Sep 13, 2022
<groupId>org.apache.maven</groupId>
<artifactId>maven-core</artifactId>
<version>${maven-core.version}</version>
<scope>provided</scope>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the following Maven error:

[INFO] --- maven-plugin-plugin:3.6.4:helpmojo (generated-helpmojo) @ avro-maven-plugin ---
[ERROR] 

Some dependencies of Maven Plugins are expected to be in provided scope.
Please make sure that dependencies listed below declared in POM
have set '<scope>provided</scope>' as well.

The following dependencies are in wrong scope:
 * org.apache.maven:maven-core:jar:3.3.9:compile
 * org.apache.maven:maven-model:jar:3.3.9:compile
 * org.apache.maven:maven-settings:jar:3.3.9:compile
 * org.apache.maven:maven-settings-builder:jar:3.3.9:compile
 * org.apache.maven:maven-builder-support:jar:3.3.9:compile
 * org.apache.maven:maven-repository-metadata:jar:3.3.9:compile
 * org.apache.maven:maven-artifact:jar:3.3.9:compile
 * org.apache.maven:maven-plugin-api:jar:3.3.9:compile
 * org.apache.maven:maven-model-builder:jar:3.3.9:compile
 * org.apache.maven:maven-aether-provider:jar:3.3.9:compile

<artifactId>avro-parent</artifactId>
<groupId>org.apache.avro</groupId>
<version>1.11.0-SNAPSHOT</version>
<version>1.12.0-SNAPSHOT</version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems something is wrong with the release process. Several pom.xml files were still pointing to the old 1.11.0-SNAPSHOT parent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 82 to 84
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
<version>\${junit5.version}</version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it (only) uses the vintage engine. Do we also want to run JUnit5 (Jupiter) tests?

In that case, we also need to add the Jupiter engine IIRC.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh -- as far as I remember @opwvhk is correct, and yet JUnit5 tests are being run! (TestDataFile is definitely a JUnit5 migrated class).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, my mistake, I see -- yes, this archetype can probably be safely left as it is (on JUnit4) or entirely migrate the generated test to JUnit5

Lets not mix JUnit4 and JUnit5 in the archetype, however!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I will migrate the archetype to JUnit 5.x!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with 65c3ac3

Comment on lines 82 to 84
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
<version>\${junit5.version}</version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, my mistake, I see -- yes, this archetype can probably be safely left as it is (on JUnit4) or entirely migrate the generated test to JUnit5

Lets not mix JUnit4 and JUnit5 in the archetype, however!

<artifactId>avro-parent</artifactId>
<groupId>org.apache.avro</groupId>
<version>1.12.0-SNAPSHOT</version>
<relativePath>../</relativePath>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for info, is this a best practice?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least Intellij IDEA suggests so. It shows ../ as a warning.
I'm OK to revert it.

@RyanSkraba
Copy link
Contributor

Just for info -- adding up the test cases before the change: 144 versus 10504 after the change 😱

@martin-g
Copy link
Member Author

Just for info -- adding up the test cases before the change: 144 versus 10504 after the change scream

Yup, the CI will become slower again :-)

Copy link
Contributor

@RyanSkraba RyanSkraba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

@RyanSkraba RyanSkraba merged commit 859645d into master Sep 14, 2022
@RyanSkraba RyanSkraba deleted the avro-3628-junit-5.x-vintage branch September 14, 2022 18:29
RyanSkraba pushed a commit that referenced this pull request Sep 14, 2022
* AVRO-3628: [Java] JUnit 4.x tests are not executed

Use JUnit 5.x Vintage engine instead of JUnit 4.x

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

* AVRO-3628: Migrate avro-service-archetype tests to JUnit 5.x

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Java Pull Requests for Java binding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants