chore: update maven surefire plugin #580

JoeWang1127 · 2023-03-21T00:32:51Z

Fixes #577 ☕️

generated-files-bot · 2023-03-21T02:12:56Z

Warning: This pull request is touching the following templated files:

.kokoro/build.sh

mpeddada1 · 2023-03-22T14:41:11Z

pom.xml

@@ -828,7 +826,7 @@
                <goals>
                  <goal>test</goal>
                </goals>
-                <phase>test</phase>


For the sake of maintaining Kokoro efficiency, would it be possible to keep the test phase? The advantage that it brings over integration-test and verify is that it doesn't require classes to be packaged/deployed so I'm concerned that using integration-test might increase the duration of the builds.

Do you mean I should create another execution, so we can have two executions like

<execution> <id>test-native</id> <goals> <goal>test</goal> </goals> <phase>test</phase> </execution> <execution> <id>integration-test-native</id> <goals> <goal>test</goal> </goals> <phase>integration-test</phase> </execution>

mpeddada1 · 2023-03-22T14:44:51Z

pom.xml

@@ -804,8 +803,7 @@
          <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-surefire-plugin</artifactId>
-            <!-- Must use older version of surefire plugin for native-image testing. -->
-            <version>2.22.2</version>


Thanks for the upgrade @JoeWang1127! Would it be possible to do a quick check with a few handwritten libraries? (pubsub, spanner and bigquery, for example) I remember this causing some issues a while back when I tried it out so. what to double check if this is still a problem. We could create a draft PR in those libs, copy-paste this profile, rename it to something like native-2 and modify the .kokoro/build.sh to use -Pnative-2)

Sure, when you think the profile is OK, I'll verify it in handwritten libraries.

burkedavison · 2023-03-22T15:15:26Z

.kokoro/build.sh

    RETURN_CODE=$?
    ;;
 graalvm17)
    # Run Unit and Integration Tests with Native Image
-    mvn -B ${INTEGRATION_TEST_ARGS} -ntp -Pnative test
+    mvn -B ${INTEGRATION_TEST_ARGS} -ntp -Pnative integration-test


I agree with @mpeddada1 's concerns, but want to also add as a side-note:

mvn integration-test should rarely be directly invoked. Instead, mvn post-integration-test or mvn verify should be preferred depending on the situation. See the Maven lifecycle documentation for additional information.

In this specific case of -Pnative, the configurations become unique due to the behavior of the native image plugin, and we should be using the mvn test goal - not verify or post-integration-test.

I changed it to integration-test because it is a limitation in maven, please see the reference

Thanks for link, @JoeWang1127. Correct me if I'm looking at an incorrect section but it looks like the integration-test phase was only suggested as a workaround for those experiencing the the long classpath issue in Windows. Reference: https://graalvm.github.io/native-build-tools/latest/maven-plugin.html#long_classpath_and_shading_support. Since our test jobs aren't on windows we shouldn't be experiencing the same issue.

That being said, testing on different platforms can be something to consider in the future. cc/ @burkedavison

@mpeddada1 thanks, I think you're right, the integration-test phase is used in Windows.

I'll change back to test phase.

It has the follow error if I changed to test phase.

[ERROR] Test configuration file wasn't found. Make sure that test execution wasn't skipped.

I'm guessing there's an issue with JUnit dependency (this is specified in the reference). I'll keep looking.

I think you're on the right track. According to the reference:

However, if you are using a version of Maven Surefire prior to 3.0 M4 you will need to add an explicit dependency on the junit-platform-launcher artifact to the dependencies section of your native profile configuration as in the following example.

Perhaps we can also try explicitly specifying versions greater than 3.0.0-M5, like 3.0.0-M6. This looks like a relevant issue:graalvm/native-build-tools#151

JoeWang1127 · 2023-03-23T00:31:26Z

Hi @burkedavison and @mpeddada1, I have two questions related to this PR. PTAL.

in downstream-maven-plugins.yaml, the repository used in native test is java-orgpolicy (line 48), which points to https://github.com/googleapis/java-orgpolicy.git in client-library-check.sh (line 58) eventually. However, this repo is migrated to google-cloud-java, should we change the pointer in client-library-check.sh or it's intended?
If I updated the surefire plugin to 3.0.0, there's an error when running mvn -Pnative test:

[ERROR] Test configuration file wasn't found. Make sure that test execution wasn't skipped.

This is harder than I thought so I created a test project to verify the surefire plugin. It works fine.

Do you have any other thoughts about this?

PS: I left a comment in graalvm/native-build-tools#378, thanks @mpeddada1

burkedavison · 2023-03-23T14:56:12Z

