Skip to content
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

chore: add aggregate test coverage collection of gax within the showcase and gax modules #1430

Merged
merged 22 commits into from
Mar 16, 2023
Merged

chore: add aggregate test coverage collection of gax within the showcase and gax modules #1430

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d7c919a
chore: add aggregate test coverage collection for showcase and gax
mpeddada1 Mar 2, 2023
135022c
add profile to collect test coverage
mpeddada1 Mar 2, 2023
d2be02f
add fmt-maven-plugin
mpeddada1 Mar 2, 2023
22cda89
add maven compiler version property
mpeddada1 Mar 2, 2023
cd0de15
skip fmt
mpeddada1 Mar 2, 2023
cddb9f2
add version to fmt plugin
mpeddada1 Mar 2, 2023
c116da1
add jar plugin to showcase
mpeddada1 Mar 2, 2023
208cfff
add jar plugin to gapic-showcase
mpeddada1 Mar 2, 2023
02ef0cb
empty commit to rerun builds
mpeddada1 Mar 2, 2023
7f720b8
empty commit
mpeddada1 Mar 3, 2023
68557fe
fix syntax in dep declaration
mpeddada1 Mar 3, 2023
57ddf93
remove spaces
mpeddada1 Mar 3, 2023
fcc5257
add README for coverage-report module
mpeddada1 Mar 6, 2023
45a0987
replace monorepo with repsitory in README
mpeddada1 Mar 6, 2023
47cf349
add profile to specify modules
mpeddada1 Mar 10, 2023
d86667f
Merge branch 'main' of github.com:googleapis/gapic-generator-java int…
mpeddada1 Mar 15, 2023
42f13c0
undo workaround
mpeddada1 Mar 15, 2023
f813b69
modify unit test step and clean up README
mpeddada1 Mar 15, 2023
47fc1db
try calling mvn install before bazelisk
mpeddada1 Mar 15, 2023
879784b
undo mvn compile replacement
mpeddada1 Mar 15, 2023
5db0d69
install mvn modules before integration tests
mpeddada1 Mar 15, 2023
31365de
add enableTestCoverage parameter and fix showcase compilation
mpeddada1 Mar 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions coverage-report/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
<?xml version='1.0' encoding='UTF-8'?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.cloud</groupId>
<artifactId>coverage-report</artifactId>
<packaging>pom</packaging>
<version>0.0.1-SNAPSHOT</version>
<name>Jacoco Aggregrate Test Coverage Report</name>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<checkstyle.skip>true</checkstyle.skip>
<clirr.skip>true</clirr.skip>
<enforcer.skip>true</enforcer.skip>
<maven.compiler.release>8</maven.compiler.release>
<fmt.skip>true</fmt.skip>
</properties>

<dependencies>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>gapic-showcase</artifactId>
<version>0.0.1-SNAPSHOT</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.api</groupId>
<artifactId>gax</artifactId>
<version>2.23.3-SNAPSHOT</version> <!-- {x-version-update:gax:current} -->
</dependency>
<dependency>
<groupId>com.google.api</groupId>
<artifactId>gax-grpc</artifactId>
<version>2.23.3-SNAPSHOT</version> <!-- {x-version-update:gax:current} -->
</dependency>
<dependency>
<groupId>com.google.api</groupId>
<artifactId>gax-httpjson</artifactId>
<version>0.108.3-SNAPSHOT</version> <!-- {x-version-update:gax:current} -->
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.8</version>
<executions>
<execution>
<id>unit-tests-report-aggregate</id>
<goals>
<goal>report-aggregate</goal>
</goals>
<phase>test</phase>
</execution>
<execution>
<id>integration-tests-report-aggregate</id>
<goals>
<goal>report-aggregate</goal>
</goals>
<phase>integration-test</phase>
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.coveo</groupId>
<artifactId>fmt-maven-plugin</artifactId>
<version>2.9</version>
</plugin>
</plugins>
</build>


</project>
78 changes: 78 additions & 0 deletions gapic-generator-java-pom-parent/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,86 @@
</plugins>
</build>
</profile>

