test: verifying test, lint, clirr, and graalvm in checks by suztomo · Pull Request #456 · googleapis/java-shared-config

suztomo · 2022-03-28T19:05:29Z

My attempt to run "test", "lint", and "clirr" in downstream tests. These checks ensure the maven-surefire-plugin, fmt-maven-plugin, and clirr-maven-plugin work as expected in the 2 client libraries (GAPIC and hand-written).

I don't intent to catch every failure cases in every downstream libraries, but I believe this setup will catch 90% of cases where plugin upgrades don't work in downstream repositories (and we would re-release the shared config again). Example: #452

CC: @Neenu1995

What if upgrading fmt-maven-plugin require adjusting Java code?

This PR changes the lint as a required check. This may introduce circular dependency.

For example, upgrading google-java-format to 1.15 requires code change in the downstream repository #452 (comment)
This require code change in the repository:

 /** Static utility methods for working with Errors returned from the service. */
 public class Errors {
-  private Errors() {};
+  private Errors() {}
+  ;

But we first would need to release the shared config before making the formatting change. How do we deal with it?

=> We can run mvn com.coveo:fmt-maven-plugin:format in the script to format the code before running com.coveo:fmt-maven-plugin:check:

suztomo · 2022-03-28T19:08:16Z

This detected the #452 in the check. Good:

suztomo · 2022-03-28T19:10:48Z

We would need to update the required checks:

The following simple Python script can generate the list for j in [8, 11]: for r in ["java-trace", "java-bigquery"]: for t in ["test", "lint", "clirr"]: print(' - "build (%s, %s, %s)"' % (j, r, t))

generated-files-bot · 2022-03-28T19:42:09Z

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

.github/sync-repo-settings.yaml

suztomo · 2022-03-28T20:14:25Z

We would need to update the required checks:

In this PR, I keep the existing required "dependencies" checks as they are, but this PR adds new checks as required.

suztomo · 2022-03-29T16:47:12Z

The GraalVM test failed because it didn't run with GraalVM:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  16.201 s
[INFO] Finished at: 2022-03-29T16:45:43Z
[INFO] ------------------------------------------------------------------------
Error:  Failed to execute goal org.graalvm.buildtools:native-maven-plugin:0.9.11:test (test-native) on project google-cloud-orgpolicy: Execution test-native of goal org.graalvm.buildtools:native-maven-plugin:0.9.11:test failed: A required class was missing while executing org.graalvm.buildtools:native-maven-plugin:0.9.11:test: org/graalvm/nativeimage/hosted/Feature

Note: probably I do not go for this option. I could setup GraalVM binary in GitHub Actions but this does not reflect what we would have in downstream repositories.

suztomo · 2022-03-29T16:57:09Z

This ayltai/setup-graalvm is handy:

Note: probably I might not go for this option to install GraalVM. I could setup GraalVM binary in GitHub Actions like this, but this does not reflect what we would have in downstream repositories, which use testing-infra-docker.

How can we ensure the GraalVM version in testing-infra-docker and native-image-version from the shared config?

(Maybe manually synching the version still works)

suztomo · 2022-03-29T17:11:34Z

Upgrade actions https://github.com/googleapis/java-eventarc/blob/main/.github/workflows/ci.yaml#L80

action/checkout to v3

setup-java to v3

suztomo · 2022-03-29T17:15:20Z

.github/workflows/downstream-maven-plugins.yaml

+        - java-orgpolicy
+    steps:
+    - uses: actions/checkout@v2
+    - uses: stCarolas/setup-maven@v4


Remove this

suztomo · 2022-03-29T17:32:37Z

.github/workflows/downstream-maven-plugins.yaml

+    - uses: stCarolas/setup-maven@v4
+      with:
+        maven-version: 3.8.1
+    - uses: ayltai/setup-graalvm@v1


@Neenu1995 @mpeddada1 Do we allow/ban this GitHub Actins?

As long as it doesn't access our secrets we are fine with using any action. But this one looks like it is not being actively developed anymore. So may not be a good long term solution.

Thanks. I'll use this then.

I see the owner maintains the repository. https://github.com/ayltai/setup-graalvm/commits/master
When we need alternative we can just use curl and unzip the content.

My bad. I saw the last release was in May 2021. Looks like it is being being actively developed.

suztomo · 2022-03-29T17:43:49Z

11 minutes to finish GraalVM test. Not too bad but I will revisit this long wait time by asking feedback among the devs here in few months.

suztomo · 2022-03-29T17:46:38Z

168cc88 should fail at the GraalVM test. As per @mpeddada1 , native-maven-plugin 0.9.11 and GraalVM 21.2.0 are incompatible. Let's see whether this new test can detect the incompatibility.

mpeddada1 · 2022-03-29T17:50:23Z

Nice! It caught the incompatibility:

Failed to execute goal org.graalvm.buildtools:native-maven-plugin:0.9.11:test (test-native) on project google-cloud-orgpolicy: Execution of /opt/hostedtoolcache/GraalVM/java11-linux-amd64-21.2.0/x64/bin/native-image @/tmp/native-image1865512348312126488args org.graalvm.junit.platform.NativeImageJUnitLauncher returned non-zero result -> [Help 1]

mpeddada1 · 2022-03-29T17:51:39Z

@suztomo @Neenu1995 Is there any way we can keep this test consistent with the version in the testing-docker-infra?

suztomo · 2022-03-29T18:03:29Z

@mpeddada1 I don't think it's possible. I leave a comment in YAML file: https://github.com/googleapis/java-shared-config/pull/456/files#r837768926

If it's possible, then the question is which version of the container testing-docker-infra should this repository reference? Latest release, main, or certain versions.

suztomo · 2022-03-29T18:10:28Z

.github/workflows/downstream-maven-plugins.yaml

+        # When a new version of native-maven-plugin fails to run in a downstream
+        # library, it's likely to be an incompatibility with the GraalVM version.
+        # In that case, you need to upgrade the Docker container used in the
+        # tests in the downstream repositories (not just this value below).
+        # Example: https://github.com/googleapis/testing-infra-docker/pull/195
+        graalvm-version: 22.0.0.2


Added an instruction on how to upgrade GraalVM version in testing-infra-docker.

mpeddada1 · 2022-03-29T18:19:24Z

@mpeddada1 I don't think it's possible. I leave a comment in YAML file: https://github.com/googleapis/java-shared-config/pull/456/files#r837768926
If it's possible, then the question is which version of the container testing-docker-infra should this repository reference? Latest release, main, or certain versions.

The official page usually recommends being as specific as possible with the version tag. Yeah you're right, it might also be difficult to automate because there is a chance that the tag we specify won't be available on https://github.com/graalvm/container/pkgs/container/jdk. Manual upgrade sounds good for now! The comment will be helpful.

Neenu1995 · 2022-03-29T19:07:03Z

If we want to use the exact same image in test-infra-docker, then we will have to move the tests to kokoro. But I don't think that is a direction we want to take.

test: verifying test, lint, and clirr in checks

b75e431

suztomo requested a review from a team March 28, 2022 19:05

suztomo added 5 commits March 28, 2022 15:13

Merge remote-tracking branch 'origin/main' into lint_downstream

a26f843

test: renaming job name

c344f2c

test: reverted dependencies job and adding build jobs

e9af28f

ci: apt-get update before installing libxml2-utils

1c8a91a

ci: adding the new builds as required checks

6219a7b

The following simple Python script can generate the list for j in [8, 11]: for r in ["java-trace", "java-bigquery"]: for t in ["test", "lint", "clirr"]: print(' - "build (%s, %s, %s)"' % (j, r, t))

suztomo requested a review from Neenu1995 March 28, 2022 19:53

suztomo added 3 commits March 29, 2022 12:38

Merge remote-tracking branch 'origin/main' into lint_downstream

ea5a008

test: trying graalvm job-type

83941f6

test: graalvm not for handwritten library

8fc7aed

ci: setup graalvm

dc4cf3e

Note: probably I do not go for this option. I could setup GraalVM binary in GitHub Actions but this does not reflect what we would have in downstream repositories.

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

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

suztomo commented Mar 29, 2022

View reviewed changes

test: added comment and upgraded GitHub Actions

c5cd072

Neenu1995 approved these changes Mar 29, 2022

View reviewed changes

suztomo commented Mar 29, 2022

View reviewed changes

ci: testing old GraalVM version

168cc88

suztomo changed the title ~~test: verifying test, lint, and clirr in checks~~ test: verifying test, lint, clirr, and graalvm in checks Mar 29, 2022

ci: using GraalVM 22.0.0.2

1d090e9

suztomo commented Mar 29, 2022

View reviewed changes

suztomo added the automerge Merge the pull request once unit tests and other checks pass. label Mar 29, 2022

mpeddada1 approved these changes Mar 29, 2022

View reviewed changes

suztomo removed the automerge Merge the pull request once unit tests and other checks pass. label Mar 29, 2022

suztomo merged commit d5c45b8 into googleapis:main Mar 29, 2022

suztomo deleted the lint_downstream branch March 29, 2022 19:09

suztomo mentioned this pull request Apr 5, 2022

Downstream check with lint, clirr, and others #454

Closed

Conversation

suztomo commented Mar 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What if upgrading fmt-maven-plugin require adjusting Java code?

Uh oh!

suztomo commented Mar 28, 2022

Uh oh!

suztomo commented Mar 28, 2022

Uh oh!

generated-files-bot bot commented Mar 28, 2022

Uh oh!

suztomo commented Mar 28, 2022

Uh oh!

suztomo commented Mar 29, 2022

Uh oh!

suztomo commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

suztomo commented Mar 29, 2022

Uh oh!

suztomo Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

suztomo Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

Neenu1995 Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

suztomo Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

Neenu1995 Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

suztomo commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

suztomo commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpeddada1 commented Mar 29, 2022

Uh oh!

mpeddada1 commented Mar 29, 2022

Uh oh!

suztomo commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

suztomo Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

mpeddada1 Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

mpeddada1 commented Mar 29, 2022

Uh oh!

Neenu1995 commented Mar 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

suztomo commented Mar 28, 2022 •

edited

Loading

suztomo commented Mar 29, 2022 •

edited

Loading

suztomo commented Mar 29, 2022 •

edited

Loading

suztomo commented Mar 29, 2022 •

edited

Loading

suztomo commented Mar 29, 2022 •

edited

Loading