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

mpeddada1 · 2023-03-02T21:00:23Z

Coverage

In order to get test coverage of gax within the showcase and gax modules, a new module coverage-report needed to be created. The coverage module declares gapic-showcase (with a test scope), gax, gax-grpc, gax-httpjson as dependencies and relies on jacoco:report-aggregate to collect aggregate metrics.

Unit test coverage

Gathering just unit test metrics requires mvn clean test -DenableTestCoverage to be called.

Integration test coverage

Gathering just integration test metrics requires mvn clean verify -DskipUnitTests -DenableTestCoverage -Penable-integration-tests to be called. The DskipUnitTests is needed skip the maven-surefire-plugin.

mpeddada1 · 2023-03-02T21:27:14Z

graalvm downstream check is resulting in Error: Test configuration file wasn't found. Make sure that test execution wasn't skipped.

pom.xml

burkedavison · 2023-03-06T16:12:48Z

gax-java/pom.xml

        <artifactId>maven-jar-plugin</artifactId>
        <executions>
          <execution>
+            <phase>test</phase>


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?

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.

@blakeli0 and @suztomo for visibility. Great work!

burkedavison · 2023-03-06T16:15:15Z

gapic-generator-java-pom-parent/pom.xml

    </profile>
+
+    <profile>
+      <id>test-coverage</id>


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?

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

blakeli0 · 2023-03-09T16:30:51Z

pom.xml

    <module>java-common-protos</module>
    <module>java-iam</module>
    <module>gapic-generator-java-bom</module>
+    <module>showcase</module>


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?

@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?

Good idea! Done.

mpeddada1 · 2023-03-13T17:07:58Z

@suztomo @blakeli0 Ready for another round of review.

suztomo · 2023-03-15T15:03:00Z

As described in #1270, calling mvn test was resulting in compilation errors for gax test-jars.

#1474 might resolve this problem. If you had to implement some workaround for the GAX test JARs, would you revisit it? If not applicable, then that's good.

…o test-coverage

mpeddada1 · 2023-03-15T15:29:29Z

#1474 might resolve this problem. If you had to implement some workaround for the GAX test JARs, would you revisit it? If not applicable, then that's good.

Awesome! Thank you. It works without the workaround:)

suztomo

Would you add a check in .github/workflows (either existing one or new file) to run the two commands for unit tests and integration tests.

suztomo · 2023-03-15T16:06:00Z

coverage-report/README.md

@@ -0,0 +1,16 @@
+## Coverage Report
+
+This module is meant to gather aggregate jacoco test coverage metrics across the `gax-java` and `showcase` modules.


Suggested change

This module is meant to gather aggregate jacoco test coverage metrics across the `gax-java` and `showcase` modules.

This module gathers aggregated jacoco test coverage metrics across the `gax-java` and `showcase` modules.

Done, thank you!

suztomo · 2023-03-15T16:07:08Z

coverage-report/README.md

+In order to view aggregate unit test coverage of GAX in both `gax-java` and `showcase`:
+
+1. At the root of the repository, run `mvn clean test -DenableTestCoverage`.
+2. The metrics can be found at `${HOME}/gapic-generator-java/coverage-report/target/site/jacoco-aggregate/index.html`


I believe the home directory is irrelevant?

Agreed, starting with gapic-generator-java should be sufficient.

coverage-report/README.md

gapic-generator-java-pom-parent/pom.xml

mpeddada1 · 2023-03-15T21:28:19Z

Would you add a check in .github/workflows (either existing one or new file) to run the two commands for unit tests and integration tests.