@JoeWang1127 : However, this repo is migrated to google-cloud-java, should we change the pointer in client-library-check.sh or it's intended?

We should change it to google-cloud-java.

JoeWang1127 · 2023-03-25T19:58:13Z

pom.xml

-            <!-- Must use older version of surefire plugin for native-image testing. -->
-            <version>2.22.2</version>
+            <dependencies>
+              <dependency>


Move junit-vintage-engine inside maven-surefire-plugin, otherwise the plugin would use surefire-junit4 provider instead of the surefire-junit-platform provider.

reference

JoeWang1127 · 2023-03-25T20:01:26Z

pom.xml

-          <artifactId>junit-platform-native</artifactId>
-          <version>0.9.20</version>
-          <scope>test</scope>
+          <groupId>org.opentest4j</groupId>


This dependency is needed to resolve class org.opentest4j.TestAbortedException

Error log without this dependency:

Failures (1): JUnit Jupiter => org.junit.platform.commons.JUnitException: TestEngine with ID 'junit-jupiter' failed to execute tests org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:113) org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88) org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54) org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67) org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52) [...] Caused by: java.lang.NoClassDefFoundError: org.opentest4j.TestAbortedException org.junit.jupiter.engine.support.OpenTest4JAndJUnit4AwareThrowableCollector.<clinit>(OpenTest4JAndJUnit4AwareThrowableCollector.java:38) org.junit.jupiter.engine.support.JupiterThrowableCollectorFactory.createThrowableCollector(JupiterThrowableCollectorFactory.java:34) org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:89) org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35) org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57) [...]

Full log: https://github.com/googleapis/java-shared-config/actions/runs/4520881632/jobs/7962275716

JoeWang1127 · 2023-03-25T20:04:08Z

pom.xml

@@ -95,13 +95,6 @@
            </excludes>
            <reportNameSuffix>sponge_log</reportNameSuffix>
          </configuration>
-          <dependencies>
-            <dependency>


I'm not sure where this dependency is used.

It turns out that this dependency prevents maven-surefire-plugin using Junit 5 Platform.

Before the change:

[INFO] --- maven-surefire-plugin:3.0.0:test (default-test) @ google-cloud-orgpolicy --- [INFO] Using configured provider org.apache.maven.surefire.junitcore.JUnitCoreProvider

After the change:

[INFO] --- maven-surefire-plugin:3.0.0:test (default-test) @ google-cloud-orgpolicy --- [INFO] Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider

It looks like this dependency was added while ago in #37 to fix an issue with mvn verify in java-speech. To double check, perhaps we can run a test with java-speech to make sure that's still not the case?

JoeWang1127 · 2023-03-25T20:31:02Z

Hi @burkedavison and @mpeddada1, I had to remove a dependency in maven-surefire-plugin to pass the checks. Please see comment.

Since I modified the shared config, I built a SNAPSHOT version and test it against java-storage on my local environment, the native test succeeds. This is the build log.

burkedavison · 2023-03-27T13:53:56Z

LGTM, but would like this approved by @mpeddada1

mpeddada1

Excellent investigation @JoeWang1127! Added a few comments.

mpeddada1 · 2023-03-28T22:27:53Z

pom.xml

-          <artifactId>junit-platform-native</artifactId>
-          <version>0.9.20</version>
-          <scope>test</scope>
+          <groupId>org.opentest4j</groupId>


Could we add a source comment for this dependency addition?

mpeddada1 · 2023-03-28T22:36:42Z

pom.xml

@@ -95,13 +95,6 @@
            </excludes>
            <reportNameSuffix>sponge_log</reportNameSuffix>
          </configuration>
-          <dependencies>
-            <dependency>


It looks like this dependency was added while ago in #37 to fix an issue with mvn verify in java-speech. To double check, perhaps we can run a test with java-speech to make sure that's still not the case?

JoeWang1127 · 2023-03-29T14:16:33Z

I tested mvn -Pnative test on google-cloud-java/java-speech and it worked. This is the build log.

Steps to reproduce the test result:

checkout this branch and change the version to 1.5.5-SNAPSHOT.
run mvn clean install on shared config.
checkout main branch of google-cloud-java.
change the shared config version to 1.5.5-SNAPSHOT in google-cloud-pom-parent/pom.xml.
in java-speech module, run mvn clean install
in java-speech module, run mvn -Pnative test

I also re-test java-storage with the above steps and it worked. This is the build log.

chore: update maven surefire plugin

ad27ee7

product-auto-label bot added the size: xs Pull request size is extra small. label Mar 21, 2023

JoeWang1127 added 2 commits March 20, 2023 21:41

add junit 5 dep

