Skip to content

chore: use sdk-platform-java-config in downstream checks for spanner and pubsub; update junit-vintage to 5.10.2 #746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mpeddada1 merged 7 commits into from
Feb 13, 2024
Merged

chore: use sdk-platform-java-config in downstream checks for spanner and pubsub; update junit-vintage to 5.10.2 #746

mpeddada1 merged 7 commits into from
Feb 13, 2024

Conversation

@mpeddada1
Copy link
Contributor

@mpeddada1 mpeddada1 commented Feb 12, 2024 •
edited
Loading

For java-spanner, java-pubsub dependencies dw check:

  • Use sdk-platform-java-config.
  • Since we are verifying changes in java-shared-config, explicitly set shared-dependencies in sdk-platform-java-config to currently released version to ensure that we're not testing any SNAPSHOT changes from sdk-platform-java.

Additional fix to address Graalvm downstream check failure:

The downstream GraalVM checks have been failing with:

Exception in thread "main" java.lang.NoSuchMethodError: org.junit.platform.engine.TestDescriptor.getAncestors()
Step #2: 	at org.junit.platform.launcher.core.StackTracePruningEngineExecutionListener.getTestClassNames(StackTracePruningEngineExecutionListener.java:50)
Step #2: 	at org.junit.platform.launcher.core.StackTracePruningEngineExecutionListener.executionFinished(StackTracePruningEngineExecutionListener.java:39)
Step #2: 	at org.junit.platform.launcher.core.DelegatingEngineExecutionListener.executionFinished(DelegatingEngineExecutionListener.java:46)
Step #2: 	at org.junit.platform.launcher.core.OutcomeDelayingEngineExecutionListener.reportEngineFailure(OutcomeDelayingEngineExecutionListener.java:83)
Step #2: 	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:203)
Step #2: 	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:169)
Step #2: 	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:93)
Step #2: 	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:58)
Step #2: 	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:141)
Step #2: 	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:57)
Step #2: 	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:103)
Step #2: 	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:94)
Step #2: 	at org.junit.platform.launcher.core.DelegatingLauncher.execute(DelegatingLauncher.java:52)
Step #2: 	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:70)
Step #2: 	at org.graalvm.junit.platform.NativeImageJUnitLauncher.execute(NativeImageJUnitLauncher.java:74)
Step #2: 	at org.graalvm.junit.platform.NativeImageJUnitLauncher.main(NativeImageJUnitLauncher.java:129)

This error occurs starting with native-maven-plugin:0.9.24 which introduced some changes to be compatible with junit:5.10.0 (graalvm/native-build-tools#472).

This PR updates the junit-vintage-engine to the latest version.

@mpeddada1 mpeddada1 requested a review from a team as a code owner February 12, 2024 21:36
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Feb 12, 2024
@mpeddada1 mpeddada1 changed the title chore: use sdk-platform-java-config in downstream checks for spanner and pubsub chore: use sdk-platform-java-config in downstream checks for spanner and pubsub; update junit-vintage to 5.10.2 Feb 13, 2024
alicejli
alicejli reviewed Feb 13, 2024
View reviewed changes
burkedavison
burkedavison reviewed Feb 13, 2024
View reviewed changes
.kokoro/client-library-check.sh Outdated
setns x=http://maven.apache.org/POM/4.0.0
cd .//x:artifactId[text()="google-cloud-shared-config"]
cd ../x:version
set ${JAVA_SHARED_CONFIG_VERSION}
Copy link
Member

@burkedavison burkedavison Feb 13, 2024

Choose a reason for hiding this comment

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

nit: from a generic, function design standpoint... using parameters should be preferred over using global state, as it makes the contract of the function when seeing it in use explicit rather than implicit.

In other words:

replace_java_shared_config_version pom.xml 2.3.4 # I can read this an know exactly what is happening.

vs

replace_java_shared_config_version # I don't know what this affects w/o going to read the function

See also "pure functions" and their many benefits.

Copy link
Contributor Author

@mpeddada1 mpeddada1 Feb 13, 2024

Choose a reason for hiding this comment

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

Good point, I've refactored this to accept args instead of using global variables.

@mpeddada1 mpeddada1 mentioned this pull request Feb 13, 2024
Closed
@mpeddada1 mpeddada1 merged commit 2b6e931 into main Feb 13, 2024
@mpeddada1 mpeddada1 deleted the fix-dw-sdk-platform-java-config branch February 13, 2024 19:38
@mpeddada1 mpeddada1 mentioned this pull request Feb 14, 2024
Closed
@mpeddada1 mpeddada1 mentioned this pull request Feb 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@burkedavison burkedavison burkedavison approved these changes

+1 more reviewer

@alicejli alicejli alicejli approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size: m Pull request size is medium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mpeddada1 @burkedavison @alicejli