Myasuka commented on a change in pull request #18840:
URL: https://github.com/apache/flink/pull/18840#discussion_r815669003
##########
File path:
flink-libraries/flink-state-processing-api/src/test/java/org/apache/flink/state/api/output/SnapshotUtilsTest.java
##########
@@ -45,18 +49,51 @@
private static final List<String> EXPECTED_CALL_OPERATOR_SNAPSHOT =
Arrays.asList("prepareSnapshotPreBarrier", "snapshotState",
"notifyCheckpointComplete");
+ private static final List<SnapshotType> EXPECTED_SNAPSHOT_TYPE = new
ArrayList<>();
+
@Rule public TemporaryFolder folder = new TemporaryFolder();
private static final List<String> ACTUAL_ORDER_TRACKING =
Collections.synchronizedList(new
ArrayList<>(EXPECTED_CALL_OPERATOR_SNAPSHOT.size()));
+ private static final List<SnapshotType> ACTUAL_SNAPSHOT_TYPE = new
ArrayList<>();
+
+ @Before
+ public void initializeVariable() {
+ ACTUAL_ORDER_TRACKING.clear();
+ EXPECTED_SNAPSHOT_TYPE.clear();
+ ACTUAL_SNAPSHOT_TYPE.clear();
+ }
+
@Test
public void testSnapshotUtilsLifecycle() throws Exception {
+
EXPECTED_SNAPSHOT_TYPE.add(SavepointType.savepoint(SavepointFormatType.DEFAULT));
StreamOperator<Void> operator = new LifecycleOperator();
Path path = new Path(folder.newFolder().getAbsolutePath());
SnapshotUtils.snapshot(operator, 0, 0L, true, false, new
Configuration(), path);
+ Assert.assertEquals(EXPECTED_SNAPSHOT_TYPE, ACTUAL_SNAPSHOT_TYPE);
+ Assert.assertEquals(EXPECTED_CALL_OPERATOR_SNAPSHOT,
ACTUAL_ORDER_TRACKING);
+ }
+
+ @Test
+ public void testSnapshotUtilsLifecycleWithNativeSavepoint() throws
Exception {
Review comment:
I think we can reduce duplicate code to make one private method
`testSnapshotUtilsLifecycle(SavepointFormatType savepointFormatType)` and let
two public test mthod just call it via
`testSnapshotUtilsLifecycle(SavepointFormatType.DEFAULT)` and
`testSnapshotUtilsLifecycle(SavepointFormatType.NATIVE)`.
By doing so, we can also drop the `initializeVariable` method.
##########
File path:
flink-libraries/flink-state-processing-api/src/test/java/org/apache/flink/state/api/output/SnapshotUtilsTest.java
##########
@@ -45,18 +49,51 @@
private static final List<String> EXPECTED_CALL_OPERATOR_SNAPSHOT =
Arrays.asList("prepareSnapshotPreBarrier", "snapshotState",
"notifyCheckpointComplete");
+ private static final List<SnapshotType> EXPECTED_SNAPSHOT_TYPE = new
ArrayList<>();
+
@Rule public TemporaryFolder folder = new TemporaryFolder();
private static final List<String> ACTUAL_ORDER_TRACKING =
Collections.synchronizedList(new
ArrayList<>(EXPECTED_CALL_OPERATOR_SNAPSHOT.size()));
+ private static final List<SnapshotType> ACTUAL_SNAPSHOT_TYPE = new
ArrayList<>();
Review comment:
I don't think we need a list to hold the snapshot type. Moreover, the
expected snapshot type is what we passed in, an no need to introduce another
list of `EXPECTED_SNAPSHOT_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...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
