changliiu commented on code in PR #35042:
URL: https://github.com/apache/beam/pull/35042#discussion_r2114535620
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/SpannerChangeStreamErrorTest.java:
##########
@@ -460,19 +460,27 @@ private void mockGetPartitionsAfter(Timestamp timestamp,
ResultSet getPartitionR
}
private void mockGetWatermark(Timestamp watermark) {
+ final String minWatermark = "min_watermark";
Review Comment:
I only edit one test case because:
1. Regarding verifying correctness, I run integration test, and also real
load against a local build connector, I print out this cache variable and I can
see it is updated successfully.
2. Regarding unit-test coverage. This PR has two parts - query change, and
lastCachedMinWatermark change. The former can be covered by this test. The
later actually is implementation details. According to
[go/tott/708](https://goto.google.com/tott/708), I think this can be considered
implementation detailed change and cannot be properly captured by unit-tests.
Please re-open if you have strong opinions here. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@beam.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