Yup! I modified the Unit Tests check in ci-maven.yaml to use mvn test instead of mvn install now that the issue has been fixed. Additionally, the PR after this (#1482) is going to add both commands (mvn test and mvn verify) on the whole repo to sonar.yaml for piping the metrics to Sonarcloud.

suztomo

I meant a check to ensure “ -DenableTestCoverage” works fine. Does it exist already via SonarCloud?

.github/workflows/ci-maven.yaml

sonarqubecloud · 2023-03-16T00:57:18Z

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

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

…ase and gax modules (#1430) * chore: add aggregate test coverage collection for showcase and gax

chore: add aggregate test coverage collection for showcase and gax

d7c919a

product-auto-label bot added the size: m Pull request size is medium. label Mar 2, 2023

mpeddada1 added 11 commits March 2, 2023 17:28

add profile to collect test coverage

135022c

add fmt-maven-plugin

d2be02f

add maven compiler version property

22cda89

skip fmt

cd0de15

add version to fmt plugin

cddb9f2

add jar plugin to showcase

c116da1

add jar plugin to gapic-showcase

208cfff

empty commit to rerun builds

02ef0cb

empty commit

7f720b8

fix syntax in dep declaration

68557fe

remove spaces

57ddf93

mpeddada1 marked this pull request as ready for review March 6, 2023 15:24

mpeddada1 requested a review from a team as a code owner March 6, 2023 15:24

mpeddada1 requested a review from burkedavison March 6, 2023 15:24

burkedavison reviewed Mar 6, 2023

View reviewed changes

mpeddada1 added 2 commits March 6, 2023 13:26

add README for coverage-report module

fcc5257

replace monorepo with repsitory in README

45a0987

mpeddada1 requested review from blakeli0 and suztomo March 8, 2023 19:55

burkedavison approved these changes Mar 8, 2023

View reviewed changes

mpeddada1 changed the title ~~chore: add aggregate test coverage collection for showcase and gax~~ chore: add aggregate test coverage collection of gax within the showcase and gax modules Mar 8, 2023

blakeli0 reviewed Mar 9, 2023

View reviewed changes

add profile to specify modules

47cf349

mpeddada1 requested a review from blakeli0 March 10, 2023 21:45

mpeddada1 mentioned this pull request Mar 13, 2023

chore: add sonarcloud reporting for aggregate test coverage in gax, api-common and showcase #1482

Merged

blakeli0 approved these changes Mar 15, 2023

View reviewed changes

mpeddada1 added 2 commits March 15, 2023 11:14

Merge branch 'main' of github.com:googleapis/gapic-generator-java int…

d86667f

…o test-coverage

undo workaround

42f13c0

suztomo requested changes Mar 15, 2023

View reviewed changes

modify unit test step and clean up README

f813b69

suztomo approved these changes Mar 15, 2023

View reviewed changes

mpeddada1 added 3 commits March 15, 2023 16:23

try calling mvn install before bazelisk

47fc1db

undo mvn compile replacement

879784b

install mvn modules before integration tests

5db0d69

suztomo reviewed Mar 15, 2023

View reviewed changes

.github/workflows/ci-maven.yaml Show resolved Hide resolved

add enableTestCoverage parameter and fix showcase compilation

31365de

mpeddada1 merged commit bf68311 into main Mar 16, 2023

mpeddada1 deleted the test-coverage branch March 16, 2023 15:23

lqiu96 pushed a commit that referenced this pull request Mar 20, 2023

chore: add aggregate test coverage collection of gax within the showc…

4b54422

…ase and gax modules (#1430) * chore: add aggregate test coverage collection for showcase and gax

		@@ -0,0 +1,16 @@
		## Coverage Report

		This module is meant to gather aggregate jacoco test coverage metrics across the `gax-java` and `showcase` modules.

	This module is meant to gather aggregate jacoco test coverage metrics across the `gax-java` and `showcase` modules.
	This module gathers aggregated jacoco test coverage metrics across the `gax-java` and `showcase` modules.

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

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

Uh oh!

Conversation

mpeddada1 commented Mar 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage

Unit test coverage

Integration test coverage

Uh oh!

mpeddada1 commented Mar 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mpeddada1 Mar 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

mpeddada1 commented Mar 13, 2023

Uh oh!

suztomo commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpeddada1 commented Mar 15, 2023

Uh oh!

suztomo left a comment

Choose a reason for hiding this comment

Uh oh!

suztomo Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mpeddada1 commented Mar 15, 2023

Uh oh!

suztomo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Mar 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mpeddada1 commented Mar 2, 2023 •

edited

Loading

mpeddada1 commented Mar 2, 2023 •

edited

Loading

mpeddada1 Mar 6, 2023 •

edited

Loading

suztomo commented Mar 15, 2023 •

edited

Loading

suztomo Mar 15, 2023 •

edited

Loading