[GitHub] GJL commented on a change in pull request #6917: [FLINK-10665] [tests] Port YARNSessionFIFOITCase#testJavaAPI to new c…

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services