RocMarshal commented on code in PR #18986:
URL: https://github.com/apache/flink/pull/18986#discussion_r862828232
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/CliFrontendRunWithYarnTest.java:
##########
@@ -43,22 +44,20 @@
*
* @see org.apache.flink.client.cli.CliFrontendRunTest
*/
-public class CliFrontendRunWithYarnTest extends CliFrontendTestBase {
-
- @Rule public TemporaryFolder tmp = new TemporaryFolder();
+class CliFrontendRunWithYarnTest extends CliFrontendTestBase {
Review Comment:
Good catch~
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNHighAvailabilityITCase.java:
##########
@@ -90,19 +92,13 @@
import java.util.stream.Collectors;
import static org.apache.flink.util.Preconditions.checkState;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.empty;
-import static org.hamcrest.Matchers.instanceOf;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assume.assumeTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
/** Tests that verify correct HA behavior. */
-public class YARNHighAvailabilityITCase extends YarnTestBase {
+class YARNHighAvailabilityITCase extends YarnTestBase {
- @ClassRule public static final TemporaryFolder FOLDER = new
TemporaryFolder();
+ private static final Logger log =
LoggerFactory.getLogger(YARNHighAvailabilityITCase.class);
Review Comment:
Done~.
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnTestBase.java:
##########
@@ -1070,25 +1059,18 @@ protected void runWithArgs(
// this lets the test fail.
throw new RuntimeException("Runner failed",
runner.getRunnerError());
}
- Assert.assertTrue(
- "During the timeout period of "
- + startTimeoutSeconds
- + " seconds the "
- + "expected string \""
- + terminateAfterString
- + "\" did not show up.",
Review Comment:
@zentol Added.
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/UtilsTest.java:
##########
@@ -55,36 +52,36 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
/** Tests for various utilities. */
-public class UtilsTest extends TestLogger {
+class UtilsTest {
private static final Logger LOG = LoggerFactory.getLogger(UtilsTest.class);
- @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder();
+ @TempDir File temporaryFolder;
@Test
- public void testUberjarLocator() {
+ void testUberjarLocator() {
File dir = TestUtils.findFile("..", new
TestUtils.RootDirFilenameFilter());
- Assert.assertNotNull(dir);
- Assert.assertTrue(dir.getName().endsWith(".jar"));
+ assertThat(dir).isNotNull();
+ assertThat(dir.getName()).endsWith(".jar");
dir = dir.getParentFile().getParentFile(); // from uberjar to lib to
root
- Assert.assertTrue(dir.exists());
- Assert.assertTrue(dir.isDirectory());
- List<String> files = Arrays.asList(dir.list());
- Assert.assertTrue(files.contains("lib"));
- Assert.assertTrue(files.contains("bin"));
- Assert.assertTrue(files.contains("conf"));
+ assertThat(dir.exists()).isTrue();
+ assertThat(dir.isDirectory()).isTrue();
+ List<String> files = Arrays.asList(Objects.requireNonNull(dir.list()));
+ assertThat(files).contains("lib");
+ assertThat(files).contains("bin");
+ assertThat(files).contains("conf");
Review Comment:
nice comments.
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnTestBase.java:
##########
@@ -473,11 +472,8 @@ private static void writeHDFSSiteConfigXML(Configuration
coreSite, File targetFo
public static void ensureNoProhibitedStringInLogFiles(
final String[] prohibited, final Pattern[] whitelisted) {
File cwd = new File("target/" +
YARN_CONFIGURATION.get(TEST_CLUSTER_NAME_KEY));
- Assert.assertTrue(
- "Expecting directory " + cwd.getAbsolutePath() + " to exist",
cwd.exists());
- Assert.assertTrue(
- "Expecting directory " + cwd.getAbsolutePath() + " to be a
directory",
- cwd.isDirectory());
+ assertThat(cwd).exists();
+ assertThat(cwd).isDirectory();
Review Comment:
@zentol Yes~. Thank you for recommending me the assertJ automatic migration
tool last time. Happily, I also found an idea IDE plugin called
Assertions2AssertJ. The plugin could remind developers of code-lines need to
migrate by marking highlights, which would be a good supplement to automation
tools.
--
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]
