chore: Add Showcase Callables IT by lqiu96 · Pull Request #1483 · googleapis/sdk-platform-java

lqiu96 · 2023-03-13T17:24:40Z

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)

Part of #1439 ☕️

…e-plugin to v3.0.0 (#1488) Co-authored-by: Lawrence Qiu <[email protected]>

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Lawrence Qiu <[email protected]>

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

lqiu96 · 2023-03-22T20:39:06Z

Updating this PR following Mridula's PR: #1501

mpeddada1 · 2023-03-22T20:52:31Z

showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITUnaryCallable.java

+   */
+  @Test
+  public void testHttpJson_serverResponseError_throwsException() {
+    StatusCode.Code notFoundStatusCode = StatusCode.Code.NOT_FOUND;


You may have already covered this in the javadoc but is there an advantage of using NOT_FOUND over CANCELLED? Mostly wondering if the grpc tests should also be switched to use this.

I just picked NOT_FOUND instead of CANCELLED since the docs seem to suggest that CANCELLED status is from a user action (https://chromium.googlesource.com/external/github.com/grpc/grpc/+/refs/tags/v1.21.4-pre1/doc/statuscodes.md). Any error code should be fine since we just need the server to respond back with the error.

Whether we choose NOT_FOUND or CANCELLED, let's please make this and testGrpc_serverResponseError_throwsException consistent so we aren't implying a difference in behavior. If we want to test both behaviors, let's do so explicitly.

I'll update to CANCELLED.

mpeddada1 · 2023-03-22T20:55:49Z

showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITUnaryCallable.java

+  @Test
+  public void testHttpJson_serverResponseError_throwsException() {
+    StatusCode.Code notFoundStatusCode = StatusCode.Code.NOT_FOUND;
+    ApiException exception =


Perhaps we can make the exception more specific here? To make sure that the test doesn't pass for a false positive case since ApiException can occur for a number of reasons.

Yeah changed.

mpeddada1 · 2023-03-22T21:02:48Z

showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITUnaryCallable.java

+            () ->
+                httpJsonClient.echo(
+                    EchoRequest.newBuilder()
+                        .setError(Status.newBuilder().setCode(notFoundStatusCode.ordinal()).build())


nit: this may not work but can the inner build() call be omitted when we nest builders? i.e. does EchoRequest.newBuilder().setError(Status.newBuilder().setCode(notFoundStatusCode.ordinal())).build() work?

Moved it out to a variable

mpeddada1 · 2023-03-23T15:35:54Z

showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITUnaryCallable.java

+    assertThat(exception.getStatusCode().getCode()).isEqualTo(cancelledStatusCode);
+  }


nit: How about we use HttpJsonStatusCode.Code.CANCELLED here?

Sounds good. Updating.

sonarqubecloud · 2023-03-23T15:45:41Z

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

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

sonarqubecloud · 2023-03-23T15:48:24Z

[java_showcase_integration_tests] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

sonarqubecloud · 2023-03-23T15:52:21Z

[java_showcase_unit_tests] Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

mpeddada1

LGTM! Will let @burkedavison provide the final approval.

chore: Add Showcase Callables IT

b43a82a

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

chore: Add test scope

45bdea1

lqiu96 mentioned this pull request Mar 13, 2023

REGAPIC Showcase Coverage #1439

Closed

27 tasks

renovate-bot and others added 6 commits March 14, 2023 18:00

deps: update google http client dependencies to v1.43.1 (#1487)

5ef67e9

build(deps): update dependency org.apache.maven.plugins:maven-surefir…

ba9fb82

…e-plugin to v3.0.0 (#1488) Co-authored-by: Lawrence Qiu <[email protected]>

chore(main): release 2.15.3 (#1445)

1206c4c

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Lawrence Qiu <[email protected]>

chore(main): release 2.15.4-SNAPSHOT (#1489)

5de06df

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

chore: Refactor paged logic

640e35f

chore: Format ITCallables

46b0b1b

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Mar 14, 2023

Merge branch 'main' into main-showcase_callables

208c1ad

product-auto-label bot added size: m Pull request size is medium. and removed size: l Pull request size is large. labels Mar 14, 2023

chore: Format ITCallables

eed5def

lqiu96 mentioned this pull request Mar 16, 2023

chore: add unary-unary showcase test for gRPC support #1501

Merged

4 tasks

lqiu96 added 3 commits March 16, 2023 17:43

chore: Fix to not found status code

4d26862

Merge branch 'main' into main-showcase_callables

61ac41a

chore: Add HttpJson Unary Callable Showcase Test

ca777dd

product-auto-label bot added size: s Pull request size is small. and removed size: m Pull request size is medium. labels Mar 22, 2023

lqiu96 added 2 commits March 22, 2023 16:30

chore: Fix truth dependency

29c0516

chore: Update test name

2a7af58

lqiu96 requested review from blakeli0, burkedavison and mpeddada1 March 22, 2023 20:38

lqiu96 marked this pull request as ready for review March 22, 2023 20:39

lqiu96 requested a review from a team March 22, 2023 20:39

chore: Address code smell

5a13d48

mpeddada1 reviewed Mar 22, 2023

View reviewed changes

lqiu96 added 2 commits March 22, 2023 17:10

chore: Remove incorrect comment

a6a3676

chore: Update test to CANCELLED

533deec

mpeddada1 reviewed Mar 23, 2023

View reviewed changes

chore: Clean up PR

d3453a3

mpeddada1 approved these changes Mar 23, 2023

View reviewed changes

lqiu96 merged commit ff19831 into main Mar 23, 2023

lqiu96 deleted the main-showcase_callables branch March 23, 2023 20:53

		assertThat(exception.getStatusCode().getCode()).isEqualTo(cancelledStatusCode);
		}

Conversation

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

Uh oh!

Uh oh!

lqiu96 commented Mar 22, 2023

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!

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!

sonarqubecloud bot commented Mar 23, 2023

Uh oh!

sonarqubecloud bot commented Mar 23, 2023

Uh oh!

sonarqubecloud bot commented Mar 23, 2023

Uh oh!

mpeddada1 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lqiu96 commented Mar 13, 2023 •

edited

Loading