You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "spuru9 (via GitHub)" <gi...@apache.org> on 2026/05/26 14:59:13 UTC
[PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
spuru9 opened a new pull request, #28261:
URL: https://github.com/apache/flink/pull/28261
## What is the purpose of the change
Migrate all JUnit 4 tests in `flink-state-backends/flink-statebackend-forst` to JUnit 5 (Jupiter) as part of the ongoing migration tracked under [FLINK-25325](https://issues.apache.org/jira/browse/FLINK-25325). This eliminates the last JUnit 4 imports from the ForSt module.
JIRA: [FLINK-39767](https://issues.apache.org/jira/browse/FLINK-39767)
## Brief change log
9 test files migrated:
- `ForStIncrementalCheckpointRescalingTest` — parameterized: `@RunWith(Parameterized.class)` → `@ExtendWith(ParameterizedTestExtension.class)` with `@TestTemplate`
- `ForStInitITCase`
- `ForStMemoryControllerUtilsTest`
- `ForStMultiClassLoaderTest`
- `ForStNativeMetricOptionsTest`
- `ForStOperationsUtilsTest`
- `ForStResourceContainerTest`
- `ForStStateBackendConfigTest`
- `ForStStateBackendFactoryTest`
Per-file changes:
- `org.junit.*` imports → `org.junit.jupiter.api.*`
- `@Before` / `@After` / `@BeforeClass` / `@AfterClass` → `@BeforeEach` / `@AfterEach` / `@BeforeAll` / `@AfterAll`
- `@Rule TemporaryFolder` / `@ClassRule TemporaryFolder` → `@TempDir java.nio.file.Path` + `TempDirUtils.newFolder(...)` / `TempDirUtils.newFile(...)`
- `@Test(expected = X.class)` → `assertThatThrownBy(...).isInstanceOf(X.class)`
- Hamcrest + `org.junit.Assert.*` → AssertJ (project convention)
- Drop `extends TestLogger` and `public` modifiers per the Flink JUnit 5 Migration Guide
No production code is touched.
## Verifying this change
This change is a test refactor and is covered by the migrated tests themselves:
```
./mvnw -pl flink-state-backends/flink-statebackend-forst test
```
Result: **976 tests run, 0 failures, 0 errors, 5 skipped**. Total test count and pass/skip distribution match the pre-migration baseline.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
- The S3 file system connector: (no)
## Documentation
- Does this pull request introduce a new feature? (no)
- If yes, how is the feature documented? (not applicable)
---
##### Was generative AI tooling used to co-author this PR?
- [x] Yes — Claude Code
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on PR #28261:
URL: https://github.com/apache/flink/pull/28261#issuecomment-4611486985
@flinkbot run azure
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3332192868
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
@@ -7,7 +7,7 @@
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
- * http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
Review Comment:
why do we need it?
Did not spotless catch it?
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3333157000
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
@@ -149,7 +151,7 @@ public void testScalingUp() throws Exception {
harness.setStateBackend(getStateBackend());
harness.setCheckpointStorage(
new FileSystemCheckpointStorage(
- "file://" + rootFolder.newFolder().getAbsolutePath()));
+ "file://" + TempDirUtils.newFolder(rootFolder).getAbsolutePath()));
Review Comment:
Have used `@TempDir` now in the places only one directory is needed. But the Utils in the case of multiple.
I was using TempDirUtils.newFolder(rootFolder).getAbsolutePath())) as it is being recommended in https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit?tab=t.0#heading=h.we2n5aifcjfq
> We provide a util class TempDirUtils for implementing common functions in TemporaryFolder, like newFolder() and newFile().
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on PR #28261:
URL: https://github.com/apache/flink/pull/28261#issuecomment-4613243405
> if it is unrelated you can use bot commands to restart the build to make it green
>
> ```
> Bot commands
> The @flinkbot bot supports the following commands:
>
> @flinkbot run azure re-run the last Azure build
> ```
>
> as mentioned above
The bot command seems to be broken, so have made a empty commit. The CI has passed.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "raminqaf (via GitHub)" <gi...@apache.org>.
raminqaf commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3350320692
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
@@ -35,109 +35,110 @@
import org.apache.flink.streaming.runtime.streamrecord.StreamRecord;
import org.apache.flink.streaming.util.AbstractStreamOperatorTestHarness;
import org.apache.flink.streaming.util.KeyedOneInputStreamOperatorTestHarness;
+import org.apache.flink.testutils.junit.extensions.parameterized.Parameter;
+import org.apache.flink.testutils.junit.extensions.parameterized.ParameterizedTestExtension;
+import org.apache.flink.testutils.junit.extensions.parameterized.Parameters;
import org.apache.flink.util.Collector;
-import org.apache.flink.util.TestLogger;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import static org.apache.flink.state.forst.ForStConfigurableOptions.USE_INGEST_DB_RESTORE_MODE;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests to guard rescaling from checkpoint. */
-@RunWith(Parameterized.class)
-public class ForStIncrementalCheckpointRescalingTest extends TestLogger {
+@ExtendWith(ParameterizedTestExtension.class)
+class ForStIncrementalCheckpointRescalingTest {
- @Rule public TemporaryFolder rootFolder = new TemporaryFolder();
+ @TempDir Path rootFolder;
- @Parameterized.Parameters(name = "useIngestDbRestoreMode: {0}")
+ @Parameters(name = "useIngestDbRestoreMode: {0}")
public static Collection<Boolean> parameters() {
return Arrays.asList(false, true);
}
Review Comment:
How about
```suggestion
@Parameters(name = "useIngestDbRestoreMode: {0}")
public static List<Boolean> parameters() {
return List.of(false, true);
}
```
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStInitITCase.java:
##########
@@ -21,33 +21,29 @@
import org.apache.flink.runtime.operators.testutils.ExpectedTestException;
import org.apache.flink.util.concurrent.Executors;
-import org.junit.Assert;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
import java.io.File;
import java.io.IOException;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
/** Tests for {@link ForStStateBackend} on initialization. */
-public class ForStInitITCase {
-
- @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+class ForStInitITCase {
/**
* This test checks that the ForSt native code loader still responds to resetting the init flag.
*/
@Test
- public void testResetInitFlag() throws Exception {
+ void testResetInitFlag() throws Exception {
ForStStateBackend.resetForStLoadedFlag();
}
@Test
- public void testTempLibFolderDeletedOnFail() throws Exception {
+ void testTempLibFolderDeletedOnFail(@TempDir File tempFolder) throws Exception {
Review Comment:
Exception can be dropped
```suggestion
void testTempLibFolderDeletedOnFail(@TempDir File tempFolder) {
```
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
Review Comment:
Since you are at it, it seems that the exception here is not needed anymore
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3332195611
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
@@ -35,109 +35,111 @@
import org.apache.flink.streaming.runtime.streamrecord.StreamRecord;
import org.apache.flink.streaming.util.AbstractStreamOperatorTestHarness;
import org.apache.flink.streaming.util.KeyedOneInputStreamOperatorTestHarness;
+import org.apache.flink.testutils.junit.extensions.parameterized.Parameter;
+import org.apache.flink.testutils.junit.extensions.parameterized.ParameterizedTestExtension;
+import org.apache.flink.testutils.junit.extensions.parameterized.Parameters;
+import org.apache.flink.testutils.junit.utils.TempDirUtils;
import org.apache.flink.util.Collector;
-import org.apache.flink.util.TestLogger;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import static org.apache.flink.state.forst.ForStConfigurableOptions.USE_INGEST_DB_RESTORE_MODE;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests to guard rescaling from checkpoint. */
-@RunWith(Parameterized.class)
-public class ForStIncrementalCheckpointRescalingTest extends TestLogger {
+@ExtendWith(ParameterizedTestExtension.class)
+class ForStIncrementalCheckpointRescalingTest {
- @Rule public TemporaryFolder rootFolder = new TemporaryFolder();
+ @TempDir Path rootFolder;
- @Parameterized.Parameters(name = "useIngestDbRestoreMode: {0}")
+ @Parameters(name = "useIngestDbRestoreMode: {0}")
public static Collection<Boolean> parameters() {
return Arrays.asList(false, true);
}
- @Parameterized.Parameter public boolean useIngestDbRestoreMode;
+ @Parameter public boolean useIngestDbRestoreMode;
private final int maxParallelism = 10;
private final KeySelector<String, String> keySelector = new TestKeySelector();
private String[] records;
- @Before
- public void initRecords() throws Exception {
+ @BeforeEach
+ void initRecords() throws Exception {
records = new String[10];
records[0] = "8";
- Assert.assertEquals(
- 0,
- KeyGroupRangeAssignment.assignToKeyGroup(
- keySelector.getKey(records[0]), maxParallelism)); // group 0
+ assertThat(
+ KeyGroupRangeAssignment.assignToKeyGroup(
+ keySelector.getKey(records[0]), maxParallelism))
+ .isEqualTo(0); // group 0
records[1] = "5";
- Assert.assertEquals(
- 1,
- KeyGroupRangeAssignment.assignToKeyGroup(
- keySelector.getKey(records[1]), maxParallelism)); // group 1
+ assertThat(
+ KeyGroupRangeAssignment.assignToKeyGroup(
+ keySelector.getKey(records[1]), maxParallelism))
+ .isEqualTo(1); // group 1
Review Comment:
```suggestion
.isOne(); // group 1
```
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on PR #28261:
URL: https://github.com/apache/flink/pull/28261#issuecomment-4614969060
@raminqaf Thanks for the review. Have addressed the comments.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on PR #28261:
URL: https://github.com/apache/flink/pull/28261#issuecomment-4728059272
cc @raminqaf @snuyanzin Can you take a relook. have address the comments and CI is green now.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3333157000
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
@@ -149,7 +151,7 @@ public void testScalingUp() throws Exception {
harness.setStateBackend(getStateBackend());
harness.setCheckpointStorage(
new FileSystemCheckpointStorage(
- "file://" + rootFolder.newFolder().getAbsolutePath()));
+ "file://" + TempDirUtils.newFolder(rootFolder).getAbsolutePath()));
Review Comment:
Have used `@TempDir` now in the places only one directory is needed. But the Utils in the case of multiple, I believe its a cleaner approach to not use multiple `@TempDir`, also TempDirUtils is created for the same purpose.
I was using TempDirUtils.newFolder(rootFolder).getAbsolutePath())) as it is being recommended in https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit?tab=t.0#heading=h.we2n5aifcjfq
> We provide a util class TempDirUtils for implementing common functions in TemporaryFolder, like newFolder() and newFile().
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3332197698
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
@@ -149,7 +151,7 @@ public void testScalingUp() throws Exception {
harness.setStateBackend(getStateBackend());
harness.setCheckpointStorage(
new FileSystemCheckpointStorage(
- "file://" + rootFolder.newFolder().getAbsolutePath()));
+ "file://" + TempDirUtils.newFolder(rootFolder).getAbsolutePath()));
Review Comment:
why not `@TempDir`?
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #28261:
URL: https://github.com/apache/flink/pull/28261#issuecomment-4611469628
if it is unrelated you can use bot commands to restart the build to make it green
```
Bot commands
The @flinkbot bot supports the following commands:
@flinkbot run azure re-run the last Azure build
```
as mentioned above
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on PR #28261:
URL: https://github.com/apache/flink/pull/28261#issuecomment-4575722877
@snuyanzin Can you review this as well.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3332199799
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
@@ -181,24 +183,24 @@ public void testScalingUp() throws Exception {
// task's key-group [0, 4]
KeyGroupRange localKeyGroupRange20 = keyGroupPartitions.get(0);
- Assert.assertEquals(new KeyGroupRange(0, 4), localKeyGroupRange20);
+ assertThat(localKeyGroupRange20).isEqualTo(new KeyGroupRange(0, 4));
harness2[0] = getHarnessTest(keySelector, maxParallelism, 2, 0);
harness2[0].setStateBackend(getStateBackend());
harness2[0].setCheckpointStorage(
new FileSystemCheckpointStorage(
- "file://" + rootFolder.newFolder().getAbsolutePath()));
+ "file://" + TempDirUtils.newFolder(rootFolder).getAbsolutePath()));
harness2[0].setup();
harness2[0].initializeState(initState1);
harness2[0].open();
// task's key-group [5, 9]
KeyGroupRange localKeyGroupRange21 = keyGroupPartitions.get(1);
- Assert.assertEquals(new KeyGroupRange(5, 9), localKeyGroupRange21);
+ assertThat(localKeyGroupRange21).isEqualTo(new KeyGroupRange(5, 9));
harness2[1] = getHarnessTest(keySelector, maxParallelism, 2, 1);
harness2[1].setStateBackend(getStateBackend());
harness2[1].setCheckpointStorage(
new FileSystemCheckpointStorage(
- "file://" + rootFolder.newFolder().getAbsolutePath()));
+ "file://" + TempDirUtils.newFolder(rootFolder).getAbsolutePath()));
Review Comment:
`@TempDir`?
here and in other places
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3333114709
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStResourceContainerTest.java:
##########
@@ -298,20 +296,22 @@ public ColumnFamilyOptions createColumnOptions(
ColumnFamilyOptions columnOptions = container.getColumnOptions();
BlockBasedTableConfig actual =
(BlockBasedTableConfig) columnOptions.tableFormatConfig();
- assertThat(actual.indexType(), is(IndexType.kTwoLevelIndexSearch));
- assertThat(actual.partitionFilters(), is(true));
- assertThat(actual.pinTopLevelIndexAndFilter(), is(true));
- assertFalse(actual.filterPolicy() == blockBasedFilter);
+ assertThat(actual.indexType()).isEqualTo(IndexType.kTwoLevelIndexSearch);
+ assertThat(actual.partitionFilters()).isTrue();
+ assertThat(actual.pinTopLevelIndexAndFilter()).isTrue();
+ assertThat(actual.filterPolicy()).isNotSameAs(blockBasedFilter);
}
- assertFalse("Block based filter is left unclosed.", blockBasedFilter.isOwningHandle());
+ assertThat(blockBasedFilter.isOwningHandle())
+ .as("Block based filter is left unclosed.")
+ .isFalse();
}
@Test
- public void testDirectoryResources() throws Exception {
- Path localJobPath = new Path(TMP_FOLDER.newFolder().getPath());
+ void testDirectoryResources() throws Exception {
+ Path localJobPath = new Path(TempDirUtils.newFolder(tmpFolder).getPath());
Review Comment:
Multiple distinct temp dirs are needed here, so TempDirUtils.newFolder(tmpFolder) is simpler than injecting several `@TempDir` params.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #28261:
URL: https://github.com/apache/flink/pull/28261#issuecomment-4545476737
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "346f7d395b004ced2879edfeb14c5e69b51c0507",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "346f7d395b004ced2879edfeb14c5e69b51c0507",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 346f7d395b004ced2879edfeb14c5e69b51c0507 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3332199000
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
@@ -181,24 +183,24 @@ public void testScalingUp() throws Exception {
// task's key-group [0, 4]
KeyGroupRange localKeyGroupRange20 = keyGroupPartitions.get(0);
- Assert.assertEquals(new KeyGroupRange(0, 4), localKeyGroupRange20);
+ assertThat(localKeyGroupRange20).isEqualTo(new KeyGroupRange(0, 4));
harness2[0] = getHarnessTest(keySelector, maxParallelism, 2, 0);
harness2[0].setStateBackend(getStateBackend());
harness2[0].setCheckpointStorage(
new FileSystemCheckpointStorage(
- "file://" + rootFolder.newFolder().getAbsolutePath()));
+ "file://" + TempDirUtils.newFolder(rootFolder).getAbsolutePath()));
Review Comment:
`@TempDir`?
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on PR #28261:
URL: https://github.com/apache/flink/pull/28261#issuecomment-4611399365
@snuyanzin PTAL again, and merge if looks fine
(The CI failure in in python package, and unrelated)
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin merged PR #28261:
URL: https://github.com/apache/flink/pull/28261
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3332194627
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStIncrementalCheckpointRescalingTest.java:
##########
@@ -35,109 +35,111 @@
import org.apache.flink.streaming.runtime.streamrecord.StreamRecord;
import org.apache.flink.streaming.util.AbstractStreamOperatorTestHarness;
import org.apache.flink.streaming.util.KeyedOneInputStreamOperatorTestHarness;
+import org.apache.flink.testutils.junit.extensions.parameterized.Parameter;
+import org.apache.flink.testutils.junit.extensions.parameterized.ParameterizedTestExtension;
+import org.apache.flink.testutils.junit.extensions.parameterized.Parameters;
+import org.apache.flink.testutils.junit.utils.TempDirUtils;
import org.apache.flink.util.Collector;
-import org.apache.flink.util.TestLogger;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import static org.apache.flink.state.forst.ForStConfigurableOptions.USE_INGEST_DB_RESTORE_MODE;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests to guard rescaling from checkpoint. */
-@RunWith(Parameterized.class)
-public class ForStIncrementalCheckpointRescalingTest extends TestLogger {
+@ExtendWith(ParameterizedTestExtension.class)
+class ForStIncrementalCheckpointRescalingTest {
- @Rule public TemporaryFolder rootFolder = new TemporaryFolder();
+ @TempDir Path rootFolder;
- @Parameterized.Parameters(name = "useIngestDbRestoreMode: {0}")
+ @Parameters(name = "useIngestDbRestoreMode: {0}")
public static Collection<Boolean> parameters() {
return Arrays.asList(false, true);
}
- @Parameterized.Parameter public boolean useIngestDbRestoreMode;
+ @Parameter public boolean useIngestDbRestoreMode;
private final int maxParallelism = 10;
private final KeySelector<String, String> keySelector = new TestKeySelector();
private String[] records;
- @Before
- public void initRecords() throws Exception {
+ @BeforeEach
+ void initRecords() throws Exception {
records = new String[10];
records[0] = "8";
- Assert.assertEquals(
- 0,
- KeyGroupRangeAssignment.assignToKeyGroup(
- keySelector.getKey(records[0]), maxParallelism)); // group 0
+ assertThat(
+ KeyGroupRangeAssignment.assignToKeyGroup(
+ keySelector.getKey(records[0]), maxParallelism))
+ .isEqualTo(0); // group 0
Review Comment:
```suggestion
.isZero(); // group 0
```
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3332211130
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStStateBackendFactoryTest.java:
##########
@@ -125,15 +122,9 @@ public void testLoadForStStateBackendMixed() throws Exception {
// ------------------------------------------------------------------------
private static void checkPaths(String[] pathsArray, String... paths) {
- assertNotNull(pathsArray);
- assertNotNull(paths);
-
- assertEquals(pathsArray.length, paths.length);
-
- HashSet<String> pathsSet = new HashSet<>(Arrays.asList(pathsArray));
-
- for (String path : paths) {
- assertTrue(pathsSet.contains(path));
- }
+ assertThat(pathsArray).isNotNull();
+ assertThat(paths).isNotNull();
+ assertThat(pathsArray).hasSize(paths.length);
+ assertThat(pathsArray).contains(paths);
Review Comment:
how about `containsExactlyInAnyOrder`?
then no need to check for size
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-39767][tests][JUnit5 migration] Module: flink-statebackend-forst [flink]
Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on code in PR #28261:
URL: https://github.com/apache/flink/pull/28261#discussion_r3332825913
##########
flink-state-backends/flink-statebackend-forst/src/test/java/org/apache/flink/state/forst/ForStStateBackendFactoryTest.java:
##########
@@ -97,11 +94,11 @@ public void testLoadForStStateBackend() throws Exception {
* parameters over configuration-defined parameters.
*/
@Test
- public void testLoadForStStateBackendMixed() throws Exception {
- final String localDir1 = tmp.newFolder().getAbsolutePath();
- final String localDir2 = tmp.newFolder().getAbsolutePath();
- final String localDir3 = tmp.newFolder().getAbsolutePath();
- final String localDir4 = tmp.newFolder().getAbsolutePath();
+ void testLoadForStStateBackendMixed() throws Exception {
+ final String localDir1 = TempDirUtils.newFolder(tmp).getAbsolutePath();
Review Comment:
Kept TempDirUtils.newFolder(tmp) since the paths are used as strings and one test needs four — tidier than multiple @TempDir params; distinctness is needed either way.
--
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-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org