Skip to content

chore(datastore): Clean up a few test files with new V3 changes#12987

Merged
lqiu96 merged 2 commits intomainfrom
cleanup-transport
May 4, 2026
Merged

chore(datastore): Clean up a few test files with new V3 changes#12987
lqiu96 merged 2 commits intomainfrom
cleanup-transport

Conversation

@lqiu96
Copy link
Copy Markdown
Member

@lqiu96 lqiu96 commented May 4, 2026

gRPC Transport is the new default so there is no need to explicitly set it.

@lqiu96 lqiu96 requested a review from jinseopkim0 May 4, 2026 14:32
@lqiu96 lqiu96 requested a review from a team as a code owner May 4, 2026 14:32
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes explicit gRPC transport configurations from several test cases, relying instead on default settings, and simplifies redundant conditional logic in ITDatastoreClientSideMetrics and RemoteDatastoreHelper. The review feedback highlights that several variable names and comments still reference gRPC, which is now misleading given the changes. Recommendations include renaming these variables for clarity, removing stale comments, and consolidating redundant test logic.

@@ -298,19 +293,19 @@ public void testHostWithGrpcAndHttp() {
String customHost = "http://localhost:" + PORT;
DatastoreOptions grpcTransportOptionsCustomHost =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The variable name grpcTransportOptionsCustomHost is misleading as it no longer explicitly sets gRPC transport. Consider renaming it to something like defaultOptionsCustomHost to accurately reflect its configuration.

@lqiu96 lqiu96 enabled auto-merge (squash) May 4, 2026 14:49
@lqiu96 lqiu96 merged commit f9c122a into main May 4, 2026
129 of 131 checks passed
@lqiu96 lqiu96 deleted the cleanup-transport branch May 4, 2026 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants