YARN-10676. Improve code quality in TestTimelineAuthenticationFilterForV1. Contributed by Szilard Nemeth.

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/security/TestTimelineAuthenticationFilterForV1.java

@@ -18,8 +18,6 @@
 
 package org.apache.hadoop.yarn.server.timeline.security;
 
-import static org.junit.Assert.assertTrue;
-
 import java.io.File;
 import java.security.PrivilegedExceptionAction;
 import java.util.Arrays;
@@ -49,12 +47,16 @@ import org.apache.hadoop.yarn.server.applicationhistoryservice.ApplicationHistor
 import org.apache.hadoop.yarn.server.timeline.MemoryTimelineStore;
 import org.apache.hadoop.yarn.server.timeline.TimelineStore;
 import static org.apache.hadoop.yarn.conf.YarnConfiguration.TIMELINE_HTTP_AUTH_PREFIX;
+import static org.junit.Assert.fail;
+
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Test cases for authentication via TimelineAuthenticationFilter while
@@ -62,17 +64,20 @@ import org.junit.runners.Parameterized;
  */
 @RunWith(Parameterized.class)
 public class TestTimelineAuthenticationFilterForV1 {
+  private static final Logger LOG =
+      LoggerFactory.getLogger(TestTimelineAuthenticationFilterForV1.class);
 
   private static final String FOO_USER = "foo";
   private static final String BAR_USER = "bar";
   private static final String HTTP_USER = "HTTP";
+  private static final String PRINCIPAL = HTTP_USER + "/localhost";
 
   private static final File TEST_ROOT_DIR = new File(
       System.getProperty("test.build.dir", "target/test-dir"),
           TestTimelineAuthenticationFilterForV1.class.getName() + "-root");
-  private static File httpSpnegoKeytabFile = new File(
+  private static final File httpSpnegoKeytabFile = new File(
       KerberosTestUtils.getKeytabFile());
-  private static String httpSpnegoPrincipal =
+  private static final String httpSpnegoPrincipal =
       KerberosTestUtils.getServerPrincipal();
   private static final String BASEDIR =
       System.getProperty("test.build.dir", "target/test-dir") + "/"
@@ -100,16 +105,16 @@ public class TestTimelineAuthenticationFilterForV1 {
       testMiniKDC = new MiniKdc(MiniKdc.createConf(), TEST_ROOT_DIR);
       testMiniKDC.start();
       testMiniKDC.createPrincipal(
-          httpSpnegoKeytabFile, HTTP_USER + "/localhost");
+          httpSpnegoKeytabFile, PRINCIPAL);
     } catch (Exception e) {
-      assertTrue("Couldn't setup MiniKDC", false);
+      LOG.error("Failed to setup MiniKDC", e);
+      fail("Couldn't setup MiniKDC");
     }
 
     try {
       testTimelineServer = new ApplicationHistoryServer();
       conf = new Configuration(false);
-      conf.setStrings(TIMELINE_HTTP_AUTH_PREFIX + "type",
-          "kerberos");
+      conf.setStrings(TIMELINE_HTTP_AUTH_PREFIX + "type", "kerberos");
       conf.set(TIMELINE_HTTP_AUTH_PREFIX +
           KerberosAuthenticationHandler.PRINCIPAL, httpSpnegoPrincipal);
       conf.set(TIMELINE_HTTP_AUTH_PREFIX +
@@ -150,8 +155,8 @@ public class TestTimelineAuthenticationFilterForV1 {
       testTimelineServer.init(conf);
       testTimelineServer.start();
     } catch (Exception e) {
-      e.printStackTrace();
-      assertTrue("Couldn't setup TimelineServer", false);
+      LOG.error("Failed to setup TimelineServer", e);
+      fail("Couldn't setup TimelineServer");
     }
   }
 
@@ -181,7 +186,7 @@ public class TestTimelineAuthenticationFilterForV1 {
 
   @Test
   public void testPutTimelineEntities() throws Exception {
-    KerberosTestUtils.doAs(HTTP_USER + "/localhost", new Callable<Void>() {
+    KerberosTestUtils.doAs(PRINCIPAL, new Callable<Void>() {
       @Override
       public Void call() throws Exception {
         TimelineClient client = createTimelineClientForUGI();
@@ -191,11 +196,16 @@ public class TestTimelineAuthenticationFilterForV1 {
         entityToStore.setEntityId("entity1");
         entityToStore.setStartTime(0L);
         TimelinePutResponse putResponse = client.putEntities(entityToStore);
-        Assert.assertEquals(0, putResponse.getErrors().size());
+        if (putResponse.getErrors().size() > 0) {
+          LOG.error("putResponse errors: {}", putResponse.getErrors());
+        }
+        Assert.assertTrue("There were some errors in the putResponse",
+            putResponse.getErrors().isEmpty());
         TimelineEntity entityToRead =
             testTimelineServer.getTimelineStore().getEntity("entity1",
                 TestTimelineAuthenticationFilterForV1.class.getName(), null);
-        Assert.assertNotNull(entityToRead);
+        Assert.assertNotNull("Timeline entity should not be null",
+            entityToRead);
         return null;
       }
     });
@@ -203,7 +213,7 @@ public class TestTimelineAuthenticationFilterForV1 {
 
   @Test
   public void testPutDomains() throws Exception {
-    KerberosTestUtils.doAs(HTTP_USER + "/localhost", new Callable<Void>() {
+    KerberosTestUtils.doAs(PRINCIPAL, new Callable<Void>() {
       @Override
       public Void call() throws Exception {
         TimelineClient client = createTimelineClientForUGI();
@@ -216,7 +226,8 @@ public class TestTimelineAuthenticationFilterForV1 {
         TimelineDomain domainToRead =
             testTimelineServer.getTimelineStore().getDomain(
                 TestTimelineAuthenticationFilterForV1.class.getName());
-        Assert.assertNotNull(domainToRead);
+        Assert.assertNotNull("Timeline domain should not be null",
+            domainToRead);
         return null;
       }
     });
@@ -225,7 +236,7 @@ public class TestTimelineAuthenticationFilterForV1 {
   @Test
   public void testDelegationTokenOperations() throws Exception {
     TimelineClient httpUserClient =
-        KerberosTestUtils.doAs(HTTP_USER + "/localhost",
+        KerberosTestUtils.doAs(PRINCIPAL,
             new Callable<TimelineClient>() {
             @Override
             public TimelineClient call() throws Exception {
@@ -233,43 +244,51 @@ public class TestTimelineAuthenticationFilterForV1 {
             }
           });
     UserGroupInformation httpUser =
-        KerberosTestUtils.doAs(HTTP_USER + "/localhost",
+        KerberosTestUtils.doAs(PRINCIPAL,
             new Callable<UserGroupInformation>() {
             @Override
             public UserGroupInformation call() throws Exception {
               return UserGroupInformation.getCurrentUser();
             }
           });
+
     // Let HTTP user to get the delegation for itself
     Token<TimelineDelegationTokenIdentifier> token =
         httpUserClient.getDelegationToken(httpUser.getShortUserName());
-    Assert.assertNotNull(token);
+    Assert.assertNotNull("Delegation token should not be null", token);
     TimelineDelegationTokenIdentifier tDT = token.decodeIdentifier();
-    Assert.assertNotNull(tDT);
-    Assert.assertEquals(new Text(HTTP_USER), tDT.getOwner());
+    Assert.assertNotNull("Delegation token identifier should not be null",
+        tDT);
+    Assert.assertEquals("Owner of delegation token identifier does not match",
+        new Text(HTTP_USER), tDT.getOwner());
 
     // Renew token
-    Assert.assertFalse(token.getService().toString().isEmpty());
+    Assert.assertFalse("Service field of token should not be empty",
+        token.getService().toString().isEmpty());
     // Renew the token from the token service address
     long renewTime1 = httpUserClient.renewDelegationToken(token);
     Thread.sleep(100);
     token.setService(new Text());
-    Assert.assertTrue(token.getService().toString().isEmpty());
-    // If the token service address is not avaiable, it still can be renewed
+    Assert.assertTrue("Service field of token should be empty",
+        token.getService().toString().isEmpty());
+    // If the token service address is not available, it still can be renewed
     // from the configured address
     long renewTime2 = httpUserClient.renewDelegationToken(token);
-    Assert.assertTrue(renewTime1 < renewTime2);
+    Assert.assertTrue("renewTime2 should be later than renewTime1",
+        renewTime1 < renewTime2);
 
     // Cancel token
-    Assert.assertTrue(token.getService().toString().isEmpty());
-    // If the token service address is not avaiable, it still can be canceled
+    Assert.assertTrue("Service field of token should be empty",
+        token.getService().toString().isEmpty());
+    // If the token service address is not available, it still can be canceled
     // from the configured address
     httpUserClient.cancelDelegationToken(token);
     // Renew should not be successful because the token is canceled
     try {
       httpUserClient.renewDelegationToken(token);
-      Assert.fail();
+      Assert.fail("Renew of delegation token should not be successful");
     } catch (Exception e) {
+      LOG.info("Exception while renewing delegation token", e);
       Assert.assertTrue(e.getMessage().contains(
             "Renewal request for unknown token"));
     }
@@ -280,33 +299,39 @@ public class TestTimelineAuthenticationFilterForV1 {
     TimelineClient fooUserClient = fooUgi.doAs(
         new PrivilegedExceptionAction<TimelineClient>() {
           @Override
-          public TimelineClient run() throws Exception {
+          public TimelineClient run() {
             return createTimelineClientForUGI();
           }
         });
     token = fooUserClient.getDelegationToken(httpUser.getShortUserName());
-    Assert.assertNotNull(token);
+    Assert.assertNotNull("Delegation token should not be null", token);
     tDT = token.decodeIdentifier();
-    Assert.assertNotNull(tDT);
-    Assert.assertEquals(new Text(FOO_USER), tDT.getOwner());
-    Assert.assertEquals(new Text(HTTP_USER), tDT.getRealUser());
+    Assert.assertNotNull("Delegation token identifier should not be null",
+        tDT);
+    Assert.assertEquals("Owner of delegation token is not the expected",
+        new Text(FOO_USER), tDT.getOwner());
+    Assert.assertEquals("Real user of delegation token is not the expected",
+        new Text(HTTP_USER), tDT.getRealUser());
 
     // Renew token as the renewer
     final Token<TimelineDelegationTokenIdentifier> tokenToRenew = token;
     renewTime1 = httpUserClient.renewDelegationToken(tokenToRenew);
     renewTime2 = httpUserClient.renewDelegationToken(tokenToRenew);
-    Assert.assertTrue(renewTime1 < renewTime2);
+    Assert.assertTrue("renewTime2 should be later than renewTime1",
+        renewTime1 < renewTime2);
 
     // Cancel token
-    Assert.assertFalse(tokenToRenew.getService().toString().isEmpty());
+    Assert.assertFalse("Service field of token should not be empty",
+        tokenToRenew.getService().toString().isEmpty());
     // Cancel the token from the token service address
     fooUserClient.cancelDelegationToken(tokenToRenew);
 
     // Renew should not be successful because the token is canceled
     try {
       httpUserClient.renewDelegationToken(tokenToRenew);
-      Assert.fail();
+      Assert.fail("Renew of delegation token should not be successful");
     } catch (Exception e) {
+      LOG.info("Exception while renewing delegation token", e);
       Assert.assertTrue(
           e.getMessage().contains("Renewal request for unknown token"));
     }
@@ -324,8 +349,9 @@ public class TestTimelineAuthenticationFilterForV1 {
 
     try {
       barUserClient.getDelegationToken(httpUser.getShortUserName());
-      Assert.fail();
+      Assert.fail("Retrieval of delegation token should not be successful");
     } catch (Exception e) {
+      LOG.info("Exception while retrieving delegation token", e);
       Assert.assertTrue(e.getCause() instanceof AuthorizationException ||
           e.getCause() instanceof AuthenticationException);
     }