chore: declare test-jar in GAX's testlib artifacts by JoeWang1127 · Pull Request #1474 · googleapis/sdk-platform-java · GitHub
Skip to content
This repository was archived by the owner on May 14, 2026. It is now read-only.

chore: declare test-jar in GAX's testlib artifacts#1474

Merged
JoeWang1127 merged 8 commits into
mainfrom
chore/enable-test-jar
Mar 10, 2023
Merged

chore: declare test-jar in GAX's testlib artifacts#1474
JoeWang1127 merged 8 commits into
mainfrom
chore/enable-test-jar

Conversation

@JoeWang1127

@JoeWang1127 JoeWang1127 commented Mar 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thank you for opening a Pull Request! For general contributing guidelines, please refer to contributing guide

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #1144 ☕️

@conventional-commit-lint-gcf

conventional-commit-lint-gcf Bot commented Mar 9, 2023 •
edited
Loading

Copy link
Copy Markdown

@product-auto-label product-auto-label Bot added the size: xs Pull request size is extra small. label Mar 9, 2023
@JoeWang1127 JoeWang1127 force-pushed the chore/enable-test-jar branch from 12bdf6d to 2484061 Compare March 9, 2023 22:52
@JoeWang1127 JoeWang1127 mentioned this pull request Mar 9, 2023
Closed
@JoeWang1127 JoeWang1127 linked an issue Mar 9, 2023 that may be closed by this pull request
After repository merge IntelliJ is showing some classes missing #1144
Closed
suztomo
suztomo reviewed Mar 10, 2023
View reviewed changes
Comment thread gapic-generator-java/pom.xml
<dependency>
<groupId>com.google.api</groupId>
<artifactId>gax</artifactId>
<type>test-jar</type>

@suztomo suztomo Mar 10, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add source code comment why this test-jar is important?

@JoeWang1127 JoeWang1127 Mar 10, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

@suztomo

suztomo commented Mar 10, 2023

Copy link
Copy Markdown
Member

Assuming this is the correct fix, will we need any change in the users of GAX’s testlib-classifier users?

@JoeWang1127

JoeWang1127 commented Mar 10, 2023

Copy link
Copy Markdown
Contributor Author

Assuming this is the correct fix, will we need any change in the users of GAX’s testlib-classifier users?

I think they need to add <type>test-jar</type> in their pom file.

I found out this in the failing Kokoro checks

[ERROR]   The project com.google.cloud:google-cloud-accessapproval:2.13.0 (/tmpfs/src/github/gapic-generator-java/google-cloud-java/java-accessapproval/google-cloud-accessapproval/pom.xml) has 3 errors
[ERROR]     'dependencies.dependency.version' for com.google.api:gax:jar:testlib is missing. @ line 80, column 21
[ERROR]     'dependencies.dependency.version' for com.google.api:gax-grpc:jar:testlib is missing. @ line 86, column 17
[ERROR]     'dependencies.dependency.version' for com.google.api:gax-httpjson:jar:testlib is missing. @ line 92, column 17

@suztomo

suztomo commented Mar 10, 2023

Copy link
Copy Markdown
Member

My own confirmation.

Remove local Maven repository cache: $ rm -rf ~/.m2/repository/com/google/api/gax*

Checkout main (not this pull request):

suztomo@suztomo-macbookpro.roam.internal ~/gapic-generator-java
~/gapic-generator-java $ git checkout main
Already on 'main'
Your branch is up to date with 'origin/main'.
suztomo@suztomo-macbookpro.roam.internal ~/gapic-generator-java
~/gapic-generator-java $ git pull
Already up to date.

Open gapic-generator-java project in IntelliJ and tried running the gapic-generator-java module's unit tests. It shows the error.

Screenshot 2023-03-10 at 10 19 09 AM

Now checkout this pull request.

~/gapic-generator-java $ gh pr checkout 1474

Run the unit tests in IntelliJ. It succeeded.

Screenshot 2023-03-10 at 10 25 48 AM

@suztomo suztomo changed the title chore: add test jar chore: declare test-jar in GAX's testlib artifacts Mar 10, 2023
@suztomo

