fix(java): include integration tests for native image testing #720
Conversation
product-auto-label
bot
added
the
api: spanner
mpeddada1
force-pushed
the
native-run-it
branch
from
7fe2b0e
to
f2548f8
Compare
| <excludedGroups>com.google.cloud.spanner.IntegrationTest</excludedGroups> | ||
| <exclude>com.google.cloud.spanner.IntegrationTest</exclude> |
There was a problem hiding this comment.
https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#excludedGroups
(TestNG/JUnit47 provider with JUnit4.8+ only and JUnit5+ provider since 2.22.0) Excluded groups/categories/tags. Any methods/classes/etc with one of the groups/categories/tags specified in this list will specifically not be run.
Was that wrong usage of excludedGroups before this PR?
There was a problem hiding this comment.
Ideally, we want to run integration tests and unit tests (ending in *ClientTest) with the native profile. For this purpose, we tend to ignore any exclusions that take place in other maven-surefire-plugin configurations within the java-shared-config pom or other child poms: https://github.com/googleapis/java-shared-config/blob/ac44ba11d2da2f649670867c520f90253a8a41ff/pom.xml#L807, so that we can explicitly include these tests. However, with the use of excludedGroups in java-spanner-jdbc, this overriding of the exclusions gets ignored, therefore causing no integration tests to run in native mode. Changing this to exclude fixes this issue.
There was a problem hiding this comment.
Was that wrong usage of excludedGroups before this PR?
it seems correct usage to skip the integration test marked with @Category(ParallelIntegrationTest.class) in this repository.
@mpeddada1 With this change in this PR, does mvn test skip the tests marked with @Category(ParallelIntegrationTest.class), such as ITJdbcSimpleStatementsTest and ITJdbcReadOnlyTest?
There was a problem hiding this comment.
Good question. Just ran mvn test with this change and can confirm that tests annotated with @Category(ParallelIntegrationTest.class) are skipped. Only unit tests are executed.
|
Thank you for the review @Neenu1995 and @suztomo! |
🤖 I have created a release *beep* *boop* --- ### [2.5.9](v2.5.8...v2.5.9) (2022-02-03) ### Bug Fixes * **java:** replace excludedGroup with exclude ([#720](#720)) ([7f13c88](7f13c88)) ### Dependencies * **java:** update actions/github-script action to v5 ([#1339](#1339)) ([#725](#725)) ([4f96ec1](4f96ec1)) * update actions/github-script action to v5 ([#724](#724)) ([5c1d6ff](5c1d6ff)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v2.7.0 ([#728](#728)) ([b0a32d8](b0a32d8)) * update opencensus.version to v0.31.0 ([#727](#727)) ([fce0770](fce0770)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Some background:
We want to run integration tests and unit tests (ending in *ClientTest) with the native profile. For this purpose, we want to ignore any exclusions that take place in other maven-surefire-plugin configurations within the java-shared-config pom or other child poms so that we can explicitly include these tests. However, using
excludedGroupsin java-spanner-jdbc causes the overriding of the exclusion to be ignored. This results in integration tests being skipped in native mode which leads to native-image testing to fail with the message:Test configuration file wasn't found. Make sure that test execution wasn't skipped.This change replaces the use of
excludedGroupswithexclude, which allows the the maven-surefire-plugin configuration for thenativeprofile to take precedence whenmvn test -P nativeis called. Manual testing withmvn testresults in only unit tests being run, therefore showing no change in current behavior for standard Java.Fixes #719 ☕️