stevenzwu commented on code in PR #9728:
URL: https://github.com/apache/iceberg/pull/9728#discussion_r1576706517
##########
core/src/test/java/org/apache/iceberg/TestFileScanTaskParser.java:
##########
@@ -33,23 +33,64 @@ public void testNullArguments() {
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Invalid file scan task: null");
- Assertions.assertThatThrownBy(() -> FileScanTaskParser.fromJson(null,
true))
+ Assertions.assertThatThrownBy(() -> FileScanTaskParser.fromJson((String)
null, true))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Invalid JSON string for file scan task: null");
+
+ Assertions.assertThatThrownBy(() -> ScanTaskParser.toJson(null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid scan task: null");
+
+ Assertions.assertThatThrownBy(() -> ScanTaskParser.fromJson(null, true))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid JSON string for scan task: null");
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
- public void testParser(boolean caseSensitive) {
+ public void testFileScanTaskParser(boolean caseSensitive) {
PartitionSpec spec = TestBase.SPEC;
- FileScanTask fileScanTask = createScanTask(spec, caseSensitive);
+ FileScanTask fileScanTask = createFileScanTask(spec, caseSensitive);
String jsonStr = FileScanTaskParser.toJson(fileScanTask);
- Assertions.assertThat(jsonStr).isEqualTo(expectedFileScanTaskJson());
+
Assertions.assertThat(jsonStr).isEqualTo(fileScanTaskJsonWithoutTaskType());
FileScanTask deserializedTask = FileScanTaskParser.fromJson(jsonStr,
caseSensitive);
assertFileScanTaskEquals(fileScanTask, deserializedTask, spec,
caseSensitive);
}
- private FileScanTask createScanTask(PartitionSpec spec, boolean
caseSensitive) {
+ /** Test backward compatibility where task-type field is absent from the
JSON string */
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testFileScanTaskParserWithoutTaskTypeField(boolean
caseSensitive) {
+ PartitionSpec spec = TestBase.SPEC;
+ FileScanTask fileScanTask = createFileScanTask(spec, caseSensitive);
+ FileScanTask deserializedTask =
+ FileScanTaskParser.fromJson(fileScanTaskJsonWithoutTaskType(),
caseSensitive);
+ assertFileScanTaskEquals(fileScanTask, deserializedTask, spec,
caseSensitive);
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testScanTaskParser(boolean caseSensitive) {
Review Comment:
`ScanTaskParser` is the facade/dispatch class. I don't want to add all task
types testing into a single `TestScanTaskParser`. I think it is a bit cleaner
to have one test class for each task type.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
