ci: add downstream compatibility checks by burkedavison · Pull Request #1909 · googleapis/sdk-platform-java

burkedavison · 2023-08-07T20:22:56Z

Adds a new GitHub workflow to perform downstream testing on client libraries.

Installs sdk-platform-java to local maven.
Checks out the downstream repository at the latest release associated with the main branch, not HEAD.
Updates all dependencies on java-shared-dependency to the local version.
Compiles and invokes unit tests for the given client libraries.

Does not invoke integration tests. Downstream integration testing is limited to google-cloud-java via the existing GraalVM Kokoro presubmits.

meltsufin

It looks good, but would it make sense to add some tests for the functions added in common.sh?

blakeli0 · 2023-08-08T17:10:13Z

Checks out the downstream repository at the latest release associated with the main branch, not HEAD.

What's the reason to not checkout HEAD on main branch? To avoid possible frictions?

blakeli0 · 2023-08-08T17:19:32Z

  exit 1
 fi
+# Use default value for REPOS_UNDER_TEST if unset. If set to empty string, maintain empty string.
+REPOS_UNDER_TEST=${REPOS_UNDER_TEST-"java-storage"}


Is it intentional to have java-storage here?

Yes. It's the only current repository with GraalVM tests that don't require cloud project resources.

Thanks! Can we add comments to explain it?

blakeli0 · 2023-08-08T17:20:53Z

+source "$scriptDir/common.sh"
+
+echo "Setup maven mirror"
+mkdir -p "${HOME}/.m2"


I might be missing something, I thought .m2 folders would be created automatically by running mvn command?

Likely, however we haven't yet invoked mvn at this point.

What's the content in settings.xml? If we don't push to anywhere, I think we might be able to just use the default?

https://github.com/googleapis/sdk-platform-java/blob/main/settings.xml

It uses the Google mirror instead of maven central directly.

This logic is not new to this PR. If we remove it from here, we should remove use of the mirror in all the repos that use it. I'd ask this discussion not be considered part of this PR.

https://github.com/googleapis/java-shared-config/blob/763419aefd7efb40b30641641198ca6491b380d2/.kokoro/presubmit/downstream-build.sh#L42-L44

sdk-platform-java/.kokoro/presubmit/downstream-build.sh

Lines 28 to 30 in 33e40ff

# Use GCP Maven Mirror

mkdir -p "${HOME}/.m2"

cp settings.xml "${HOME}/.m2"

https://github.com/googleapis/google-cloud-java/blob/a082d632ddb3aa413886b731e0b0298f2bec8170/.kokoro/build.sh#L26-L28

This logic is not new to this PR. If we remove it from here, we should remove use of the mirror in all the repos that use it. I'd ask this discussion not be considered part of this PR.

Agreed. It was mostly for my own understanding, @suztomo what's the benefit of using Google mirror? I guess it's more stable than Maven central when downloading dependencies locally?

It's also faster in Kokoro CI because both are in Google'a infrastructure.

burkedavison · 2023-08-08T17:32:11Z

What's the reason to not checkout HEAD on main branch? To avoid possible frictions?

To avoid any issues with main not always being stable. Across all repos, it's possible that main is not always ready to release and may enter into a bad state. OTOH, it's very unlikely that a release commit is in a bad state.
To make it more difficult to introduce a breaking change by updating sdk-platform-java and the downstream repo at the same time. It's still possible that the downstream repo prepares for the breaking change, releases, then makes the sdk-platform-java breaking change -- but less likely without triggering a conversation.

blakeli0

LGTM. Pending resolution of add more comments.

sonarqubecloud · 2023-08-09T15:51:57Z

[gapic-generator-java-root] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

sonarqubecloud · 2023-08-09T15:56:51Z

[java_showcase_integration_tests] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

sonarqubecloud · 2023-08-09T16:01:39Z

[java_showcase_unit_tests] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

meltsufin

Thanks for adding the tests!

ci: add downstream compatibility checks

1b2ea99

product-auto-label Bot added the size: m Pull request size is medium. label Aug 7, 2023

burkedavison added do not merge Indicates a pull request not ready for merge, due to either quality or timing. and removed size: m Pull request size is medium. labels Aug 7, 2023

ci: add downstream compatibility github actions job

c10426b

product-auto-label Bot added the size: m Pull request size is medium. label Aug 7, 2023

burkedavison added 9 commits August 7, 2023 16:37

ci: install xmllint

f84fa8e

ci: install xmllint

54cad13

ci: install xmllint

118da64

ci: provide default REPOS_UNDER_TEST value

76b6273

ci: exit script in google-cloud-java directory

3ad8b52

ci: split downstream compatibility into parallel tests

68250b6

ci: reduce log noise from sdk-platform-java install step

45bac88

ci: don't fail fast in downstream testing

c333557

ci: perform downstream testing on last release, not HEAD

b2bfb88

burkedavison added kokoro:force-run Add this label to force Kokoro to re-run the tests. owlbot:run Add this label to trigger the Owlbot post processor. labels Aug 8, 2023

gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Aug 8, 2023

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

burkedavison marked this pull request as ready for review August 8, 2023 16:31

burkedavison requested a review from a team August 8, 2023 16:31

burkedavison removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 8, 2023

burkedavison requested review from blakeli0 and meltsufin August 8, 2023 16:44

meltsufin approved these changes Aug 8, 2023

View reviewed changes

blakeli0 reviewed Aug 8, 2023

View reviewed changes

blakeli0 approved these changes Aug 8, 2023

View reviewed changes

chore: improve comments

d8c2f1e

ci: add tests for helper scripts

9dc270c

product-auto-label Bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Aug 9, 2023

meltsufin approved these changes Aug 9, 2023

View reviewed changes

burkedavison merged commit f86eca2 into main Aug 17, 2023

burkedavison deleted the downstream-compatibility branch August 17, 2023 18:01

	# Use GCP Maven Mirror
	mkdir -p "${HOME}/.m2"
	cp settings.xml "${HOME}/.m2"

Conversation

burkedavison commented Aug 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meltsufin left a comment

Choose a reason for hiding this comment

Uh oh!

blakeli0 commented Aug 8, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

burkedavison commented Aug 8, 2023

Uh oh!

blakeli0 left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Aug 9, 2023

Uh oh!

sonarqubecloud Bot commented Aug 9, 2023

Uh oh!

sonarqubecloud Bot commented Aug 9, 2023

Uh oh!

meltsufin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

burkedavison commented Aug 7, 2023 •

edited

Loading