GJL commented on a change in pull request #6917: [FLINK-10665] [tests] Port
YARNSessionFIFOITCase#testJavaAPI to new c…
URL: https://github.com/apache/flink/pull/6917#discussion_r248679390
##########
File path:
flink-yarn-tests/src/test/java/org/apache/flink/yarn/YARNSessionFIFOITCase.java
##########
@@ -276,19 +280,21 @@ public void testfullAlloc() throws IOException {
*/
@Test
public void testJavaAPI() throws Exception {
Review comment:
How about we delete `testJavaAPI`? This test only starts a session cluster
and tests whether the `ClusterConnectionInfo` and the URL of the web UI are not
null. I don't see how this is related to Yarn's FIFO scheduler, i.e., the test
does not belong here. Retrieving the `ClusterConnectionInfo` and URL of the web
ui is already covered by other tests [1][2]. Lastly, the ported version in this
PR does not have the same assertions as before. Let me know what you think,
@TisonKun.
[1]
https://github.com/apache/flink/blob/0a54983ca0207444477cfe991dbee1b611496204/flink-clients/src/test/java/org/apache/flink/client/cli/DefaultCLITest.java#L71
[2]
https://github.com/apache/flink/blob/0a54983ca0207444477cfe991dbee1b611496204/flink-clients/src/main/java/org/apache/flink/client/program/rest/RestClusterClient.java
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
