zentol commented on code in PR #18986:
URL: https://github.com/apache/flink/pull/18986#discussion_r863668139
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnTestBase.java:
##########
@@ -173,10 +172,10 @@ public abstract class YarnTestBase extends TestLogger {
};
// Temp directory which is deleted after the unit test.
- @ClassRule public static TemporaryFolder tmp = new TemporaryFolder();
+ @TempDir public static File tmp;
Review Comment:
```suggestion
@TempDir protected static File tmp;
```
?
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnTestBase.java:
##########
@@ -869,7 +856,7 @@ private static void setMiniDFSCluster(File
targetTestClassesFolder) throws Excep
}
/** Default @BeforeClass impl. Overwrite this for passing a different
configuration */
- @BeforeClass
+ @BeforeAll
public static void setup() throws Exception {
Review Comment:
```suggestion
static void setup() throws Exception {
```
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnTestBase.java:
##########
@@ -276,7 +275,7 @@ public void setupYarnClient() {
}
/** Sleep a bit between the tests (we are re-using the YARN cluster for
the tests). */
- @After
+ @AfterEach
public void shutdownYarnClient() {
Review Comment:
```suggestion
void shutdownYarnClient() {
```
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnTestBase.java:
##########
@@ -264,7 +263,7 @@ public static void populateYarnSecureConfigurations(
conf.set("hadoop.security.auth_to_local", "RULE:[1:$1] RULE:[2:$1]");
}
- @Before
+ @BeforeEach
public void setupYarnClient() {
Review Comment:
```suggestion
void setupYarnClient() {
```
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNSessionCapacitySchedulerITCase.java:
##########
@@ -445,39 +443,45 @@ private static Map<String, String> getFlinkConfig(final
String host, final int p
* With an error message, we can help users identifying the issue)
*/
@Test
- public void testNonexistingQueueWARNmessage() throws Exception {
+ void testNonexistingQueueWARNmessage() throws Exception {
runTest(
() -> {
LOG.info("Starting testNonexistingQueueWARNmessage()");
- try {
- runWithArgs(
- new String[] {
- "-j",
- flinkUberjar.getAbsolutePath(),
- "-t",
- flinkLibFolder.getAbsolutePath(),
- "-jm",
- "768m",
- "-tm",
- "1024m",
- "-qu",
- "doesntExist"
- },
- "to unknown queue: doesntExist",
- null,
- RunTypes.YARN_SESSION,
- 1);
- } catch (Exception e) {
- assertTrue(
- ExceptionUtils.findThrowableWithMessage(
- e, "to unknown queue:
doesntExist")
- .isPresent());
- }
- assertThat(
- yarTestLoggerResource.getMessages(),
- hasItem(
- containsString(
- "The specified queue 'doesntExist'
does not exist. Available queues")));
+ assertThatThrownBy(
+ () ->
+ runWithArgs(
+ new String[] {
+ "-j",
+
flinkUberjar.getAbsolutePath(),
+ "-t",
+
flinkLibFolder.getAbsolutePath(),
+ "-jm",
+ "768m",
+ "-tm",
+ "1024m",
+ "-qu",
+ "doesntExist"
+ },
+ "to unknown queue:
doesntExist",
+ null,
+ RunTypes.YARN_SESSION,
+ 1))
+ .isInstanceOf(Exception.class)
+ .satisfies(
+ new Condition<Throwable>() {
+ @Override
+ public boolean matches(Throwable
value) {
+ return
ExceptionUtils.findThrowableWithMessage(
+ value, "to unknown
queue: doesntExist")
+ .isPresent();
+ }
+ });
Review Comment:
```suggestion
.satisfies(anyCauseMatches("to unknown queue:
doesntExist"));
```
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNSessionCapacitySchedulerITCase.java:
##########
@@ -445,39 +443,45 @@ private static Map<String, String> getFlinkConfig(final
String host, final int p
* With an error message, we can help users identifying the issue)
*/
@Test
- public void testNonexistingQueueWARNmessage() throws Exception {
+ void testNonexistingQueueWARNmessage() throws Exception {
runTest(
() -> {
LOG.info("Starting testNonexistingQueueWARNmessage()");
- try {
- runWithArgs(
- new String[] {
- "-j",
- flinkUberjar.getAbsolutePath(),
- "-t",
- flinkLibFolder.getAbsolutePath(),
- "-jm",
- "768m",
- "-tm",
- "1024m",
- "-qu",
- "doesntExist"
- },
- "to unknown queue: doesntExist",
- null,
- RunTypes.YARN_SESSION,
- 1);
- } catch (Exception e) {
- assertTrue(
- ExceptionUtils.findThrowableWithMessage(
- e, "to unknown queue:
doesntExist")
- .isPresent());
- }
- assertThat(
- yarTestLoggerResource.getMessages(),
- hasItem(
- containsString(
- "The specified queue 'doesntExist'
does not exist. Available queues")));
+ assertThatThrownBy(
+ () ->
+ runWithArgs(
+ new String[] {
+ "-j",
+
flinkUberjar.getAbsolutePath(),
+ "-t",
+
flinkLibFolder.getAbsolutePath(),
+ "-jm",
+ "768m",
+ "-tm",
+ "1024m",
+ "-qu",
+ "doesntExist"
+ },
+ "to unknown queue:
doesntExist",
+ null,
+ RunTypes.YARN_SESSION,
+ 1))
+ .isInstanceOf(Exception.class)
+ .satisfies(
+ new Condition<Throwable>() {
+ @Override
+ public boolean matches(Throwable
value) {
+ return
ExceptionUtils.findThrowableWithMessage(
+ value, "to unknown
queue: doesntExist")
+ .isPresent();
+ }
+ });
+
+ assertThat(yarLoggerAuditingExtension.getMessages())
+ .anyMatch(
+ s ->
+ s.contains(
+ "The specified queue
'doesntExist' does not exist. Available queues"));
Review Comment:
```suggestion
.anySatisfies(
s ->
assertThat(s).contains(
"The specified queue
'doesntExist' does not exist. Available queues"));
```
This will give a better error message.
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNHighAvailabilityITCase.java:
##########
@@ -154,16 +146,19 @@ private void initJobGraph() throws IOException {
* Tests that Yarn will restart a killed {@link
YarnSessionClusterEntrypoint} which will then
* resume a persisted {@link JobGraph}.
*/
- @Test(timeout = 1_000 * 60 * 30)
- public void testKillYarnSessionClusterEntrypoint() throws Exception {
+ @Timeout(value = 60 * 30)
Review Comment:
```suggestion
@Timeout(value = 30, unit = TimeUnit.MINUTES)
```
This is what I had in mind.
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnTestBase.java:
##########
@@ -1176,7 +1164,7 @@ public Throwable getRunnerError() {
// -------------------------- Tear down -------------------------- //
- @AfterClass
+ @AfterAll
public static void teardown() throws Exception {
Review Comment:
```suggestion
static void teardown() throws Exception {
```
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNSessionCapacitySchedulerITCase.java:
##########
@@ -111,15 +108,15 @@ public class YARNSessionCapacitySchedulerITCase extends
YarnTestBase {
/** Toggles checking for prohibited strings in logs after the test has
run. */
private boolean checkForProhibitedLogContents = true;
- @Rule
- public final TestLoggerResource cliTestLoggerResource =
- new TestLoggerResource(CliFrontend.class, Level.INFO);
+ @RegisterExtension
+ private final LoggerAuditingExtension cliLoggerAuditingExtension =
+ new LoggerAuditingExtension(CliFrontend.class, Level.INFO);
- @Rule
- public final TestLoggerResource yarTestLoggerResource =
- new TestLoggerResource(YarnClusterDescriptor.class, Level.WARN);
+ @RegisterExtension
+ private final LoggerAuditingExtension yarLoggerAuditingExtension =
+ new LoggerAuditingExtension(YarnClusterDescriptor.class,
Level.WARN);
- @BeforeClass
+ @BeforeAll
public static void setup() throws Exception {
Review Comment:
```suggestion
static void setup() throws Exception {
```
##########
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YarnTestBase.java:
##########
@@ -850,15 +839,13 @@ private static void start(
} catch (Exception ex) {
ex.printStackTrace();
LOG.error("setup failure", ex);
- Assert.fail();
Review Comment:
this needs to stay
--
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]
