XComp commented on code in PR #20230:
URL: https://github.com/apache/flink/pull/20230#discussion_r989201719
##########
flink-formats/flink-parquet/src/test/java/org/apache/flink/formats/parquet/protobuf/ParquetProtoStreamingFileSinkITCase.java:
##########
@@ -49,13 +50,12 @@
* Simple integration test case for writing bulk encoded files with the {@link
StreamingFileSink}
* with Parquet.
*/
-public class ParquetProtoStreamingFileSinkITCase extends AbstractTestBase {
-
- @Rule public final Timeout timeoutPerTest = Timeout.seconds(20);
+@ExtendWith(MiniClusterExtension.class)
+@Timeout(20)
Review Comment:
There was a community discussion on trying to avoid timeouts (see
[discussion
thread](https://lists.apache.org/thread/q11t0y3qsw07lkb811037h4mvx0r7ncn) and
[corresponding Flink coding
guidelines](https://flink.apache.org/contributing/code-style-and-quality-common.html#avoid-timeouts-in-junit-tests)).
The decision was only made on unit tests but I don't see why the same
wouldn't also apply for integration tests. WDYT?
##########
flink-formats/flink-parquet/src/test/java/org/apache/flink/formats/parquet/vector/ParquetColumnarRowSplitReaderTest.java:
##########
@@ -137,21 +134,15 @@ public class ParquetColumnarRowSplitReaderTest {
.as(OriginalType.DECIMAL)
.named("f14"));
- @ClassRule public static final TemporaryFolder TEMPORARY_FOLDER = new
TemporaryFolder();
+ @TempDir java.io.File tmpDir;
Review Comment:
```suggestion
@TempDir File tmpDir;
```
##########
flink-formats/flink-parquet/src/test/java/org/apache/flink/formats/parquet/avro/AvroParquetStreamingFileSinkITCase.java:
##########
@@ -59,15 +60,12 @@
* Simple integration test case for writing bulk encoded files with the {@link
StreamingFileSink}
* with Parquet.
*/
-@SuppressWarnings("serial")
-public class AvroParquetStreamingFileSinkITCase extends AbstractTestBase {
-
- @Rule public final Timeout timeoutPerTest = Timeout.seconds(20);
+@ExtendWith(MiniClusterExtension.class)
+@Timeout(20)
Review Comment:
There was a community discussion on trying to avoid timeouts (see
[discussion
thread](https://lists.apache.org/thread/q11t0y3qsw07lkb811037h4mvx0r7ncn) and
[corresponding Flink coding
guidelines](https://flink.apache.org/contributing/code-style-and-quality-common.html#avoid-timeouts-in-junit-tests)).
The decision was only made on unit tests but I don't see why the same
wouldn't also apply for integration tests. WDYT?
##########
flink-formats/flink-parquet/src/test/java/org/apache/flink/formats/parquet/protobuf/ParquetProtoStreamingFileSinkITCase.java:
##########
@@ -49,13 +50,12 @@
* Simple integration test case for writing bulk encoded files with the {@link
StreamingFileSink}
* with Parquet.
*/
-public class ParquetProtoStreamingFileSinkITCase extends AbstractTestBase {
-
- @Rule public final Timeout timeoutPerTest = Timeout.seconds(20);
+@ExtendWith(MiniClusterExtension.class)
Review Comment:
Is there a difference between using `@ExtendWith` and `@RegisterExtension`
on a field member like you did it in `AvroParquetFileReadITCase`? :thinking:
shall we go for a consistent approach if not?
##########
flink-formats/flink-parquet/src/test/java/org/apache/flink/formats/parquet/row/ParquetRowDataWriterTest.java:
##########
@@ -112,26 +109,27 @@ public class ParquetRowDataWriterTest {
TypeConversions.fromLogicalToDataType(ROW_TYPE));
@Test
- public void testTypes() throws Exception {
+ void testTypes(@TempDir java.nio.file.Path folder) throws Exception {
Review Comment:
nit: Here and in the other test method: We could switch to `File` to avoid
having the fully qualified reference.
##########
flink-formats/flink-parquet/src/test/java/org/apache/flink/formats/parquet/ParquetColumnarRowInputFormatTest.java:
##########
@@ -125,21 +122,15 @@ public class ParquetColumnarRowInputFormatTest {
.as(OriginalType.DECIMAL)
.named("f14"));
- @ClassRule public static final TemporaryFolder TEMPORARY_FOLDER = new
TemporaryFolder();
+ @TempDir private java.io.File folder;
Review Comment:
```suggestion
@TempDir private File folder;
```
--
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