bb63bc2

add execution

f06ff66

product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Mar 21, 2023

JoeWang1127 added 3 commits March 20, 2023 21:57

add junit dep inside surefire plugin

f0c889d

import jupiter api dep

566ee1e

change phase to integreation test

c6eb436

JoeWang1127 added 2 commits March 21, 2023 09:42

change graalvm17 params

270651f

remove one execution

ed6d218

JoeWang1127 marked this pull request as ready for review March 21, 2023 13:51

JoeWang1127 requested a review from a team as a code owner March 21, 2023 13:51

JoeWang1127 requested a review from mpeddada1 March 21, 2023 13:51

mpeddada1 reviewed Mar 22, 2023

View reviewed changes

burkedavison reviewed Mar 22, 2023

View reviewed changes

change back to test phase

39741ea

product-auto-label bot added size: xs Pull request size is extra small. and removed size: s Pull request size is small. labels Mar 22, 2023

remove junit dep

a7f54ac

product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Mar 22, 2023

JoeWang1127 added 5 commits March 22, 2023 15:43

add junit engine inside plugin

2b08c47

restore graalvm dep

1ad2bba

add an execution in package phase

d9c387a

Merge branch 'main' into chore/update-Surefire-plugin

911428b

remove dep

2277c7d

product-auto-label bot added the size: m Pull request size is medium. label Mar 25, 2023

remove junt47

159f41e

product-auto-label bot added size: s Pull request size is small. and removed size: m Pull request size is medium. labels Mar 25, 2023

add opentest4j

0816ba4

JoeWang1127 commented Mar 25, 2023

View reviewed changes

JoeWang1127 requested review from burkedavison and mpeddada1 March 25, 2023 20:31

restore graalvm setup

78be8f8

mpeddada1 reviewed Mar 28, 2023

View reviewed changes

add comment

ff1fae9

JoeWang1127 requested a review from mpeddada1 March 29, 2023 14:35

mpeddada1 approved these changes Mar 29, 2023

View reviewed changes

trigger build

f5dc0df

JoeWang1127 added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:run Add this label to force Kokoro to re-run the tests. labels Mar 29, 2023

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 29, 2023

JoeWang1127 added 3 commits March 29, 2023 11:45

trigger build

a9a1bbf

Merge branch 'main' into chore/update-Surefire-plugin

dba4e4b

trigger build

744d8fd

JoeWang1127 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 29, 2023

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 29, 2023

JoeWang1127 merged commit 9c0de77 into main Mar 29, 2023

JoeWang1127 deleted the chore/update-Surefire-plugin branch March 29, 2023 17:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: update maven surefire plugin #580

chore: update maven surefire plugin #580

JoeWang1127 commented Mar 21, 2023 •

edited

Loading

generated-files-bot bot commented Mar 21, 2023

mpeddada1 Mar 22, 2023

JoeWang1127 Mar 22, 2023

mpeddada1 Mar 22, 2023

JoeWang1127 Mar 22, 2023

burkedavison Mar 22, 2023

JoeWang1127 Mar 22, 2023

mpeddada1 Mar 22, 2023

JoeWang1127 Mar 22, 2023

mpeddada1 Mar 22, 2023

JoeWang1127 commented Mar 23, 2023

burkedavison commented Mar 23, 2023

JoeWang1127 Mar 25, 2023 •

edited

Loading

JoeWang1127 Mar 25, 2023

JoeWang1127 Mar 25, 2023 •

edited

Loading

mpeddada1 Mar 28, 2023

JoeWang1127 commented Mar 25, 2023

burkedavison commented Mar 27, 2023

mpeddada1 left a comment

mpeddada1 Mar 28, 2023

JoeWang1127 Mar 29, 2023

mpeddada1 Mar 28, 2023

JoeWang1127 commented Mar 29, 2023 •

edited

Loading

chore: update maven surefire plugin #580

chore: update maven surefire plugin #580

Conversation

JoeWang1127 commented Mar 21, 2023 • edited Loading

generated-files-bot bot commented Mar 21, 2023

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeWang1127 commented Mar 23, 2023

burkedavison commented Mar 23, 2023

JoeWang1127 Mar 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeWang1127 Mar 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoeWang1127 commented Mar 25, 2023

burkedavison commented Mar 27, 2023

mpeddada1 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

JoeWang1127 commented Mar 29, 2023 • edited Loading

JoeWang1127 commented Mar 21, 2023 •

edited

Loading

JoeWang1127 Mar 25, 2023 •

edited

Loading

JoeWang1127 Mar 25, 2023 •

edited

Loading

JoeWang1127 commented Mar 29, 2023 •

edited

Loading