goiri commented on code in PR #4396:
URL: https://github.com/apache/hadoop/pull/4396#discussion_r891808228


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestFederationClientInterceptor.java:
##########
@@ -208,11 +219,11 @@ public void testSubmitApplication()
   }
 
   private SubmitApplicationRequest mockSubmitApplicationRequest(
-      ApplicationId appId) {
+      ApplicationId appId, int priority) {

Review Comment:
   Can we add one with APP_PRIORITY_ZERO as default instead of having to modify 
every call?



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestFederationClientInterceptor.java:
##########
@@ -898,4 +909,141 @@ public void testGetResourceTypeInfoRequest() throws 
Exception {
         
interceptor.getResourceTypeInfo(GetAllResourceTypeInfoRequest.newInstance());
     Assert.assertEquals(2, response.getResourceTypeInfo().size());
   }
+
+  @Test
+  public void testFailApplicationAttempt() throws Exception {
+    LOG.info("Test FederationClientInterceptor : Fail Application Attempt 
request.");
+
+    // null request
+    LambdaTestUtils.intercept(YarnException.class, "Missing 
failApplicationAttempt request " +
+        "or applicationId or applicationAttemptId information.",
+        () -> interceptor.failApplicationAttempt(null));
+
+    // normal request
+    ApplicationId appId =

Review Comment:
   Single line should fit within limit.



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestFederationClientInterceptor.java:
##########
@@ -898,4 +909,141 @@ public void testGetResourceTypeInfoRequest() throws 
Exception {
         
interceptor.getResourceTypeInfo(GetAllResourceTypeInfoRequest.newInstance());
     Assert.assertEquals(2, response.getResourceTypeInfo().size());
   }
+
+  @Test
+  public void testFailApplicationAttempt() throws Exception {
+    LOG.info("Test FederationClientInterceptor : Fail Application Attempt 
request.");
+
+    // null request
+    LambdaTestUtils.intercept(YarnException.class, "Missing 
failApplicationAttempt request " +
+        "or applicationId or applicationAttemptId information.",
+        () -> interceptor.failApplicationAttempt(null));
+
+    // normal request
+    ApplicationId appId =
+        ApplicationId.newInstance(System.currentTimeMillis(), 1);
+    SubmitApplicationRequest request = mockSubmitApplicationRequest(appId, 
APP_PRIORITY_ZERO);
+
+    // Submit the application
+    SubmitApplicationResponse response = 
interceptor.submitApplication(request);
+
+    Assert.assertNotNull(response);
+    Assert.assertNotNull(stateStoreUtil.queryApplicationHomeSC(appId));
+
+    // Call GetApplicationAttempts
+    GetApplicationAttemptsRequest attemptsRequest =
+        GetApplicationAttemptsRequest.newInstance(appId);
+    GetApplicationAttemptsResponse attemptsResponse =
+        interceptor.getApplicationAttempts(attemptsRequest);
+
+    // Wait for app to start
+    while(attemptsResponse.getApplicationAttemptList().size() == 0) {
+      attemptsResponse =
+          interceptor.getApplicationAttempts(attemptsRequest);
+    }
+    Assert.assertNotNull(attemptsResponse);
+
+    ApplicationAttemptId attemptId = 
attemptsResponse.getApplicationAttemptList().
+        get(0).getApplicationAttemptId();
+
+    FailApplicationAttemptRequest requestFailAppAttempt =
+        FailApplicationAttemptRequest.newInstance(attemptId);
+    FailApplicationAttemptResponse responseFailAppAttempt =
+        interceptor.failApplicationAttempt(requestFailAppAttempt);
+
+    Assert.assertNotNull(responseFailAppAttempt);
+  }
+
+  @Test
+  public void testUpdateApplicationPriority() throws Exception {
+    LOG.info("Test FederationClientInterceptor : Update Application Priority 
request.");
+
+    // null request
+    LambdaTestUtils.intercept(YarnException.class, "Missing 
updateApplicationPriority request " +
+        "or applicationId or applicationPriority information.",
+        () -> interceptor.updateApplicationPriority(null));
+
+    // normal request
+    ApplicationId appId =
+        ApplicationId.newInstance(System.currentTimeMillis(), 1);
+    SubmitApplicationRequest request = mockSubmitApplicationRequest(appId, 
APP_PRIORITY_ZERO);
+
+    // Submit the application
+    SubmitApplicationResponse response = 
interceptor.submitApplication(request);
+
+    Assert.assertNotNull(response);
+    Assert.assertNotNull(stateStoreUtil.queryApplicationHomeSC(appId));
+
+    // Call GetApplicationAttempts
+    GetApplicationAttemptsRequest attemptsRequest =
+        GetApplicationAttemptsRequest.newInstance(appId);
+    GetApplicationAttemptsResponse attemptsResponse =
+        interceptor.getApplicationAttempts(attemptsRequest);
+
+    // Wait for app to start
+    while(attemptsResponse.getApplicationAttemptList().size() == 0) {
+      attemptsResponse =
+          interceptor.getApplicationAttempts(attemptsRequest);
+    }
+    Assert.assertNotNull(attemptsResponse);
+
+    Priority priority = Priority.newInstance(20);
+    UpdateApplicationPriorityRequest requestUpdateAppPriority =
+        UpdateApplicationPriorityRequest.newInstance(appId, priority);
+    UpdateApplicationPriorityResponse responseAppPriority =
+        interceptor.updateApplicationPriority(requestUpdateAppPriority);
+
+    Assert.assertNotNull(responseAppPriority);
+    Assert.assertEquals(20,
+        responseAppPriority.getApplicationPriority().getPriority());
+  }
+
+  @Test
+  public void testUpdateApplicationTimeouts() throws Exception {
+    LOG.info("Test FederationClientInterceptor : Update Application Timeouts 
request.");
+
+    // null request
+    LambdaTestUtils.intercept(YarnException.class, "Missing 
updateApplicationTimeouts request " +
+        "or applicationId or applicationTimeouts information.",
+        () -> interceptor.updateApplicationTimeouts(null));
+
+    // normal request
+    ApplicationId appId =
+        ApplicationId.newInstance(System.currentTimeMillis(), 1);
+    SubmitApplicationRequest request = mockSubmitApplicationRequest(appId, 
APP_PRIORITY_ZERO);
+
+    // Submit the application
+    SubmitApplicationResponse response = 
interceptor.submitApplication(request);
+
+    Assert.assertNotNull(response);
+    Assert.assertNotNull(stateStoreUtil.queryApplicationHomeSC(appId));
+
+    // Call GetApplicationAttempts
+    GetApplicationAttemptsRequest attemptsRequest =
+        GetApplicationAttemptsRequest.newInstance(appId);
+    GetApplicationAttemptsResponse attemptsResponse =
+        interceptor.getApplicationAttempts(attemptsRequest);
+
+    // Wait for app to start
+    while(attemptsResponse.getApplicationAttemptList().size() == 0) {
+      attemptsResponse =
+          interceptor.getApplicationAttempts(attemptsRequest);
+    }
+    Assert.assertNotNull(attemptsResponse);
+
+    String formatISO8601 =
+            Times.formatISO8601(System.currentTimeMillis() + 5 * 1000);

Review Comment:
   Spacing



##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-router/src/test/java/org/apache/hadoop/yarn/server/router/clientrm/TestFederationClientInterceptor.java:
##########
@@ -898,4 +909,141 @@ public void testGetResourceTypeInfoRequest() throws 
Exception {
         
interceptor.getResourceTypeInfo(GetAllResourceTypeInfoRequest.newInstance());
     Assert.assertEquals(2, response.getResourceTypeInfo().size());
   }
+
+  @Test
+  public void testFailApplicationAttempt() throws Exception {
+    LOG.info("Test FederationClientInterceptor : Fail Application Attempt 
request.");
+
+    // null request
+    LambdaTestUtils.intercept(YarnException.class, "Missing 
failApplicationAttempt request " +
+        "or applicationId or applicationAttemptId information.",
+        () -> interceptor.failApplicationAttempt(null));
+
+    // normal request
+    ApplicationId appId =
+        ApplicationId.newInstance(System.currentTimeMillis(), 1);
+    SubmitApplicationRequest request = mockSubmitApplicationRequest(appId, 
APP_PRIORITY_ZERO);
+
+    // Submit the application
+    SubmitApplicationResponse response = 
interceptor.submitApplication(request);
+
+    Assert.assertNotNull(response);
+    Assert.assertNotNull(stateStoreUtil.queryApplicationHomeSC(appId));
+
+    // Call GetApplicationAttempts
+    GetApplicationAttemptsRequest attemptsRequest =
+        GetApplicationAttemptsRequest.newInstance(appId);
+    GetApplicationAttemptsResponse attemptsResponse =
+        interceptor.getApplicationAttempts(attemptsRequest);
+
+    // Wait for app to start
+    while(attemptsResponse.getApplicationAttemptList().size() == 0) {

Review Comment:
   To avoid an active wait, we should do a waitFor() or similar.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to