zentol commented on code in PR #18983:
URL: https://github.com/apache/flink/pull/18983#discussion_r865647104
##########
flink-runtime/src/test/java/org/apache/flink/runtime/resourcemanager/active/ResourceManagerDriverTestBase.java:
##########
@@ -39,12 +38,10 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertThat;
+import static org.assertj.core.api.Assertions.assertThat;
/** Common test cases for implementations of {@link ResourceManagerDriver}. */
-public abstract class ResourceManagerDriverTestBase<WorkerType extends
ResourceIDRetrievable>
- extends TestLogger {
+public abstract class ResourceManagerDriverTestBase<WorkerType extends
ResourceIDRetrievable> {
Review Comment:
Let's annotate this test instead of adding the TestLoggerExtension service
entry. As is we'd load the test logger also for all junit4 tests in
flink-runtime which as far as I was told causes issues.
##########
flink-yarn/src/test/java/org/apache/flink/yarn/RegisterApplicationMasterResponseReflectorTest.java:
##########
@@ -81,28 +75,28 @@ public void
testDoesntCallGetContainersFromPreviousAttemptsMethodIfAbsent() {
registerApplicationMasterResponseReflector.getContainersFromPreviousAttemptsUnsafe(
new Object());
- assertThat(containersFromPreviousAttemptsUnsafe, empty());
+ assertThat(containersFromPreviousAttemptsUnsafe).isEmpty();
}
@Test
- public void
testGetContainersFromPreviousAttemptsMethodReflectiveHadoop22() {
+ void testGetContainersFromPreviousAttemptsMethodReflectiveHadoop22() {
assumeTrue(
+ isHadoopVersionGreaterThanOrEquals(2, 2),
Review Comment:
could be removed since we use a newer version nowadays
##########
flink-yarn/src/test/java/org/apache/flink/yarn/RegisterApplicationMasterResponseReflectorTest.java:
##########
@@ -125,24 +119,22 @@ public void
testDoesntCallGetSchedulerResourceTypesMethodIfAbsent() {
registerApplicationMasterResponseReflector.getSchedulerResourceTypeNamesUnsafe(
new Object());
- assertFalse(schedulerResourceTypeNames.isPresent());
+ assertThat(schedulerResourceTypeNames).isNotPresent();
}
@Test
- public void testGetSchedulerResourceTypesMethodReflectiveHadoop26() {
+ void testGetSchedulerResourceTypesMethodReflectiveHadoop26() {
assumeTrue(
+ isHadoopVersionGreaterThanOrEquals(2, 6),
Review Comment:
could be removed since we use a newer version nowadays
##########
flink-yarn/src/test/java/org/apache/flink/yarn/TaskExecutorProcessSpecContainerResourcePriorityAdapterTest.java:
##########
@@ -206,40 +199,49 @@ public void testExternalResource() {
final Map<String, Long> resultExternalResources =
ResourceInformationReflector.INSTANCE.getExternalResources(resource);
- assertThat(resultExternalResources.size(), is(1));
- assertThat(
-
resultExternalResources.get(SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY),
- is(SUPPORTED_EXTERNAL_RESOURCE_MAX));
+ assertThat(resultExternalResources).hasSize(1);
+
assertThat(resultExternalResources.get(SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY))
+ .isEqualTo(SUPPORTED_EXTERNAL_RESOURCE_MAX);
}
- @Test(expected = IllegalStateException.class)
- public void testExternalResourceFailExceedMax() {
+ @Test
+ void testExternalResourceFailExceedMax() {
assumeTrue(isExternalResourceSupported());
-
- getAdapterWithExternalResources(
- SUPPORTED_EXTERNAL_RESOURCE_NAME,
SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
- .getPriorityAndResource(
-
TASK_EXECUTOR_PROCESS_SPEC_WITH_EXTERNAL_RESOURCE_EXCEED_MAX);
+ assertThatThrownBy(
+ () ->
+ getAdapterWithExternalResources(
+
SUPPORTED_EXTERNAL_RESOURCE_NAME,
+
SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
Review Comment:
I would prefer if this were called outside to clarify what call throws the
exception.
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTest.java:
##########
@@ -81,21 +73,19 @@ public class YarnFileStageTest extends TestLogger {
// Test setup and shutdown
// ------------------------------------------------------------------------
- @BeforeClass
- public static void createHDFS() throws Exception {
- Assume.assumeTrue(!OperatingSystem.isWindows());
-
- final File tempDir = CLASS_TEMP_DIR.newFolder();
+ @BeforeAll
+ public static void createHDFS(@TempDir File hdfsTempDir) throws Exception {
+ assumeTrue(!OperatingSystem.isWindows());
org.apache.hadoop.conf.Configuration hdConf = new
org.apache.hadoop.conf.Configuration();
- hdConf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR,
tempDir.getAbsolutePath());
+ hdConf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR,
hdfsTempDir.getAbsolutePath());
MiniDFSCluster.Builder builder = new MiniDFSCluster.Builder(hdConf);
hdfsCluster = builder.build();
hdfsRootPath = new Path(hdfsCluster.getURI());
}
- @AfterClass
+ @AfterAll
public static void destroyHDFS() {
Review Comment:
```suggestion
static void destroyHDFS() {
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnApplicationFileUploaderTest.java:
##########
@@ -73,15 +69,18 @@ public void testRegisterProvidedLocalResources() throws
IOException {
final Set<String> registeredResources =
yarnApplicationFileUploader.getRegisteredLocalResources().keySet();
- assertThat(
- registeredResources,
Matchers.containsInAnyOrder(libJars.keySet().toArray()));
+ assertThat(registeredResources)
+ .containsExactlyInAnyOrder(libJars.keySet().toArray(new
String[0]));
Review Comment:
```suggestion
.containsExactlyInAnyOrderElementsOf(libJars.keySet());
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTestS3ITCase.java:
##########
@@ -173,36 +175,44 @@ private void testRecursiveUploadForYarn(String scheme,
String pathSuffix) throws
}
}
- @Test
- @RetryOnFailure(times = 3)
- public void testRecursiveUploadForYarnS3n() throws Exception {
+ @TestTemplate
+ @RetryOnException(times = 3, exception = Exception.class)
+ public void testRecursiveUploadForYarnS3n(@TempDir File tempFolder) throws
Exception {
// skip test on Hadoop 3:
https://issues.apache.org/jira/browse/HADOOP-14738
- Assume.assumeTrue(
- "This test is skipped for Hadoop versions above 3",
-
VersionUtil.compareVersions(System.getProperty("hadoop.version"), "3.0.0") < 0);
+ assumeTrue(
+
VersionUtil.compareVersions(System.getProperty("hadoop.version"), "3.0.0") < 0,
+ "This test is skipped for Hadoop versions above 3");
try {
Class.forName("org.apache.hadoop.fs.s3native.NativeS3FileSystem");
} catch (ClassNotFoundException e) {
// not in the classpath, cannot run this test
String msg = "Skipping test because NativeS3FileSystem is not in
the class path";
log.info(msg);
- assumeNoException(msg, e);
+ assumingThat(
+ true,
+ () -> {
+ throw e;
+ });
}
Review Comment:
```
assumeThatThrownBy(() ->
Class.forName("org.apache.hadoop.fs.s3native.NativeS3FileSystem")
.as("Skipping test because NativeS3FileSystem is not in the class path")
.isNull();
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTestS3ITCase.java:
##########
@@ -173,36 +175,44 @@ private void testRecursiveUploadForYarn(String scheme,
String pathSuffix) throws
}
}
- @Test
- @RetryOnFailure(times = 3)
- public void testRecursiveUploadForYarnS3n() throws Exception {
+ @TestTemplate
+ @RetryOnException(times = 3, exception = Exception.class)
+ public void testRecursiveUploadForYarnS3n(@TempDir File tempFolder) throws
Exception {
// skip test on Hadoop 3:
https://issues.apache.org/jira/browse/HADOOP-14738
- Assume.assumeTrue(
- "This test is skipped for Hadoop versions above 3",
-
VersionUtil.compareVersions(System.getProperty("hadoop.version"), "3.0.0") < 0);
+ assumeTrue(
+
VersionUtil.compareVersions(System.getProperty("hadoop.version"), "3.0.0") < 0,
+ "This test is skipped for Hadoop versions above 3");
try {
Class.forName("org.apache.hadoop.fs.s3native.NativeS3FileSystem");
} catch (ClassNotFoundException e) {
// not in the classpath, cannot run this test
String msg = "Skipping test because NativeS3FileSystem is not in
the class path";
log.info(msg);
- assumeNoException(msg, e);
+ assumingThat(
+ true,
+ () -> {
+ throw e;
+ });
}
- testRecursiveUploadForYarn("s3n", "testYarn-s3n");
+ testRecursiveUploadForYarn("s3n", "testYarn-s3n", tempFolder);
}
- @Test
- @RetryOnFailure(times = 3)
- public void testRecursiveUploadForYarnS3a() throws Exception {
+ @TestTemplate
+ @RetryOnException(times = 3, exception = Exception.class)
+ public void testRecursiveUploadForYarnS3a(@TempDir File tempFolder) throws
Exception {
try {
Class.forName("org.apache.hadoop.fs.s3a.S3AFileSystem");
} catch (ClassNotFoundException e) {
// not in the classpath, cannot run this test
String msg = "Skipping test because S3AFileSystem is not in the
class path";
log.info(msg);
- assumeNoException(msg, e);
+ assumingThat(
Review Comment:
see above
##########
flink-yarn/src/test/java/org/apache/flink/yarn/UtilsTest.java:
##########
@@ -19,59 +19,50 @@
package org.apache.flink.yarn;
import org.apache.flink.configuration.Configuration;
-import org.apache.flink.core.testutils.FlinkMatchers;
import org.apache.flink.util.FlinkException;
-import org.apache.flink.util.TestLogger;
import org.apache.flink.yarn.configuration.YarnConfigOptions;
import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
import org.apache.hadoop.yarn.api.records.Resource;
import org.apache.hadoop.yarn.conf.YarnConfiguration;
-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.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
import java.util.List;
import java.util.stream.Stream;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.fail;
/** Tests for {@link Utils}. */
-public class UtilsTest extends TestLogger {
+class UtilsTest {
private static final String YARN_RM_ARBITRARY_SCHEDULER_CLAZZ =
"org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacityScheduler";
- @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();
-
@Test
- public void testDeleteApplicationFiles() throws Exception {
- final Path applicationFilesDir =
temporaryFolder.newFolder(".flink").toPath();
- Files.createFile(applicationFilesDir.resolve("flink.jar"));
- try (Stream<Path> files =
Files.list(temporaryFolder.getRoot().toPath())) {
- assertThat(files.count(), equalTo(1L));
+ void testDeleteApplicationFiles(@TempDir Path tempDir) throws Exception {
+ final Path applicationFilesDir = Files.createTempDirectory(tempDir,
".flink");
+ Files.createTempFile(applicationFilesDir, "flink", ".jar");
+ try (Stream<Path> files = Files.list(tempDir)) {
+ assertThat(files.count()).isEqualTo(1L);
}
try (Stream<Path> files = Files.list(applicationFilesDir)) {
- assertThat(files.count(), equalTo(1L));
+ assertThat(files.count()).isEqualTo(1L);
Review Comment:
```suggestion
assertThat(files).hasSize(1L);
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/FlinkYarnSessionCliTest.java:
##########
@@ -574,12 +580,13 @@ public void testMissingShipFiles() throws Exception {
final CommandLine commandLine =
flinkYarnSessionCli.parseCommandLineOptions(args, false);
- try {
- flinkYarnSessionCli.toConfiguration(commandLine);
- fail("Expected error for missing file");
- } catch (ConfigurationException ce) {
- assertEquals("Ship file missing.file does not exist",
ce.getMessage());
- }
+ assertThatThrownBy(
+ () -> {
+ flinkYarnSessionCli.toConfiguration(commandLine);
+ fail("Expected error for missing file");
Review Comment:
```suggestion
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/RegisterApplicationMasterResponseReflectorTest.java:
##########
@@ -111,12 +105,12 @@ public void
testCallsGetSchedulerResourceTypesMethodIfPresent() {
registerApplicationMasterResponseReflector.getSchedulerResourceTypeNamesUnsafe(
new HasMethod());
- assertTrue(schedulerResourceTypeNames.isPresent());
- assertThat(schedulerResourceTypeNames.get(),
containsInAnyOrder("MEMORY", "CPU"));
+ assertThat(schedulerResourceTypeNames).isPresent();
+
assertThat(schedulerResourceTypeNames.get()).containsAll(Arrays.asList("MEMORY",
"CPU"));
Review Comment:
```suggestion
assertThat(schedulerResourceTypeNames.get()).contains("MEMORY",
"CPU");
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/TaskExecutorProcessSpecContainerResourcePriorityAdapterTest.java:
##########
@@ -206,40 +199,49 @@ public void testExternalResource() {
final Map<String, Long> resultExternalResources =
ResourceInformationReflector.INSTANCE.getExternalResources(resource);
- assertThat(resultExternalResources.size(), is(1));
- assertThat(
-
resultExternalResources.get(SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY),
- is(SUPPORTED_EXTERNAL_RESOURCE_MAX));
+ assertThat(resultExternalResources).hasSize(1);
+
assertThat(resultExternalResources.get(SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY))
+ .isEqualTo(SUPPORTED_EXTERNAL_RESOURCE_MAX);
Review Comment:
```suggestion
assertThat(resultExternalResources).containsExactly(
entry(
SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY,
SUPPORTED_EXTERNAL_RESOURCE_MAX));
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/TaskExecutorProcessSpecContainerResourcePriorityAdapterTest.java:
##########
@@ -206,40 +199,49 @@ public void testExternalResource() {
final Map<String, Long> resultExternalResources =
ResourceInformationReflector.INSTANCE.getExternalResources(resource);
- assertThat(resultExternalResources.size(), is(1));
- assertThat(
-
resultExternalResources.get(SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY),
- is(SUPPORTED_EXTERNAL_RESOURCE_MAX));
+ assertThat(resultExternalResources).hasSize(1);
+
assertThat(resultExternalResources.get(SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY))
+ .isEqualTo(SUPPORTED_EXTERNAL_RESOURCE_MAX);
}
- @Test(expected = IllegalStateException.class)
- public void testExternalResourceFailExceedMax() {
+ @Test
+ void testExternalResourceFailExceedMax() {
assumeTrue(isExternalResourceSupported());
-
- getAdapterWithExternalResources(
- SUPPORTED_EXTERNAL_RESOURCE_NAME,
SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
- .getPriorityAndResource(
-
TASK_EXECUTOR_PROCESS_SPEC_WITH_EXTERNAL_RESOURCE_EXCEED_MAX);
+ assertThatThrownBy(
+ () ->
+ getAdapterWithExternalResources(
+
SUPPORTED_EXTERNAL_RESOURCE_NAME,
+
SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
+ .getPriorityAndResource(
+
TASK_EXECUTOR_PROCESS_SPEC_WITH_EXTERNAL_RESOURCE_EXCEED_MAX))
+ .isInstanceOf(IllegalStateException.class);
}
- @Test(expected = IllegalStateException.class)
- public void testExternalResourceFailResourceTypeNotSupported() {
+ @Test
+ void testExternalResourceFailResourceTypeNotSupported() {
assumeTrue(isExternalResourceSupported());
-
- getAdapterWithExternalResources(
- UNSUPPORTED_EXTERNAL_RESOURCE_NAME,
- UNSUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
- .getPriorityAndResource(
-
TASK_EXECUTOR_PROCESS_SPEC_WITH_UNSUPPORTED_EXTERNAL_RESOURCE);
+ assertThatThrownBy(
+ () ->
+ getAdapterWithExternalResources(
+
UNSUPPORTED_EXTERNAL_RESOURCE_NAME,
+
UNSUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
+ .getPriorityAndResource(
+
TASK_EXECUTOR_PROCESS_SPEC_WITH_UNSUPPORTED_EXTERNAL_RESOURCE))
+ .isInstanceOf(IllegalStateException.class);
}
- @Test(expected = IllegalStateException.class)
- public void testExternalResourceFailHadoopVersionNotSupported() {
- assumeFalse(isExternalResourceSupported());
+ @Test
+ void testExternalResourceFailHadoopVersionNotSupported() {
- getAdapterWithExternalResources(
- SUPPORTED_EXTERNAL_RESOURCE_NAME,
SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
-
.getPriorityAndResource(TASK_EXECUTOR_PROCESS_SPEC_WITH_EXTERNAL_RESOURCE);
+ assumeFalse(isExternalResourceSupported());
+ assertThatThrownBy(
+ () ->
+ getAdapterWithExternalResources(
Review Comment:
see above
##########
flink-yarn/src/test/java/org/apache/flink/yarn/UtilsTest.java:
##########
@@ -109,31 +100,34 @@ public void testSharedLibWithNonQualifiedPath() throws
Exception {
final List<org.apache.hadoop.fs.Path> sharedLibs =
Utils.getQualifiedRemoteSharedPaths(flinkConfig, yarnConfig);
- assertThat(sharedLibs.size(), is(1));
- assertThat(sharedLibs.get(0).toUri().toString(), is(qualifiedPath));
+ assertThat(sharedLibs).hasSize(1);
+
assertThat(sharedLibs.get(0).toUri().toString()).isEqualTo(qualifiedPath);
}
@Test
- public void testSharedLibIsNotRemotePathShouldThrowException() throws
IOException {
+ void testSharedLibIsNotRemotePathShouldThrowException() {
final String localLib = "file:///flink/sharedLib";
final Configuration flinkConfig = new Configuration();
flinkConfig.set(YarnConfigOptions.PROVIDED_LIB_DIRS,
Collections.singletonList(localLib));
- try {
- Utils.getQualifiedRemoteSharedPaths(flinkConfig, new
YarnConfiguration());
- fail("We should throw an exception when the shared lib is set to
local path.");
- } catch (FlinkException ex) {
- final String msg =
- "The \""
- + YarnConfigOptions.PROVIDED_LIB_DIRS.key()
- + "\" should only "
- + "contain dirs accessible from all worker nodes";
- assertThat(ex, FlinkMatchers.containsMessage(msg));
- }
+ final String msg =
+ "The \""
+ + YarnConfigOptions.PROVIDED_LIB_DIRS.key()
+ + "\" should only "
+ + "contain dirs accessible from all worker nodes";
+ assertThatThrownBy(
+ () -> {
+ Utils.getQualifiedRemoteSharedPaths(
+ flinkConfig, new YarnConfiguration());
+ fail(
+ "We should throw an exception when the
shared lib is set to local path.");
Review Comment:
```suggestion
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTest.java:
##########
@@ -320,9 +302,9 @@ private static void testCopySingleFileFromLocal(
localResourceDirectory,
LocalResourceType.FILE);
- assertThat(
- classpath,
- containsInAnyOrder(new Path(localResourceDirectory,
localFile).toString()));
+ assertThat(classpath)
+ .containsExactlyInAnyOrder(
Review Comment:
```suggestion
.containsExactly(
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/TaskExecutorProcessSpecContainerResourcePriorityAdapterTest.java:
##########
@@ -206,40 +199,49 @@ public void testExternalResource() {
final Map<String, Long> resultExternalResources =
ResourceInformationReflector.INSTANCE.getExternalResources(resource);
- assertThat(resultExternalResources.size(), is(1));
- assertThat(
-
resultExternalResources.get(SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY),
- is(SUPPORTED_EXTERNAL_RESOURCE_MAX));
+ assertThat(resultExternalResources).hasSize(1);
+
assertThat(resultExternalResources.get(SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY))
+ .isEqualTo(SUPPORTED_EXTERNAL_RESOURCE_MAX);
}
- @Test(expected = IllegalStateException.class)
- public void testExternalResourceFailExceedMax() {
+ @Test
+ void testExternalResourceFailExceedMax() {
assumeTrue(isExternalResourceSupported());
-
- getAdapterWithExternalResources(
- SUPPORTED_EXTERNAL_RESOURCE_NAME,
SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
- .getPriorityAndResource(
-
TASK_EXECUTOR_PROCESS_SPEC_WITH_EXTERNAL_RESOURCE_EXCEED_MAX);
+ assertThatThrownBy(
+ () ->
+ getAdapterWithExternalResources(
+
SUPPORTED_EXTERNAL_RESOURCE_NAME,
+
SUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
+ .getPriorityAndResource(
+
TASK_EXECUTOR_PROCESS_SPEC_WITH_EXTERNAL_RESOURCE_EXCEED_MAX))
+ .isInstanceOf(IllegalStateException.class);
}
- @Test(expected = IllegalStateException.class)
- public void testExternalResourceFailResourceTypeNotSupported() {
+ @Test
+ void testExternalResourceFailResourceTypeNotSupported() {
assumeTrue(isExternalResourceSupported());
-
- getAdapterWithExternalResources(
- UNSUPPORTED_EXTERNAL_RESOURCE_NAME,
- UNSUPPORTED_EXTERNAL_RESOURCE_CONFIG_KEY)
- .getPriorityAndResource(
-
TASK_EXECUTOR_PROCESS_SPEC_WITH_UNSUPPORTED_EXTERNAL_RESOURCE);
+ assertThatThrownBy(
+ () ->
+ getAdapterWithExternalResources(
Review Comment:
see above
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTest.java:
##########
@@ -443,7 +419,9 @@ private static void verifyDirectoryRecursive(
String relativePath =
absolutePathString.substring(workDirPrefixLength);
targetFiles.put(relativePath, in.readUTF());
- assertEquals("extraneous data in file " + relativePath,
-1, in.read());
+ if (in.read() != -1) {
+ fail("extraneous data in file " + relativePath);
+ }
Review Comment:
```suggestion
assertThat(in.read())
.as("extraneous data in file " + relativePath)
.isNotEqualTo(-1);
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTest.java:
##########
@@ -81,21 +73,19 @@ public class YarnFileStageTest extends TestLogger {
// Test setup and shutdown
// ------------------------------------------------------------------------
- @BeforeClass
- public static void createHDFS() throws Exception {
- Assume.assumeTrue(!OperatingSystem.isWindows());
-
- final File tempDir = CLASS_TEMP_DIR.newFolder();
+ @BeforeAll
+ public static void createHDFS(@TempDir File hdfsTempDir) throws Exception {
Review Comment:
```suggestion
static void createHDFS(@TempDir File hdfsTempDir) throws Exception {
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTestS3ITCase.java:
##########
@@ -24,71 +24,70 @@
import org.apache.flink.core.fs.FileSystem;
import org.apache.flink.core.fs.Path;
import org.apache.flink.runtime.fs.hdfs.HadoopFileSystem;
-import org.apache.flink.testutils.junit.RetryOnFailure;
-import org.apache.flink.testutils.junit.RetryRule;
+import org.apache.flink.testutils.junit.RetryOnException;
+import org.apache.flink.testutils.junit.extensions.retry.RetryExtension;
import org.apache.flink.testutils.s3.S3TestCredentials;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.util.TestLoggerExtension;
import org.apache.hadoop.util.VersionUtil;
-import org.junit.AfterClass;
-import org.junit.Assume;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.PrintStream;
+import java.nio.file.Files;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.greaterThan;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assume.assumeFalse;
-import static org.junit.Assume.assumeNoException;
+import static org.junit.jupiter.api.Assumptions.assumeFalse;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import static org.junit.jupiter.api.Assumptions.assumingThat;
/**
* Tests for verifying file staging during submission to YARN works with the
S3A file system.
*
* <p>Note that the setup is similar to
* <tt>org.apache.flink.fs.s3hadoop.HadoopS3FileSystemITCase</tt>.
*/
-public class YarnFileStageTestS3ITCase extends TestLogger {
+@ExtendWith({TestLoggerExtension.class, RetryExtension.class})
Review Comment:
```suggestion
@ExtendWith(RetryExtension.class)
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTest.java:
##########
@@ -104,7 +94,7 @@ public static void destroyHDFS() {
hdfsRootPath = null;
}
- @Before
+ @BeforeEach
public void initConfig() {
Review Comment:
```suggestion
void initConfig() {
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/UtilsTest.java:
##########
@@ -19,59 +19,50 @@
package org.apache.flink.yarn;
import org.apache.flink.configuration.Configuration;
-import org.apache.flink.core.testutils.FlinkMatchers;
import org.apache.flink.util.FlinkException;
-import org.apache.flink.util.TestLogger;
import org.apache.flink.yarn.configuration.YarnConfigOptions;
import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
import org.apache.hadoop.yarn.api.records.Resource;
import org.apache.hadoop.yarn.conf.YarnConfiguration;
-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.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
import java.util.List;
import java.util.stream.Stream;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.fail;
/** Tests for {@link Utils}. */
-public class UtilsTest extends TestLogger {
+class UtilsTest {
private static final String YARN_RM_ARBITRARY_SCHEDULER_CLAZZ =
"org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.CapacityScheduler";
- @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();
-
@Test
- public void testDeleteApplicationFiles() throws Exception {
- final Path applicationFilesDir =
temporaryFolder.newFolder(".flink").toPath();
- Files.createFile(applicationFilesDir.resolve("flink.jar"));
- try (Stream<Path> files =
Files.list(temporaryFolder.getRoot().toPath())) {
- assertThat(files.count(), equalTo(1L));
+ void testDeleteApplicationFiles(@TempDir Path tempDir) throws Exception {
+ final Path applicationFilesDir = Files.createTempDirectory(tempDir,
".flink");
+ Files.createTempFile(applicationFilesDir, "flink", ".jar");
+ try (Stream<Path> files = Files.list(tempDir)) {
+ assertThat(files.count()).isEqualTo(1L);
}
try (Stream<Path> files = Files.list(applicationFilesDir)) {
- assertThat(files.count(), equalTo(1L));
+ assertThat(files.count()).isEqualTo(1L);
}
Utils.deleteApplicationFiles(applicationFilesDir.toString());
- try (Stream<Path> files =
Files.list(temporaryFolder.getRoot().toPath())) {
- assertThat(files.count(), equalTo(0L));
+ try (Stream<Path> files = Files.list(tempDir.toFile().toPath())) {
+ assertThat(files.count()).isEqualTo(0L);
Review Comment:
```suggestion
assertThat(files).isEmpty();
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTestS3ITCase.java:
##########
@@ -24,71 +24,70 @@
import org.apache.flink.core.fs.FileSystem;
import org.apache.flink.core.fs.Path;
import org.apache.flink.runtime.fs.hdfs.HadoopFileSystem;
-import org.apache.flink.testutils.junit.RetryOnFailure;
-import org.apache.flink.testutils.junit.RetryRule;
+import org.apache.flink.testutils.junit.RetryOnException;
+import org.apache.flink.testutils.junit.extensions.retry.RetryExtension;
import org.apache.flink.testutils.s3.S3TestCredentials;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.util.TestLoggerExtension;
import org.apache.hadoop.util.VersionUtil;
-import org.junit.AfterClass;
-import org.junit.Assume;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.PrintStream;
+import java.nio.file.Files;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.greaterThan;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assume.assumeFalse;
-import static org.junit.Assume.assumeNoException;
+import static org.junit.jupiter.api.Assumptions.assumeFalse;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import static org.junit.jupiter.api.Assumptions.assumingThat;
/**
* Tests for verifying file staging during submission to YARN works with the
S3A file system.
*
* <p>Note that the setup is similar to
* <tt>org.apache.flink.fs.s3hadoop.HadoopS3FileSystemITCase</tt>.
*/
-public class YarnFileStageTestS3ITCase extends TestLogger {
+@ExtendWith({TestLoggerExtension.class, RetryExtension.class})
+public class YarnFileStageTestS3ITCase {
- private static final String TEST_DATA_DIR = "tests-" + UUID.randomUUID();
-
- @ClassRule public static final TemporaryFolder TEMP_FOLDER = new
TemporaryFolder();
+ private static final Logger log =
LoggerFactory.getLogger(YarnFileStageTestS3ITCase.class);
- @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
-
- @Rule public final RetryRule retryRule = new RetryRule();
+ private static final String TEST_DATA_DIR = "tests-" + UUID.randomUUID();
Review Comment:
is this unused?
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerDriverTest.java:
##########
@@ -175,15 +175,13 @@ public void testShutdownRequestCausesFatalError() throws
Exception {
Throwable throwable =
throwableCompletableFuture.get(TIMEOUT_SEC, TimeUnit.SECONDS);
assertThat(
- ExceptionUtils.findThrowable(
- throwable,
ResourceManagerException.class)
- .isPresent(),
- is(true));
+ ExceptionUtils.findThrowable(
+ throwable,
ResourceManagerException.class))
+ .isPresent();
assertThat(
- ExceptionUtils.findThrowableWithMessage(
- throwable,
ERROR_MESSAGE_ON_SHUTDOWN_REQUEST)
- .isPresent(),
- is(true));
+
ExceptionUtils.findThrowableWithMessage(
+ throwable,
ERROR_MESSAGE_ON_SHUTDOWN_REQUEST))
+ .isPresent();
Review Comment:
```
assertThat(throwable)
.satisfies(anyCauseMatches(ResourceManagerException.class)
.satisfies(anyCauseMatches(ERROR_MESSAGE_ON_SHUTDOWN_REQUEST);
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnFileStageTestS3ITCase.java:
##########
@@ -24,71 +24,70 @@
import org.apache.flink.core.fs.FileSystem;
import org.apache.flink.core.fs.Path;
import org.apache.flink.runtime.fs.hdfs.HadoopFileSystem;
-import org.apache.flink.testutils.junit.RetryOnFailure;
-import org.apache.flink.testutils.junit.RetryRule;
+import org.apache.flink.testutils.junit.RetryOnException;
+import org.apache.flink.testutils.junit.extensions.retry.RetryExtension;
import org.apache.flink.testutils.s3.S3TestCredentials;
-import org.apache.flink.util.TestLogger;
+import org.apache.flink.util.TestLoggerExtension;
import org.apache.hadoop.util.VersionUtil;
-import org.junit.AfterClass;
-import org.junit.Assume;
-import org.junit.BeforeClass;
-import org.junit.ClassRule;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.PrintStream;
+import java.nio.file.Files;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.greaterThan;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assume.assumeFalse;
-import static org.junit.Assume.assumeNoException;
+import static org.junit.jupiter.api.Assumptions.assumeFalse;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import static org.junit.jupiter.api.Assumptions.assumingThat;
/**
* Tests for verifying file staging during submission to YARN works with the
S3A file system.
*
* <p>Note that the setup is similar to
* <tt>org.apache.flink.fs.s3hadoop.HadoopS3FileSystemITCase</tt>.
*/
-public class YarnFileStageTestS3ITCase extends TestLogger {
+@ExtendWith({TestLoggerExtension.class, RetryExtension.class})
+public class YarnFileStageTestS3ITCase {
- private static final String TEST_DATA_DIR = "tests-" + UUID.randomUUID();
-
- @ClassRule public static final TemporaryFolder TEMP_FOLDER = new
TemporaryFolder();
+ private static final Logger log =
LoggerFactory.getLogger(YarnFileStageTestS3ITCase.class);
- @Rule public final TemporaryFolder tempFolder = new TemporaryFolder();
-
- @Rule public final RetryRule retryRule = new RetryRule();
+ private static final String TEST_DATA_DIR = "tests-" + UUID.randomUUID();
/** Number of tests executed. */
private static int numRecursiveUploadTests = 0;
/** Will be updated by {@link #checkCredentialsAndSetup()} if the test is
not skipped. */
private static boolean skipTest = true;
- @BeforeClass
- public static void checkCredentialsAndSetup() throws IOException {
+ @BeforeAll
+ public static void checkCredentialsAndSetup(@TempDir File tempFolder)
throws IOException {
Review Comment:
```suggestion
static void checkCredentialsAndSetup(@TempDir File tempFolder) throws
IOException {
```
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerDriverTest.java:
##########
@@ -293,9 +304,22 @@ public void testTerminationWaitsOnContainerStopError()
throws Exception {
getDriver().terminate();
});
- assertThat(
- driverHasTerminatedFuture,
-
FlinkMatchers.willNotComplete(Duration.ofMillis(20L)));
+ assertThat(driverHasTerminatedFuture)
+ .satisfies(
+ new
Condition<CompletableFuture<?>>() {
Review Comment:
see above
##########
flink-yarn/src/test/java/org/apache/flink/yarn/entrypoint/YarnEntrypointUtilsTest.java:
##########
@@ -132,37 +130,36 @@ public void testDynamicParameterOverloading() throws
IOException {
Configuration overloadedConfiguration =
loadConfiguration(initialConfiguration, dynamicParameters);
- assertThat(
- overloadedConfiguration.get(JobManagerOptions.JVM_METASPACE),
- is(MemorySize.MAX_VALUE));
+
assertThat(overloadedConfiguration.get(JobManagerOptions.JVM_METASPACE))
+ .isEqualTo(MemorySize.MAX_VALUE);
}
@Nonnull
- private static Configuration loadConfiguration(Configuration
initialConfiguration)
- throws IOException {
+ private Configuration loadConfiguration(Configuration
initialConfiguration) throws IOException {
return loadConfiguration(initialConfiguration, new HashMap<>());
}
@Nonnull
- private static Configuration loadConfiguration(
+ private Configuration loadConfiguration(
Configuration initialConfiguration, Configuration
dynamicParameters)
throws IOException {
return loadConfiguration(initialConfiguration, dynamicParameters, new
HashMap<>());
}
@Nonnull
- private static Configuration loadConfiguration(
+ private Configuration loadConfiguration(
Configuration initialConfiguration, Map<String, String> env)
throws IOException {
return loadConfiguration(initialConfiguration, new Configuration(),
env);
}
@Nonnull
- private static Configuration loadConfiguration(
Review Comment:
why are these no longer static? The tempBaseDir could be static as well.
##########
flink-yarn/src/test/java/org/apache/flink/yarn/YarnResourceManagerDriverTest.java:
##########
@@ -242,9 +240,22 @@ public void testTerminationWaitsOnContainerStopSuccess()
throws Exception {
getDriver().terminate();
});
- assertThat(
- driverHasTerminatedFuture,
-
FlinkMatchers.willNotComplete(Duration.ofMillis(20L)));
+ assertThat(driverHasTerminatedFuture)
+ .satisfies(
+ new
Condition<CompletableFuture<?>>() {
+ @Override
+ public boolean
matches(CompletableFuture<?> item) {
+ try {
+ item.get(20,
TimeUnit.MILLISECONDS);
+ } catch (TimeoutException
timeoutException) {
+ return true;
+ } catch
(InterruptedException
+ |
ExecutionException e) {
+ return false;
+ }
+ return false;
+ }
+ });
Review Comment:
```suggestion
assertThatThrownBy(() ->
driverHasTerminatedFuture.get(20, TimeUnit.MILLISECONDS)
.isInstanceOf(TimeoutException.class);
```
--
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]
