snuyanzin commented on code in PR #21410:
URL: https://github.com/apache/flink/pull/21410#discussion_r1043364332
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -98,14 +100,14 @@ protected boolean supportsKeySerializerCheck() {
return true;
}
- @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
+ @TempDir public static File tempFolder;
- @Before
+ @BeforeEach
public void before() {
env = MockEnvironment.builder().build();
}
- @After
+ @AfterEach
public void after() {
Review Comment:
```suggestion
void after() {
```
Could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -156,9 +158,10 @@ public void
testKeyedValueStateRegistrationFailsIfNewStateSerializerIsIncompatib
fail("should have failed");
} catch (Exception expected) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
Review Comment:
I guess this could be replaced with usage of `assertThatThrownBy`
```java
assertThatThrownBy(
() ->
testKeyedValueStateUpgrade(
new ValueStateDescriptor<>(
stateName, new
TestType.V1TestTypeSerializer()),
new ValueStateDescriptor<>(
stateName,
new
TestType.IncompatibleTestTypeSerializer())))
.isInstanceOf(StateMigrationException.class);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -381,7 +385,7 @@ public void testKeyedMapStateAsIs() throws Exception {
stateName, IntSerializer.INSTANCE, new
TestType.V1TestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void testKeyedMapStateStateMigration() throws Exception {
Review Comment:
```suggestion
void testKeyedMapStateStateMigration() throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -540,7 +545,7 @@ private void testKeyedMapStateUpgrade(
// Tests for keyed priority queue state
//
-------------------------------------------------------------------------------
- @Test
+ @TestTemplate
public void
testPriorityQueueStateCreationFailsIfNewSerializerIsNotCompatible()
Review Comment:
```suggestion
void testPriorityQueueStateCreationFailsIfNewSerializerIsNotCompatible()
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -98,14 +100,14 @@ protected boolean supportsKeySerializerCheck() {
return true;
}
- @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
+ @TempDir public static File tempFolder;
- @Before
+ @BeforeEach
public void before() {
Review Comment:
```suggestion
void before() {
```
Could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -593,13 +598,14 @@ public void
testStateBackendRestoreFailsIfNewKeySerializerRequiresMigration() th
fail("should have failed");
} catch (Exception expected) {
// the new key serializer requires migration; this should fail the
restore
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
}
}
- @Test
+ @TestTemplate
public void
testStateBackendRestoreSucceedsIfNewKeySerializerRequiresReconfiguration()
Review Comment:
```suggestion
void
testStateBackendRestoreSucceedsIfNewKeySerializerRequiresReconfiguration()
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -119,7 +121,7 @@ public void after() {
// Tests for keyed ValueState
//
-------------------------------------------------------------------------------
- @Test
+ @TestTemplate
public void testKeyedValueStateMigration() throws Exception {
Review Comment:
```suggestion
void testKeyedValueStateMigration() throws Exception {
```
Could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -249,7 +252,7 @@ public void testKeyedListStateMigration() throws Exception {
new TestType.V2TestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void testKeyedListStateSerializerReconfiguration() throws Exception
{
Review Comment:
```suggestion
void testKeyedListStateSerializerReconfiguration() throws Exception {
```
Could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -131,7 +133,7 @@ public void testKeyedValueStateMigration() throws Exception
{
new TestType.V2TestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void testKeyedValueStateSerializerReconfiguration() throws
Exception {
Review Comment:
```suggestion
void testKeyedValueStateSerializerReconfiguration() throws Exception {
```
Could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -144,7 +146,7 @@ public void testKeyedValueStateSerializerReconfiguration()
throws Exception {
new
TestType.ReconfigurationRequiringTestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void
testKeyedValueStateRegistrationFailsIfNewStateSerializerIsIncompatible() {
Review Comment:
```suggestion
void
testKeyedValueStateRegistrationFailsIfNewStateSerializerIsIncompatible() {
```
Could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -690,7 +697,7 @@ private void testKeySerializerUpgrade(
// Tests for namespace serializer in keyed state backends
//
-------------------------------------------------------------------------------
- @Test
+ @TestTemplate
public void
testKeyedStateRegistrationFailsIfNewNamespaceSerializerRequiresMigration()
Review Comment:
```suggestion
void
testKeyedStateRegistrationFailsIfNewNamespaceSerializerRequiresMigration()
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -262,7 +265,7 @@ public void testKeyedListStateSerializerReconfiguration()
throws Exception {
new
TestType.ReconfigurationRequiringTestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void
testKeyedListStateRegistrationFailsIfNewStateSerializerIsIncompatible() {
Review Comment:
```suggestion
void
testKeyedListStateRegistrationFailsIfNewStateSerializerIsIncompatible() {
```
Could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -274,9 +277,10 @@ public void
testKeyedListStateRegistrationFailsIfNewStateSerializerIsIncompatibl
fail("should have failed");
} catch (Exception expected) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
Review Comment:
i guess it could be replaced with usage of `assertThatThrownBy`
something like
```java
assertThatThrownBy(
() ->
testKeyedListStateUpgrade(
new ListStateDescriptor<>(
stateName, new
TestType.V1TestTypeSerializer()),
new ListStateDescriptor<>(
stateName,
new
TestType.IncompatibleTestTypeSerializer())))
.isInstanceOf(StateMigrationException.class);
```
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -800,7 +809,7 @@ private void testNamespaceSerializerUpgrade(
// Operator state backend partitionable list state tests
//
-------------------------------------------------------------------------------
- @Test
+ @TestTemplate
public void testOperatorParitionableListStateMigration() throws Exception {
Review Comment:
```suggestion
void testOperatorParitionableListStateMigration() throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -237,7 +240,7 @@ private void testKeyedValueStateUpgrade(
// Tests for keyed ListState
//
-------------------------------------------------------------------------------
- @Test
+ @TestTemplate
public void testKeyedListStateMigration() throws Exception {
Review Comment:
```suggestion
void testKeyedListStateMigration() throws Exception {
```
Could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -370,7 +374,7 @@ private void testKeyedListStateUpgrade(
// Tests for keyed MapState
//
-------------------------------------------------------------------------------
- @Test
+ @TestTemplate
public void testKeyedMapStateAsIs() throws Exception {
Review Comment:
```suggestion
void testKeyedMapStateAsIs() throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -395,7 +399,7 @@ public void testKeyedMapStateStateMigration() throws
Exception {
new TestType.V2TestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void testKeyedMapStateSerializerReconfiguration() throws Exception {
Review Comment:
```suggestion
void testKeyedMapStateSerializerReconfiguration() throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -909,7 +918,7 @@ public void testOperatorUnionListStateMigration() throws
Exception {
new TestType.V2TestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void testOperatorUnionListStateSerializerReconfiguration() throws
Exception {
Review Comment:
```suggestion
void testOperatorUnionListStateSerializerReconfiguration() throws
Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -839,8 +848,8 @@ public void
testOperatorParitionableListStateRegistrationFailsIfNewSerializerIsI
fail("should have failed.");
} catch (Exception e) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent());
+ assertThat(ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -921,7 +930,7 @@ public void
testOperatorUnionListStateSerializerReconfiguration() throws Excepti
new
TestType.ReconfigurationRequiringTestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void
testOperatorUnionListStateRegistrationFailsIfNewSerializerIsIncompatible() {
Review Comment:
```suggestion
void
testOperatorUnionListStateRegistrationFailsIfNewSerializerIsIncompatible() {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1024,7 +1033,7 @@ public void testBroadcastStateKeyMigration() throws
Exception {
IntSerializer.INSTANCE));
}
- @Test
+ @TestTemplate
public void testBroadcastStateValueSerializerReconfiguration() throws
Exception {
Review Comment:
```suggestion
void testBroadcastStateValueSerializerReconfiguration() throws Exception
{
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -409,7 +413,7 @@ public void testKeyedMapStateSerializerReconfiguration()
throws Exception {
new
TestType.ReconfigurationRequiringTestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void
testKeyedMapStateRegistrationFailsIfNewStateSerializerIsIncompatible() {
Review Comment:
```suggestion
void
testKeyedMapStateRegistrationFailsIfNewStateSerializerIsIncompatible() {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -424,9 +428,10 @@ public void
testKeyedMapStateRegistrationFailsIfNewStateSerializerIsIncompatible
new TestType.IncompatibleTestTypeSerializer()));
fail("should have failed");
} catch (Exception expected) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1038,7 +1047,7 @@ public void
testBroadcastStateValueSerializerReconfiguration() throws Exception
new
TestType.ReconfigurationRequiringTestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void testBroadcastStateKeySerializerReconfiguration() throws
Exception {
Review Comment:
```suggestion
void testBroadcastStateKeySerializerReconfiguration() throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1052,7 +1061,7 @@ public void
testBroadcastStateKeySerializerReconfiguration() throws Exception {
IntSerializer.INSTANCE));
}
- @Test
+ @TestTemplate
public void
testBroadcastStateRegistrationFailsIfNewValueSerializerIsIncompatible() {
Review Comment:
```suggestion
void
testBroadcastStateRegistrationFailsIfNewValueSerializerIsIncompatible() {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -996,7 +1005,7 @@ private void testOperatorUnionListStateUpgrade(
// Operator state backend broadcast state tests
//
-------------------------------------------------------------------------------
- @Test
+ @TestTemplate
public void testBroadcastStateValueMigration() throws Exception {
Review Comment:
```suggestion
void testBroadcastStateValueMigration() throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1068,12 +1077,12 @@ public void
testBroadcastStateRegistrationFailsIfNewValueSerializerIsIncompatibl
fail("should have failed.");
} catch (Exception e) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent());
+ assertThat(ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent())
+ .isTrue();
}
}
- @Test
+ @TestTemplate
public void
testBroadcastStateRegistrationFailsIfNewKeySerializerIsIncompatible() {
Review Comment:
```suggestion
void
testBroadcastStateRegistrationFailsIfNewKeySerializerIsIncompatible() {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -935,8 +944,8 @@ public void
testOperatorUnionListStateRegistrationFailsIfNewSerializerIsIncompat
fail("should have failed.");
} catch (Exception e) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent());
+ assertThat(ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1010,7 +1019,7 @@ public void testBroadcastStateValueMigration() throws
Exception {
new TestType.V2TestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void testBroadcastStateKeyMigration() throws Exception {
Review Comment:
```suggestion
void testBroadcastStateKeyMigration() throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -583,7 +588,7 @@ public void
testPriorityQueueStateCreationFailsIfNewSerializerIsNotCompatible()
// Tests for key serializer in keyed state backends
//
-------------------------------------------------------------------------------
- @Test
+ @TestTemplate
public void
testStateBackendRestoreFailsIfNewKeySerializerRequiresMigration() throws
Exception {
Review Comment:
```suggestion
void testStateBackendRestoreFailsIfNewKeySerializerRequiresMigration()
throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1216,7 +1224,7 @@ public void testStateMigrationAfterChangingTTL() throws
Exception {
testKeyedValueStateUpgrade(initialAccessDescriptor,
newAccessDescriptorAfterRestore);
}
- @Test
+ @TestTemplate
public void testStateMigrationAfterChangingTTLFromEnablingToDisabling() {
Review Comment:
```suggestion
void testStateMigrationAfterChangingTTLFromEnablingToDisabling() {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -572,8 +577,8 @@ public void
testPriorityQueueStateCreationFailsIfNewSerializerIsNotCompatible()
fail("should have failed");
} catch (Exception e) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent());
+ assertThat(ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1068,12 +1077,12 @@ public void
testBroadcastStateRegistrationFailsIfNewValueSerializerIsIncompatibl
fail("should have failed.");
} catch (Exception e) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent());
+ assertThat(ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent())
+ .isTrue();
Review Comment:
i guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1231,13 +1239,14 @@ public void
testStateMigrationAfterChangingTTLFromEnablingToDisabling() {
testKeyedValueStateUpgrade(initialAccessDescriptor,
newAccessDescriptorAfterRestore);
fail("should have failed");
} catch (Exception expected) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -593,13 +598,14 @@ public void
testStateBackendRestoreFailsIfNewKeySerializerRequiresMigration() th
fail("should have failed");
} catch (Exception expected) {
// the new key serializer requires migration; this should fail the
restore
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1231,13 +1239,14 @@ public void
testStateMigrationAfterChangingTTLFromEnablingToDisabling() {
testKeyedValueStateUpgrade(initialAccessDescriptor,
newAccessDescriptorAfterRestore);
fail("should have failed");
} catch (Exception expected) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
}
}
- @Test
+ @TestTemplate
public void testStateMigrationAfterChangingTTLFromDisablingToEnabling() {
Review Comment:
```suggestion
void testStateMigrationAfterChangingTTLFromDisablingToEnabling() {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -619,9 +625,10 @@ public void
testStateBackendRestoreFailsIfNewKeySerializerIsIncompatible() throw
fail("should have failed");
} catch (Exception expected) {
// the new key serializer is incompatible; this should fail the
restore
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1089,8 +1098,8 @@ public void
testBroadcastStateRegistrationFailsIfNewKeySerializerIsIncompatible(
fail("should have failed.");
} catch (Exception e) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent());
+ assertThat(ExceptionUtils.findThrowable(e,
StateMigrationException.class).isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1195,12 +1204,11 @@ public void internalCopySerializerTest(
previousSerializer.snapshotConfiguration());
testProvider.registerNewSerializerForRestoredState(newSerializerForRestoredState);
- assertEquals(
- testProvider.currentSchemaSerializer().getClass(),
- internalCopySerializer.getClass());
+ assertThat(internalCopySerializer.getClass())
+ .isEqualTo(testProvider.currentSchemaSerializer().getClass());
}
- @Test
+ @TestTemplate
public void testStateMigrationAfterChangingTTL() throws Exception {
Review Comment:
```suggestion
void testStateMigrationAfterChangingTTL() throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -1253,9 +1262,10 @@ public void
testStateMigrationAfterChangingTTLFromDisablingToEnabling() {
testKeyedValueStateUpgrade(initialAccessDescriptor,
newAccessDescriptorAfterRestore);
fail("should have failed");
} catch (Exception expected) {
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
IllegalStateException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
IllegalStateException.class)
+ .isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -725,9 +733,10 @@ public void
testKeyedStateRegistrationFailsIfNewNamespaceSerializerIsIncompatibl
fail("should have failed");
} catch (Exception expected) {
// the new namespace serializer is incompatible; this should fail
the restore
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -812,7 +821,7 @@ public void testOperatorParitionableListStateMigration()
throws Exception {
new TestType.V2TestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void testOperatorParitionableListStateSerializerReconfiguration()
throws Exception {
Review Comment:
```suggestion
void testOperatorParitionableListStateSerializerReconfiguration() throws
Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -700,21 +707,22 @@ public void
testKeyedStateRegistrationFailsIfNewNamespaceSerializerRequiresMigra
fail("should have failed");
} catch (Exception expected) {
// the new namespace serializer requires migration; this should
fail the restore
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
Review Comment:
I guess it could be replaced with usage of `assertThatThrownBy`
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -897,7 +906,7 @@ private void testOperatorPartitionableListStateUpgrade(
// Operator state backend union list state tests
//
-------------------------------------------------------------------------------
- @Test
+ @TestTemplate
public void testOperatorUnionListStateMigration() throws Exception {
Review Comment:
```suggestion
void testOperatorUnionListStateMigration() throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -824,7 +833,7 @@ public void
testOperatorParitionableListStateSerializerReconfiguration() throws
new
TestType.ReconfigurationRequiringTestTypeSerializer()));
}
- @Test
+ @TestTemplate
public void
testOperatorParitionableListStateRegistrationFailsIfNewSerializerIsIncompatible()
Review Comment:
```suggestion
void
testOperatorParitionableListStateRegistrationFailsIfNewSerializerIsIncompatible()
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -608,7 +614,7 @@ public void
testStateBackendRestoreSucceedsIfNewKeySerializerRequiresReconfigura
new TestType.ReconfigurationRequiringTestTypeSerializer());
}
- @Test
+ @TestTemplate
public void testStateBackendRestoreFailsIfNewKeySerializerIsIncompatible()
throws Exception {
Review Comment:
```suggestion
void testStateBackendRestoreFailsIfNewKeySerializerIsIncompatible()
throws Exception {
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -700,21 +707,22 @@ public void
testKeyedStateRegistrationFailsIfNewNamespaceSerializerRequiresMigra
fail("should have failed");
} catch (Exception expected) {
// the new namespace serializer requires migration; this should
fail the restore
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
}
}
- @Test
+ @TestTemplate
public void
testKeyedStateRegistrationSucceedsIfNewNamespaceSerializerRequiresReconfiguration()
throws Exception {
testNamespaceSerializerUpgrade(
new TestType.V1TestTypeSerializer(),
new TestType.ReconfigurationRequiringTestTypeSerializer());
}
- @Test
+ @TestTemplate
public void
testKeyedStateRegistrationFailsIfNewNamespaceSerializerIsIncompatible()
Review Comment:
```suggestion
void
testKeyedStateRegistrationFailsIfNewNamespaceSerializerIsIncompatible()
```
could be package private
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendMigrationTestBase.java:
##########
@@ -700,21 +707,22 @@ public void
testKeyedStateRegistrationFailsIfNewNamespaceSerializerRequiresMigra
fail("should have failed");
} catch (Exception expected) {
// the new namespace serializer requires migration; this should
fail the restore
- Assert.assertTrue(
- ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
- .isPresent());
+ assertThat(
+ ExceptionUtils.findThrowable(expected,
StateMigrationException.class)
+ .isPresent())
+ .isTrue();
}
}
- @Test
+ @TestTemplate
public void
testKeyedStateRegistrationSucceedsIfNewNamespaceSerializerRequiresReconfiguration()
Review Comment:
```suggestion
void
testKeyedStateRegistrationSucceedsIfNewNamespaceSerializerRequiresReconfiguration()
```
could be package private
--
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]
