You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/05/13 13:29:04 UTC
[GitHub] [flink] snuyanzin opened a new pull request, #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
snuyanzin opened a new pull request, #19716:
URL: https://github.com/apache/flink/pull/19716
## What is the purpose of the change
Update the flink-connector-files module to AssertJ and JUnit 5 following the [JUnit 5 Migration Guide](https://docs.google.com/document/d/1514Wa_aNB9bJUen4xm5uiuXOooOJTtXqS_Jqk9KJitU/edit)
## Brief change log
Use JUnit5 and AssertJ in tests instead of JUnit4 and Hamcrest
## Verifying this change
This change is a code cleanup without any test coverage.
## 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)
--
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
[GitHub] [flink] snuyanzin commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r874674793
##########
flink-core/src/test/java/org/apache/flink/api/common/typeutils/TypeInformationTestBase.java:
##########
@@ -21,24 +21,23 @@
import org.apache.flink.api.common.ExecutionConfig;
import org.apache.flink.api.common.typeinfo.TypeInformation;
import org.apache.flink.util.InstantiationUtil;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.util.TestLoggerExtension;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
import java.io.IOException;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Abstract test base for type information. */
-public abstract class TypeInformationTestBase<T extends TypeInformation<?>> extends TestLogger {
+@ExtendWith(TestLoggerExtension.class)
+public abstract class TypeInformationTestBase<T extends TypeInformation<?>> {
Review Comment:
yes, sure, I've rebased 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
[GitHub] [flink] snuyanzin commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r875296706
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/sink/FileSinkCompactionSwitchITCase.java:
##########
@@ -108,55 +104,56 @@ public class FileSinkCompactionSwitchITCase extends TestLogger {
protected static final int NUM_BUCKETS = 4;
- @ClassRule public static final TemporaryFolder TEMPORARY_FOLDER = new TemporaryFolder();
+ @TempDir private static java.nio.file.Path tmpDir;
- @Rule public final SharedObjects sharedObjects = SharedObjects.create();
+ @RegisterExtension final SharedObjectsExtension sharedObjects = SharedObjectsExtension.create();
Review Comment:
done
--
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
[GitHub] [flink] zentol commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r874481750
##########
flink-core/src/test/java/org/apache/flink/api/common/typeutils/TypeInformationTestBase.java:
##########
@@ -21,24 +21,23 @@
import org.apache.flink.api.common.ExecutionConfig;
import org.apache.flink.api.common.typeinfo.TypeInformation;
import org.apache.flink.util.InstantiationUtil;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.util.TestLoggerExtension;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
import java.io.IOException;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Abstract test base for type information. */
-public abstract class TypeInformationTestBase<T extends TypeInformation<?>> extends TestLogger {
+@ExtendWith(TestLoggerExtension.class)
+public abstract class TypeInformationTestBase<T extends TypeInformation<?>> {
Review Comment:
#19744 looks good to me; can you rebase this PR on top of the other PR? Then I can get started reviewing this one.
--
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
[GitHub] [flink] snuyanzin commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r875293274
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/sink/writer/FileSinkMigrationITCase.java:
##########
@@ -58,17 +56,15 @@
import java.util.stream.LongStream;
import static org.apache.flink.runtime.testutils.CommonTestUtils.waitForAllTaskRunning;
-import static org.junit.Assert.assertEquals;
+import static org.assertj.core.api.Assertions.assertThat;
/**
* Tests migrating from {@link StreamingFileSink} to {@link FileSink}. It trigger a savepoint for
* the {@link StreamingFileSink} job and restore the {@link FileSink} job from the savepoint taken.
*/
-public class FileSinkMigrationITCase extends TestLogger {
+class FileSinkMigrationITCase {
- @ClassRule public static final TemporaryFolder TEMPORARY_FOLDER = new TemporaryFolder();
-
- @Rule public final SharedObjects sharedObjects = SharedObjects.create();
+ @RegisterExtension final SharedObjectsExtension sharedObjects = SharedObjectsExtension.create();
Review Comment:
done
--
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
[GitHub] [flink] zentol commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r873685730
##########
flink-core/src/test/java/org/apache/flink/api/common/typeutils/TypeInformationTestBase.java:
##########
@@ -21,24 +21,23 @@
import org.apache.flink.api.common.ExecutionConfig;
import org.apache.flink.api.common.typeinfo.TypeInformation;
import org.apache.flink.util.InstantiationUtil;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.util.TestLoggerExtension;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
import java.io.IOException;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Abstract test base for type information. */
-public abstract class TypeInformationTestBase<T extends TypeInformation<?>> extends TestLogger {
+@ExtendWith(TestLoggerExtension.class)
+public abstract class TypeInformationTestBase<T extends TypeInformation<?>> {
Review Comment:
This must be a separate commit, and ideally even a separate ticket.
--
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
[GitHub] [flink] snuyanzin commented on pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on PR #19716:
URL: https://github.com/apache/flink/pull/19716#issuecomment-1126274461
@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
[GitHub] [flink] snuyanzin commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r874476281
##########
flink-core/src/test/java/org/apache/flink/api/common/typeutils/TypeInformationTestBase.java:
##########
@@ -21,24 +21,23 @@
import org.apache.flink.api.common.ExecutionConfig;
import org.apache.flink.api.common.typeinfo.TypeInformation;
import org.apache.flink.util.InstantiationUtil;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.util.TestLoggerExtension;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
import java.io.IOException;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Abstract test base for type information. */
-public abstract class TypeInformationTestBase<T extends TypeInformation<?>> extends TestLogger {
+@ExtendWith(TestLoggerExtension.class)
+public abstract class TypeInformationTestBase<T extends TypeInformation<?>> {
Review Comment:
Yes, probably you're right
I've created a separate ticket for that https://issues.apache.org/jira/browse/FLINK-27662
and a corresponding PR https://github.com/apache/flink/pull/19744
--
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
[GitHub] [flink] zentol commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r874783096
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/stream/StreamingFileWriterTest.java:
##########
@@ -161,12 +160,12 @@ public void testCommitImmediately() throws Exception {
harness.notifyOfCompletedCheckpoint(1);
List<String> partitions = collect(harness);
- Assert.assertEquals(Arrays.asList("1", "2"), partitions);
+ assertThat(partitions).isEqualTo(Arrays.asList("1", "2"));
Review Comment:
```suggestion
assertThat(partitions).containsExactly("1", "2");
```
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/stream/compact/CompactCoordinatorTest.java:
##########
@@ -127,21 +128,20 @@ private void runCoordinator(
}
private void assertEndCompaction(CoordinatorOutput output, long checkpointId) {
- Assert.assertTrue(output instanceof EndCompaction);
+ assertThat(output).isInstanceOf(EndCompaction.class);
EndCompaction end = (EndCompaction) output;
- Assert.assertEquals(checkpointId, end.getCheckpointId());
+ assertThat(end.getCheckpointId()).isEqualTo(checkpointId);
}
private void assertUnit(
CoordinatorOutput output, int unitId, String partition, List<String> fileNames) {
- Assert.assertTrue(output instanceof CompactionUnit);
+ assertThat(output).isInstanceOf(CompactionUnit.class);
CompactionUnit unit = (CompactionUnit) output;
- Assert.assertEquals(unitId, unit.getUnitId());
- Assert.assertEquals(partition, unit.getPartition());
- Assert.assertEquals(
- fileNames,
- unit.getPaths().stream().map(Path::getName).collect(Collectors.toList()));
+ assertThat(unit.getUnitId()).isEqualTo(unitId);
+ assertThat(unit.getPartition()).isEqualTo(partition);
+ assertThat(unit.getPaths().stream().map(Path::getName).collect(Collectors.toList()))
+ .isEqualTo(fileNames);
Review Comment:
```suggestion
assertThat(unit.getPaths().stream().map(Path::getName)).containsExactlyElementsOf(fileNames);
```
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/stream/StreamingFileWriterTest.java:
##########
@@ -105,7 +104,7 @@ public void testFailover() throws Exception {
state = harness.snapshot(2, 2);
harness.notifyOfCompletedCheckpoint(2);
List<String> partitions = collect(harness);
- Assert.assertEquals(Arrays.asList("1", "2", "3", "4"), partitions);
+ assertThat(partitions).isEqualTo(Arrays.asList("1", "2", "3", "4"));
Review Comment:
see below
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/stream/StreamingFileWriterTest.java:
##########
@@ -118,7 +117,7 @@ public void testFailover() throws Exception {
state = harness.snapshot(3, 3);
harness.notifyOfCompletedCheckpoint(3);
List<String> partitions = collect(harness);
- Assert.assertEquals(Arrays.asList("3", "4", "5"), partitions);
+ assertThat(partitions).isEqualTo(Arrays.asList("3", "4", "5"));
Review Comment:
see below
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/stream/StreamingFileWriterTest.java:
##########
@@ -136,12 +135,12 @@ public void testFailover() throws Exception {
harness.notifyOfCompletedCheckpoint(5);
List<String> partitions = collect(harness);
// should not contains partition {9}
- Assert.assertEquals(Arrays.asList("4", "5", "6", "7", "8"), partitions);
+ assertThat(partitions).isEqualTo(Arrays.asList("4", "5", "6", "7", "8"));
Review Comment:
see below
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/stream/StreamingFileWriterTest.java:
##########
@@ -60,25 +58,26 @@
import static org.apache.flink.connector.file.table.FileSystemConnectorOptions.SINK_PARTITION_COMMIT_POLICY_KIND;
import static org.apache.flink.connector.file.table.FileSystemConnectorOptions.SINK_PARTITION_COMMIT_TRIGGER;
import static org.apache.flink.connector.file.table.FileSystemConnectorOptions.SINK_PARTITION_COMMIT_WATERMARK_TIME_ZONE;
+import static org.assertj.core.api.Assertions.assertThat;
/** Test for {@link StreamingFileWriter}. */
-public class StreamingFileWriterTest {
+class StreamingFileWriterTest {
- @ClassRule public static final TemporaryFolder TEMPORARY_FOLDER = new TemporaryFolder();
private final OutputFileConfig outputFileConfig = OutputFileConfig.builder().build();
private final DateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd");
+ @TempDir private java.nio.file.Path tmpDir;
private Path path;
- @Before
- public void before() throws IOException {
- File file = TEMPORARY_FOLDER.newFile();
+ @BeforeEach
+ void before() throws IOException {
+ File file = tmpDir.toFile();
file.delete();
path = new Path(file.toURI());
Review Comment:
```suggestion
path = new Path(tmpDir.resolve("tmp").toUri());
```
his part is a bit strange.
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/PartitionWriterTest.java:
##########
@@ -23,26 +23,39 @@
import org.apache.flink.connector.file.table.PartitionWriter.Context;
import org.apache.flink.core.fs.FileSystem;
import org.apache.flink.core.fs.Path;
-import org.apache.flink.table.utils.LegacyRowResource;
import org.apache.flink.types.Row;
+import org.apache.flink.types.RowUtils;
-import org.junit.Assert;
-import org.junit.ClassRule;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import static org.assertj.core.api.Assertions.assertThat;
+
/** Test for {@link PartitionWriter}s. */
-public class PartitionWriterTest {
+class PartitionWriterTest {
+
+ @TempDir private java.nio.file.Path tmpDir;
- @Rule public final LegacyRowResource usesLegacyRows = LegacyRowResource.INSTANCE;
Review Comment:
you could keep this field and manually call the before/after methods. That's preferable imo to duplicating the contained logic (and if we ever create an extension it also points us to this test).
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/FileSystemCommitterTest.java:
##########
@@ -21,99 +21,97 @@
import org.apache.flink.core.fs.FileSystem;
import org.apache.flink.core.fs.Path;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.ClassRule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.BeforeEach;
+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.Files;
+import java.nio.file.Paths;
import java.util.LinkedHashMap;
import java.util.Optional;
-/** Test for {@link FileSystemCommitter}. */
-public class FileSystemCommitterTest {
-
- @ClassRule public static final TemporaryFolder TEMP_FOLDER = new TemporaryFolder();
-
- private File tmpFile;
- private File outputFile;
+import static org.assertj.core.api.Assertions.assertThat;
- private Path tmpPath;
+/** Test for {@link FileSystemCommitter}. */
+class FileSystemCommitterTest {
private FileSystemFactory fileSystemFactory = FileSystem::get;
private TableMetaStoreFactory metaStoreFactory;
+ @TempDir private java.nio.file.Path outputPath;
+ @TempDir private java.nio.file.Path path;
- @Before
+ @BeforeEach
public void before() throws IOException {
- tmpFile = TEMP_FOLDER.newFolder();
- outputFile = TEMP_FOLDER.newFolder();
-
- tmpPath = new Path(tmpFile.getPath());
- Path outputPath = new Path(outputFile.getPath());
- metaStoreFactory = new TestMetaStoreFactory(outputPath);
+ metaStoreFactory = new TestMetaStoreFactory(new Path(outputPath.toString()));
}
@SuppressWarnings("ResultOfMethodCallIgnored")
- private void createFile(String path, String... files) throws IOException {
- File p1 = new File(tmpFile, path);
- p1.mkdirs();
+ private void createFile(java.nio.file.Path parent, String path, String... files)
+ throws IOException {
+ java.nio.file.Path dir = Files.createDirectories(Paths.get(parent.toString(), path));
for (String file : files) {
- new File(p1, file).createNewFile();
+ Files.createFile(Paths.get(dir.toString(), file));
Review Comment:
```suggestion
Files.createFile(dir.resolve(file));
```
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/stream/StreamingFileWriterTest.java:
##########
@@ -92,7 +91,7 @@ public void testFailover() throws Exception {
harness.processElement(row("4"), 0);
harness.notifyOfCompletedCheckpoint(1);
List<String> partitions = collect(harness);
- Assert.assertEquals(Arrays.asList("1", "2"), partitions);
+ assertThat(partitions).isEqualTo(Arrays.asList("1", "2"));
Review Comment:
see below
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/FileSystemOutputFormatTest.java:
##########
@@ -86,20 +86,19 @@ public void testClosingWithoutInput() throws Exception {
}
@Test
- public void testNonPartition() throws Exception {
+ void testNonPartition() throws Exception {
AtomicReference<FileSystemOutputFormat<Row>> ref = new AtomicReference<>();
try (OneInputStreamOperatorTestHarness<Row, Object> testHarness =
createSink(false, false, false, new LinkedHashMap<>(), ref)) {
writeUnorderedRecords(testHarness);
- assertEquals(1, getFileContentByPath(tmpFile).size());
+ assertThat(getFileContentByPath(tmpPath)).hasSize(1);
}
ref.get().finalizeGlobal(1);
- Map<File, String> content = getFileContentByPath(outputFile);
- assertEquals(1, content.size());
- assertEquals(
- "a1,1,p1\n" + "a2,2,p1\n" + "a2,2,p2\n" + "a3,3,p1\n",
- content.values().iterator().next());
+ Map<File, String> content = getFileContentByPath(outputPath);
+ assertThat(content).hasSize(1);
+ assertThat(content.values().iterator().next())
+ .isEqualTo("a1,1,p1\n" + "a2,2,p1\n" + "a2,2,p2\n" + "a3,3,p1\n");
Review Comment:
```suggestion
assertThat(content.values()).containsExactly("a1,1,p1\n" + "a2,2,p1\n" + "a2,2,p2\n" + "a3,3,p1\n");
```
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/sink/writer/FileSinkMigrationITCase.java:
##########
@@ -58,17 +56,15 @@
import java.util.stream.LongStream;
import static org.apache.flink.runtime.testutils.CommonTestUtils.waitForAllTaskRunning;
-import static org.junit.Assert.assertEquals;
+import static org.assertj.core.api.Assertions.assertThat;
/**
* Tests migrating from {@link StreamingFileSink} to {@link FileSink}. It trigger a savepoint for
* the {@link StreamingFileSink} job and restore the {@link FileSink} job from the savepoint taken.
*/
-public class FileSinkMigrationITCase extends TestLogger {
+class FileSinkMigrationITCase {
- @ClassRule public static final TemporaryFolder TEMPORARY_FOLDER = new TemporaryFolder();
-
- @Rule public final SharedObjects sharedObjects = SharedObjects.create();
+ @RegisterExtension final SharedObjectsExtension sharedObjects = SharedObjectsExtension.create();
Review Comment:
private
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/sink/FileSinkCompactionSwitchITCase.java:
##########
@@ -108,55 +104,56 @@ public class FileSinkCompactionSwitchITCase extends TestLogger {
protected static final int NUM_BUCKETS = 4;
- @ClassRule public static final TemporaryFolder TEMPORARY_FOLDER = new TemporaryFolder();
+ @TempDir private static java.nio.file.Path tmpDir;
- @Rule public final SharedObjects sharedObjects = SharedObjects.create();
+ @RegisterExtension final SharedObjectsExtension sharedObjects = SharedObjectsExtension.create();
Review Comment:
can be private
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/sink/utils/IntegerFileSinkTestDataUtils.java:
##########
@@ -117,25 +115,25 @@ public static void checkIntegerSequenceSinkOutput(
String path, int numRecords, int numBuckets, int numSources) throws Exception {
File dir = new File(path);
String[] subDirNames = dir.list();
- assertNotNull(subDirNames);
+ assertThat(subDirNames).isNotNull();
Arrays.sort(subDirNames, Comparator.comparingInt(Integer::parseInt));
- assertEquals(numBuckets, subDirNames.length);
+ assertThat(subDirNames).hasSize(numBuckets);
for (int i = 0; i < numBuckets; ++i) {
- assertEquals(Integer.toString(i), subDirNames[i]);
+ assertThat(subDirNames[i]).isEqualTo(Integer.toString(i));
// now check its content
File bucketDir = new File(path, subDirNames[i]);
- assertTrue(
- bucketDir.getAbsolutePath() + " Should be a existing directory",
- bucketDir.isDirectory());
+ assertThat(bucketDir.isDirectory())
+ .as(bucketDir.getAbsolutePath() + " Should be a existing directory")
+ .isTrue();
Review Comment:
```suggestion
assertThat(bucketDir)
.as(bucketDir.getAbsolutePath() + " Should be a existing directory")
.isDirectory();
```
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/sink/FileSinkITBase.java:
##########
@@ -47,20 +44,17 @@ public abstract class FileSinkITBase extends TestLogger {
protected static final double FAILOVER_RATIO = 0.4;
- @ClassRule public static final TemporaryFolder TEMPORARY_FOLDER = new TemporaryFolder();
-
- @Parameterized.Parameter public boolean triggerFailover;
-
- @Parameterized.Parameters(name = "triggerFailover = {0}")
- public static Collection<Object[]> params() {
- return Arrays.asList(new Object[] {false}, new Object[] {true});
+ public static Stream<Boolean> params() {
Review Comment:
can this be private?
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/FileSystemOutputFormatTest.java:
##########
@@ -147,39 +145,40 @@ public void testStaticPartition() throws Exception {
testHarness.processElement(new StreamRecord<>(Row.of("a2", 2), 1L));
testHarness.processElement(new StreamRecord<>(Row.of("a2", 2), 1L));
testHarness.processElement(new StreamRecord<>(Row.of("a3", 3), 1L));
- assertEquals(1, getFileContentByPath(tmpFile).size());
+ assertThat(getFileContentByPath(tmpPath)).hasSize(1);
}
ref.get().finalizeGlobal(1);
- Map<File, String> content = getFileContentByPath(outputFile);
- assertEquals(1, content.size());
- assertEquals("c=p1", content.keySet().iterator().next().getParentFile().getName());
- assertEquals("a1,1\n" + "a2,2\n" + "a2,2\n" + "a3,3\n", content.values().iterator().next());
- assertFalse(new File(tmpFile.toURI()).exists());
+ Map<File, String> content = getFileContentByPath(outputPath);
+ assertThat(content).hasSize(1);
+ assertThat(content.keySet().iterator().next().getParentFile().getName()).isEqualTo("c=p1");
+ assertThat(content.values().iterator().next())
+ .isEqualTo("a1,1\n" + "a2,2\n" + "a2,2\n" + "a3,3\n");
+ assertThat(new File(tmpPath.toUri())).doesNotExist();
}
@Test
- public void testDynamicPartition() throws Exception {
+ void testDynamicPartition() throws Exception {
AtomicReference<FileSystemOutputFormat<Row>> ref = new AtomicReference<>();
try (OneInputStreamOperatorTestHarness<Row, Object> testHarness =
createSink(false, true, false, new LinkedHashMap<>(), ref)) {
writeUnorderedRecords(testHarness);
- assertEquals(2, getFileContentByPath(tmpFile).size());
+ assertThat(getFileContentByPath(tmpPath)).hasSize(2);
}
ref.get().finalizeGlobal(1);
- Map<File, String> content = getFileContentByPath(outputFile);
+ Map<File, String> content = getFileContentByPath(outputPath);
Map<String, String> sortedContent = new TreeMap<>();
content.forEach((file, s) -> sortedContent.put(file.getParentFile().getName(), s));
- assertEquals(2, sortedContent.size());
- assertEquals("a1,1\n" + "a2,2\n" + "a3,3\n", sortedContent.get("c=p1"));
- assertEquals("a2,2\n", sortedContent.get("c=p2"));
- assertFalse(new File(tmpFile.toURI()).exists());
+ assertThat(sortedContent).hasSize(2);
+ assertThat(sortedContent.get("c=p1")).isEqualTo("a1,1\n" + "a2,2\n" + "a3,3\n");
+ assertThat(sortedContent.get("c=p2")).isEqualTo("a2,2\n");
Review Comment:
this _could_ be re-written to `contains(entry(...,...), ...)`
--
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
[GitHub] [flink] snuyanzin commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r875294629
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/FileSystemOutputFormatTest.java:
##########
@@ -147,39 +145,40 @@ public void testStaticPartition() throws Exception {
testHarness.processElement(new StreamRecord<>(Row.of("a2", 2), 1L));
testHarness.processElement(new StreamRecord<>(Row.of("a2", 2), 1L));
testHarness.processElement(new StreamRecord<>(Row.of("a3", 3), 1L));
- assertEquals(1, getFileContentByPath(tmpFile).size());
+ assertThat(getFileContentByPath(tmpPath)).hasSize(1);
}
ref.get().finalizeGlobal(1);
- Map<File, String> content = getFileContentByPath(outputFile);
- assertEquals(1, content.size());
- assertEquals("c=p1", content.keySet().iterator().next().getParentFile().getName());
- assertEquals("a1,1\n" + "a2,2\n" + "a2,2\n" + "a3,3\n", content.values().iterator().next());
- assertFalse(new File(tmpFile.toURI()).exists());
+ Map<File, String> content = getFileContentByPath(outputPath);
+ assertThat(content).hasSize(1);
+ assertThat(content.keySet().iterator().next().getParentFile().getName()).isEqualTo("c=p1");
+ assertThat(content.values().iterator().next())
+ .isEqualTo("a1,1\n" + "a2,2\n" + "a2,2\n" + "a3,3\n");
+ assertThat(new File(tmpPath.toUri())).doesNotExist();
}
@Test
- public void testDynamicPartition() throws Exception {
+ void testDynamicPartition() throws Exception {
AtomicReference<FileSystemOutputFormat<Row>> ref = new AtomicReference<>();
try (OneInputStreamOperatorTestHarness<Row, Object> testHarness =
createSink(false, true, false, new LinkedHashMap<>(), ref)) {
writeUnorderedRecords(testHarness);
- assertEquals(2, getFileContentByPath(tmpFile).size());
+ assertThat(getFileContentByPath(tmpPath)).hasSize(2);
}
ref.get().finalizeGlobal(1);
- Map<File, String> content = getFileContentByPath(outputFile);
+ Map<File, String> content = getFileContentByPath(outputPath);
Map<String, String> sortedContent = new TreeMap<>();
content.forEach((file, s) -> sortedContent.put(file.getParentFile().getName(), s));
- assertEquals(2, sortedContent.size());
- assertEquals("a1,1\n" + "a2,2\n" + "a3,3\n", sortedContent.get("c=p1"));
- assertEquals("a2,2\n", sortedContent.get("c=p2"));
- assertFalse(new File(tmpFile.toURI()).exists());
+ assertThat(sortedContent).hasSize(2);
+ assertThat(sortedContent.get("c=p1")).isEqualTo("a1,1\n" + "a2,2\n" + "a3,3\n");
+ assertThat(sortedContent.get("c=p2")).isEqualTo("a2,2\n");
Review Comment:
done
--
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
[GitHub] [flink] zentol merged pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
zentol merged PR #19716:
URL: https://github.com/apache/flink/pull/19716
--
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
[GitHub] [flink] flinkbot commented on pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #19716:
URL: https://github.com/apache/flink/pull/19716#issuecomment-1126061144
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "f8c3129f7ed0028a2a4002cab0ef26877b5f3653",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "f8c3129f7ed0028a2a4002cab0ef26877b5f3653",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* f8c3129f7ed0028a2a4002cab0ef26877b5f3653 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
[GitHub] [flink] snuyanzin commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r875295088
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/table/PartitionWriterTest.java:
##########
@@ -23,26 +23,39 @@
import org.apache.flink.connector.file.table.PartitionWriter.Context;
import org.apache.flink.core.fs.FileSystem;
import org.apache.flink.core.fs.Path;
-import org.apache.flink.table.utils.LegacyRowResource;
import org.apache.flink.types.Row;
+import org.apache.flink.types.RowUtils;
-import org.junit.Assert;
-import org.junit.ClassRule;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import static org.assertj.core.api.Assertions.assertThat;
+
/** Test for {@link PartitionWriter}s. */
-public class PartitionWriterTest {
+class PartitionWriterTest {
+
+ @TempDir private java.nio.file.Path tmpDir;
- @Rule public final LegacyRowResource usesLegacyRows = LegacyRowResource.INSTANCE;
Review Comment:
ok, thanks,
I've made these methods public and started to call them
--
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
[GitHub] [flink] snuyanzin commented on a diff in pull request #19716: [FLINK-27607][tests] Migrate module flink-connector-files to JUnit5
Posted by GitBox <gi...@apache.org>.
snuyanzin commented on code in PR #19716:
URL: https://github.com/apache/flink/pull/19716#discussion_r875293489
##########
flink-connectors/flink-connector-files/src/test/java/org/apache/flink/connector/file/sink/FileSinkITBase.java:
##########
@@ -47,20 +44,17 @@ public abstract class FileSinkITBase extends TestLogger {
protected static final double FAILOVER_RATIO = 0.4;
- @ClassRule public static final TemporaryFolder TEMPORARY_FOLDER = new TemporaryFolder();
-
- @Parameterized.Parameter public boolean triggerFailover;
-
- @Parameterized.Parameters(name = "triggerFailover = {0}")
- public static Collection<Object[]> params() {
- return Arrays.asList(new Object[] {false}, new Object[] {true});
+ public static Stream<Boolean> params() {
Review Comment:
yep, done
--
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