Skip to content
This repository was archived by the owner on Apr 7, 2026. It is now read-only.

fix(java): Use Junit.assert instead of Hamcrest to fix Native Image testing#1746

Closed
mpeddada1 wants to merge 14 commits intomainfrom
fix-native-test
Closed

fix(java): Use Junit.assert instead of Hamcrest to fix Native Image testing#1746
mpeddada1 wants to merge 14 commits intomainfrom
fix-native-test

Conversation

@mpeddada1
Copy link
Copy Markdown
Contributor

@mpeddada1 mpeddada1 commented Mar 11, 2022 •
edited
Loading

This PR is needed to address failures related to Native Image compilation.
Calling mvn test -Dtest=com.google.cloud.spanner.connection.it.ITQueryOptionsTest -Pnative -DfailIfNoTests=false locally results in the following error:

JUnit Vintage:ITQueryOptionsTest:verifiesQueryOptions
    MethodSource [className = 'com.google.cloud.spanner.connection.it.ITQueryOptionsTest', methodName = 'verifiesQueryOptions', methodParameterTypes = '']
    => java.lang.Error: Cannot determine correct type for matchesSafely() method.
       org.hamcrest.internal.ReflectiveTypeFinder.findExpectedType(ReflectiveTypeFinder.java:49)
       org.hamcrest.TypeSafeMatcher.<init>(TypeSafeMatcher.java:40)
       org.hamcrest.TypeSafeMatcher.<init>(TypeSafeMatcher.java:22)
       org.hamcrest.core.SubstringMatcher.<init>(SubstringMatcher.java:13)
       org.hamcrest.core.StringStartsWith.<init>(StringStartsWith.java:13)
       org.hamcrest.core.StringStartsWith.startsWith(StringStartsWith.java:38)
       org.hamcrest.CoreMatchers.startsWith(CoreMatchers.java:516)
       com.google.cloud.spanner.connection.SqlScriptVerifier.verifyExpectedException(SqlScriptVerifier.java:188)
       com.google.cloud.spanner.connection.AbstractSqlScriptVerifier.verifyStatement(AbstractSqlScriptVerifier.java:367)
       com.google.cloud.spanner.connection.AbstractSqlScriptVerifier.lambda$verifyStatementsInFile$0(AbstractSqlScriptVerifier.java:315)
       [...]

GraalVM isn't able to handle more complex assertions in Hamcrest such as assertThat().startsWith() and assertThat().contains(). The Truth framework and Junit, on the other hand, are compatible.

cc @ansh0l @meltsufin

@mpeddada1 mpeddada1 requested a review from a team March 11, 2022 00:50
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner API. label Mar 11, 2022
@mpeddada1 mpeddada1 requested a review from a team March 11, 2022 00:53
ansh0l
ansh0l approved these changes Mar 29, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ansh0l ansh0l left a comment

Choose a reason for hiding this comment

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

lgtm, Since this is a change of just the assertion library. Adding a do not merge label since we intend to merge all the native image related PRs in a single go.

Comment thread google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SqlScriptVerifier.java Outdated
import static org.hamcrest.CoreMatchers.startsWith;
import static org.hamcrest.MatcherAssert.assertThat;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
Copy link
Copy Markdown
Contributor

@ansh0l ansh0l Mar 29, 2022

Choose a reason for hiding this comment

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

@mpeddada1 I'm trying to understand the import for Truth: https://github.com/googleapis/java-spanner/blob/aac438e02e841c6ee44362f5f319505d0264ceda/google-cloud-spanner/pom.xml, but can't find it. Can you point me to the right pom?

Copy link
Copy Markdown
Member

@suztomo suztomo Mar 29, 2022

Choose a reason for hiding this comment

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

Is this you're looking for?

java-spanner/google-cloud-spanner/pom.xml

Line 315 in aac438e

<artifactId>truth</artifactId> 

    <dependency>
      <groupId>com.google.truth</groupId>
      <artifactId>truth</artifactId>
      <scope>test</scope>
    </dependency>

@ansh0l ansh0l added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 29, 2022
@mpeddada1 mpeddada1 changed the title fix(java): Use Truth instead of Hamcrest to fix Native Image testing fix(java): Use Junit.assert instead of Hamcrest to fix Native Image testing Apr 4, 2022
@ansh0l
Copy link
Copy Markdown
Contributor

ansh0l commented May 23, 2022

Closing since native image changes have been released with #1878

@ansh0l ansh0l closed this May 23, 2022
@ansh0l ansh0l reopened this May 23, 2022
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label May 23, 2022
@ansh0l
Copy link
Copy Markdown
Contributor

ansh0l commented May 23, 2022

Closing since native image changes have been released with #1878

@ansh0l ansh0l closed this May 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@meltsufin meltsufin Awaiting requested review from meltsufin

@Neenu1995 Neenu1995 Awaiting requested review from Neenu1995

2 more reviewers

@suztomo suztomo suztomo approved these changes

@ansh0l ansh0l ansh0l approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: spanner Issues related to the googleapis/java-spanner API. do not merge Indicates a pull request not ready for merge, due to either quality or timing. size: s Pull request size is small.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mpeddada1 @ansh0l @suztomo