RyuSA commented on code in PR #27413:
URL: https://github.com/apache/beam/pull/27413#discussion_r1260370611
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProtoTest.java:
##########
@@ -1049,6 +1050,58 @@ public void testIntegerTypeConversion() throws
DescriptorValidationException {
}
}
+ @Test
+ public void testTimestampTypeConversion() throws
DescriptorValidationException {
Review Comment:
I think the test needed for this fix is the Timestamp input value bounds
test.
Since they were originally designed to properly parse input values of
various "formats", I thought it would be a little too much to include a
boundary test there as well.🤔
But, yes I can add the test case to the general test instead of
testTimestampTypeConversion. Do you think I should add the test case to both of
with and without a field named "f"? (= e.g BASE_TABLE_SCHEMA and
BASE_TABLE_SCHEMA_NO_F) Or one of them?
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/TableRowToStorageApiProtoTest.java:
##########
@@ -1049,6 +1050,58 @@ public void testIntegerTypeConversion() throws
DescriptorValidationException {
}
}
+ @Test
+ public void testTimestampTypeConversion() throws
DescriptorValidationException {
Review Comment:
I think the test needed for this fix is the Timestamp input value bounds
test.
Since they were originally designed to properly parse input values of
various "formats", I thought it would be a little too much to include a
boundary test there as well.🤔
But, yes I can add the test case to the general test instead of
testTimestampTypeConversion. Do you think I should add the test case to both of
with and without a field named "f"? (= e.g BASE_TABLE_SCHEMA and
BASE_TABLE_SCHEMA_NO_F) Or one of them?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