<profile>
<id>test-coverage</id>
Copy link
Member

Choose a reason for hiding this comment

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

The PR description does a great job clarifying how this is meant to be used, but that may get lost in obscurity once the PR is merged. Could you add some information in an appropriate .md to explain the different expected workflows + report options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done. I've added a README to the coverage-report module.

<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.0.0-M8</version>
<configuration>
<!-- Excludes integration tests and smoke tests when unit tests are run -->
<excludes>
<exclude>**/*SmokeTest.java</exclude>
<exclude>**/IT*.java</exclude>
</excludes>
<reportNameSuffix>sponge_log</reportNameSuffix>
<argLine>${surefire.jacoco.args}</argLine>
<skipTests>${skipUnitTests}</skipTests>
</configuration>
</plugin>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<version>3.0.0-M8</version>
<executions>
<execution>
<goals>
<goal>integration-test</goal>
<goal>verify</goal>
</goals>
</execution>
</executions>
<configuration>
<forkedProcessTimeoutInSeconds>3600</forkedProcessTimeoutInSeconds>
<reportNameSuffix>sponge_log</reportNameSuffix>
<includes>
<include>**/IT*.java</include>
<include>**/*SmokeTest.java</include>
</includes>
<argLine>${failsafe.jacoco.args}</argLine>
</configuration>
</plugin>
</plugins>
</pluginManagement>
<plugins>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.8</version>
<executions>
<execution>
<id>unit-test-execution</id>
<goals>
<goal>prepare-agent</goal>
</goals>
<configuration>
<propertyName>surefire.jacoco.args</propertyName>
</configuration>
</execution>
<execution>
<id>integration-test-execution</id>
<phase>pre-integration-test</phase>
<goals>
<goal>prepare-agent</goal>
</goals>
<configuration>
<propertyName>failsafe.jacoco.args</propertyName>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>



mpeddada1 marked this conversation as resolved.
Show resolved Hide resolved
<repositories>
<repository>
<id>google-maven-central-copy</id>
Expand Down
1 change: 1 addition & 0 deletions gax-java/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@
<artifactId>maven-jar-plugin</artifactId>
<executions>
<execution>
<phase>test</phase>
Copy link
Member

Choose a reason for hiding this comment

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

Could you clarify what this is doing?

@blakeli0 , @suztomo : Is it possible that the <skip>true</skip> three lines down from here is the reason why we have to execute mvn install rather than mvn test to avoid linking errors?

Copy link
Contributor Author

@mpeddada1 mpeddada1 Mar 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Sure! This line helps resolve the compilation error we see when running mvn test at the root. The PR description goes over this briefly but upon comparing the mvn test with mvn install, I noticed that the jar:test-jar step which creates the testlib JAR that gapic-generator-java depends on was being executed when mvn install was called but after thesurefire:test step. On the other hand, it was not being invoked at all when mvn test was executed. This change forces the maven-jar-plugin of GAX to be executed at the test phase.

Copy link
Member

Choose a reason for hiding this comment

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

@blakeli0 and @suztomo for visibility. Great work!

<goals>
<goal>test-jar</goal>
</goals>
Expand Down
2 changes: 2 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
<module>java-common-protos</module>
<module>java-iam</module>
<module>gapic-generator-java-bom</module>
<module>showcase</module>
burkedavison marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding these two modules to the root pom here may have impact to our release, that they might be released automatically. @suztomo Do you have any concerns here?

Copy link
Member

Choose a reason for hiding this comment

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

@mpeddada1 To address the concern of accidental releases, can we define a profile so that the showcase module is part of the root project only when the profile is enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! Done.

<module>coverage-report</module>
</modules>
<!-- Do not deploy the aggregator POM -->
<build>
Expand Down
2 changes: 1 addition & 1 deletion showcase/gapic-showcase/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.cloud</groupId>
<artifactId>gapic-showcase</artifactId>
<version>0.0.1-SHAPSHOT</version>
<version>0.0.1-SNAPSHOT</version>
<packaging>jar</packaging>
<name>GAPIC Showcase Client</name>
<description>
Expand Down