Jiabao-Sun commented on code in PR #23218:
URL: https://github.com/apache/flink/pull/23218#discussion_r1320770848
##########
flink-runtime/src/test/java/org/apache/flink/runtime/state/SnapshotDirectoryTest.java:
##########
@@ -19,180 +19,167 @@
package org.apache.flink.runtime.state;
import org.apache.flink.util.FileUtils;
-import org.apache.flink.util.TestLogger;
-import org.junit.AfterClass;
-import org.junit.Assert;
-import org.junit.BeforeClass;
-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 java.nio.file.Path;
import java.util.Arrays;
import java.util.UUID;
-/** Tests for the {@link SnapshotDirectory}. */
-public class SnapshotDirectoryTest extends TestLogger {
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
- private static TemporaryFolder temporaryFolder;
+/** Tests for the {@link SnapshotDirectory}. */
+class SnapshotDirectoryTest {
- @BeforeClass
- public static void beforeClass() throws IOException {
- temporaryFolder = new TemporaryFolder();
- temporaryFolder.create();
- }
-
- @AfterClass
- public static void afterClass() {
- temporaryFolder.delete();
- }
+ @TempDir private Path temporaryFolder;
/** Tests if mkdirs for snapshot directories works. */
@Test
- public void mkdirs() throws Exception {
- File folderRoot = temporaryFolder.getRoot();
+ void mkdirs() throws Exception {
+ File folderRoot = temporaryFolder.toFile();
File newFolder = new File(folderRoot,
String.valueOf(UUID.randomUUID()));
File innerNewFolder = new File(newFolder,
String.valueOf(UUID.randomUUID()));
Path path = innerNewFolder.toPath();
- Assert.assertFalse(newFolder.isDirectory());
- Assert.assertFalse(innerNewFolder.isDirectory());
+ assertThat(newFolder).doesNotExist();
+ assertThat(innerNewFolder).doesNotExist();
SnapshotDirectory snapshotDirectory =
SnapshotDirectory.permanent(path);
- Assert.assertFalse(snapshotDirectory.exists());
- Assert.assertFalse(newFolder.isDirectory());
- Assert.assertFalse(innerNewFolder.isDirectory());
-
- Assert.assertTrue(snapshotDirectory.mkdirs());
- Assert.assertTrue(newFolder.isDirectory());
- Assert.assertTrue(innerNewFolder.isDirectory());
- Assert.assertTrue(snapshotDirectory.exists());
+ assertThat(snapshotDirectory.exists()).isFalse();
+ assertThat(newFolder).doesNotExist();
+ assertThat(innerNewFolder).doesNotExist();
Review Comment:
I think the old assertions is not accurate.
We can add asserts `newFolder` and `innerNewFolder` is not file, the passes
will also succeed because `newFolder` and `innerNewFolder` dose not exists.
The original test can also be inferred from the name and file operation of
the method to test the creation folder.
```java
// old code
Assert.assertFalse(newFolder.isDirectory());
Assert.assertFalse(innerNewFolder.isDirectory());
// will succeed too
Assert.assertFalse(newFolder.isFile());
Assert.assertFalse(innerNewFolder.isFile());
// will fail because they are not exist.
assertThat(newFolder).isFile();
assertThat(innerNewFolder).isFile();
```
--
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