suztomo commented Mar 10, 2023

Copy link
Copy Markdown
Member

@blakeli0 Can you confirm this pull request fixes the problem in your IntelliJ? (It worked form me)

suztomo
suztomo reviewed Mar 10, 2023
View reviewed changes
Comment thread gax-java/gax-bom/pom.xml
<groupId>com.google.api</groupId>
<artifactId>gax</artifactId>
<version>2.23.3-SNAPSHOT</version><!-- {x-version-update:gax:current} -->
<type>test-jar</type>

@suztomo suztomo Mar 10, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@JoeWang1127 'dependencies.dependency.version' for com.google.api:gax:jar:testlib is missing. @ line 80, column 21 came from the BOM's declaration of the artifacts. Do you really need that?

@JoeWang1127 JoeWang1127 Mar 10, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this line is needed to bring the test code.

@suztomo suztomo Mar 10, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A BOM doesn't determine what code goes to JAR file.

@JoeWang1127 JoeWang1127 Mar 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's an error in gapic-generator-java module if I delete this line and reload the project:

'dependencies.dependency.version' for com.google.api:gax-grpc:test-jar:testlib is missing.

@suztomo suztomo Mar 10, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK it’s for IntelliJ. Then it makes sense.

@blakeli0

blakeli0 commented Mar 10, 2023

Copy link
Copy Markdown
Contributor

@blakeli0 Can you confirm this pull request fixes the problem in your IntelliJ? (It worked form me)

Yes, I verified that it's working now with Joe's fix. Thank you so much @JoeWang1127 !

@product-auto-label product-auto-label Bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Mar 10, 2023
suztomo
suztomo reviewed Mar 10, 2023
View reviewed changes
Comment thread gax-java/gax-bom/pom.xml
<groupId>com.google.api</groupId>
<artifactId>gax</artifactId>
<version>2.23.3-SNAPSHOT</version><!-- {x-version-update:gax:current} -->
<classifier>testlib</classifier>

@suztomo suztomo Mar 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Trying an additional entry that does not carry "test-jar".

@JoeWang1127 JoeWang1127 Mar 10, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@suztomo thanks for the help.

@sonarqubecloud

sonarqubecloud Bot commented Mar 10, 2023

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@JoeWang1127 JoeWang1127 added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2023
@gcf-owl-bot gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2023
@sonarqubecloud

sonarqubecloud Bot commented Mar 10, 2023

Copy link
Copy Markdown

@JoeWang1127 JoeWang1127 added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2023
@gcf-owl-bot gcf-owl-bot Bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2023
@JoeWang1127 JoeWang1127 marked this pull request as ready for review March 10, 2023 19:58
@JoeWang1127 JoeWang1127 requested review from a team, blakeli0 and suztomo March 10, 2023 19:58
suztomo
suztomo approved these changes Mar 10, 2023
View reviewed changes

@suztomo suztomo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you.

@JoeWang1127 JoeWang1127 merged commit 68f24f2 into main Mar 10, 2023
@JoeWang1127 JoeWang1127 deleted the chore/enable-test-jar branch March 10, 2023 23:05
@suztomo suztomo mentioned this pull request Mar 15, 2023
Merged
suztomo pushed a commit that referenced this pull request Mar 21, 2023
@gcf-owl-bot
chore: Disable CLIRR checks on releas (#1474) (#727)
92a51f2 
Source-Link: googleapis/synthtool@7a220e2
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:6d4e3a15c62cfdcb823d60e16da7521e7c6fc00eba07c8ff12e4de9924a57d28
suztomo pushed a commit that referenced this pull request Mar 21, 2023
@gcf-owl-bot
chore: Disable CLIRR checks on releas (#1474) (#857)
a5a3eaa 
Source-Link: googleapis/synthtool@7a220e2
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:6d4e3a15c62cfdcb823d60e16da7521e7c6fc00eba07c8ff12e4de9924a57d28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers
@suztomo suztomo suztomo approved these changes
@blakeli0 blakeli0 blakeli0 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size: s Pull request size is small.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

After repository merge IntelliJ is showing some classes missing

3 participants

@JoeWang1127 @suztomo @blakeli0