YARN-7042. Clean up unit tests after YARN-6610. (Daniel Templeton via wangda)

Change-Id: I8e40f704b6fcdd5b14faa9548a27986501044fa1

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/resource/DominantResourceCalculator.java

@@ -126,7 +126,7 @@ public int compare(Resource clusterResource, Resource lhs, Resource rhs,
         diff = max[0] - max[1];
       } else if (clusterRes.length == 2) {
         // Special case to handle the common scenario of only CPU and memory
-        // so the we can optimize for performance
+        // so that we can optimize for performance
         diff = calculateSharesForMandatoryResources(clusterRes, lhs, rhs,
             lhsShares, rhsShares);
       } else {

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/util/resource/TestResourceCalculator.java

@@ -24,7 +24,7 @@
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.yarn.api.records.Resource;
 import org.apache.hadoop.yarn.conf.YarnConfiguration;
-import org.junit.BeforeClass;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -44,13 +44,18 @@ public static Collection<ResourceCalculator[]> getParameters() {
         { new DominantResourceCalculator() } });
   }
 
-  @BeforeClass
+  @Before
-  public static void setup() {
+  public void setupNoExtraResource() {
+    // This has to run before each test because we don't know when
+    // setupExtraResource() might be called
+    ResourceUtils.resetResourceTypes(new Configuration());
+  }
+
+  private static void setupExtraResource() {
     Configuration conf = new Configuration();
 
     conf.set(YarnConfiguration.RESOURCE_TYPES, "test");
     ResourceUtils.resetResourceTypes(conf);
-    ResourceUtils.getResourceTypes();
   }
 
   public TestResourceCalculator(ResourceCalculator rs) {
@@ -86,9 +91,15 @@ public void testFitsIn() {
     }
   }
 
-  private Resource newResource(long memory, int cpu, int test) {
+  private Resource newResource(long memory, int cpu) {
     Resource res = Resource.newInstance(memory, cpu);
 
+    return res;
+  }
+
+  private Resource newResource(long memory, int cpu, int test) {
+    Resource res = newResource(memory, cpu);
+
     res.setResourceValue("test", test);
 
     return res;
@@ -123,28 +134,48 @@ private void assertComparison(Resource cluster, Resource res1, Resource res2,
   }
 
   @Test
-  public void testCompare2() {
+  public void testCompareWithOnlyMandatory() {
-    // Keep cluster resources even so that the numbers are easy to understand
+    // This test is necessary because there are optimizations that are only
-    Resource cluster = Resource.newInstance(4, 4);
+    // triggered when only the mandatory resources are configured.
 
-    assertComparison(cluster, Resource.newInstance(1, 1),
+    // Keep cluster resources even so that the numbers are easy to understand
-        Resource.newInstance(1, 1), 0);
+    Resource cluster = newResource(4, 4);
-    assertComparison(cluster, Resource.newInstance(0, 0),
+
-        Resource.newInstance(0, 0), 0);
+    assertComparison(cluster, newResource(1, 1), newResource(1, 1), 0);
-    assertComparison(cluster, Resource.newInstance(2, 2),
+    assertComparison(cluster, newResource(0, 0), newResource(0, 0), 0);
-        Resource.newInstance(1, 1), 1);
+    assertComparison(cluster, newResource(2, 2), newResource(1, 1), 1);
-    assertComparison(cluster, Resource.newInstance(2, 2),
+    assertComparison(cluster, newResource(2, 2), newResource(0, 0), 1);
-        Resource.newInstance(0, 0), 1);
 
     if (resourceCalculator instanceof DefaultResourceCalculator) {
-      testCompareDefault2(cluster);
+      testCompareDefaultWithOnlyMandatory(cluster);
     } else if (resourceCalculator instanceof DominantResourceCalculator) {
-      testCompareDominant2(cluster);
+      testCompareDominantWithOnlyMandatory(cluster);
     }
   }
 
+  private void testCompareDefaultWithOnlyMandatory(Resource cluster) {
+    assertComparison(cluster, newResource(1, 1), newResource(1, 1), 0);
+    assertComparison(cluster, newResource(1, 2), newResource(1, 1), 0);
+    assertComparison(cluster, newResource(1, 1), newResource(1, 0), 0);
+    assertComparison(cluster, newResource(2, 1), newResource(1, 1), 1);
+    assertComparison(cluster, newResource(2, 1), newResource(1, 2), 1);
+    assertComparison(cluster, newResource(2, 1), newResource(1, 0), 1);
+  }
+
+  private void testCompareDominantWithOnlyMandatory(Resource cluster) {
+    assertComparison(cluster, newResource(2, 1), newResource(2, 1), 0);
+    assertComparison(cluster, newResource(2, 1), newResource(1, 2), 0);
+    assertComparison(cluster, newResource(2, 1), newResource(1, 1), 1);
+    assertComparison(cluster, newResource(2, 2), newResource(2, 1), 1);
+    assertComparison(cluster, newResource(2, 2), newResource(1, 2), 1);
+    assertComparison(cluster, newResource(3, 1), newResource(3, 0), 1);
+  }
+
   @Test
   public void testCompare() {
+    // Test with 3 resources
+    setupExtraResource();
+
     // Keep cluster resources even so that the numbers are easy to understand
     Resource cluster = newResource(4L, 4, 4);
 
@@ -160,36 +191,6 @@ public void testCompare() {
     }
   }
 
-  private void testCompareDefault2(Resource cluster) {
-    assertComparison(cluster, Resource.newInstance(1, 1),
-        Resource.newInstance(1, 1), 0);
-    assertComparison(cluster, Resource.newInstance(1, 2),
-        Resource.newInstance(1, 1), 0);
-    assertComparison(cluster, Resource.newInstance(1, 1),
-        Resource.newInstance(1, 0), 0);
-    assertComparison(cluster, Resource.newInstance(2, 1),
-        Resource.newInstance(1, 1), 1);
-    assertComparison(cluster, Resource.newInstance(2, 1),
-        Resource.newInstance(1, 2), 1);
-    assertComparison(cluster, Resource.newInstance(2, 1),
-        Resource.newInstance(1, 0), 1);
-  }
-
-  private void testCompareDominant2(Resource cluster) {
-    assertComparison(cluster, Resource.newInstance(2, 1),
-        Resource.newInstance(2, 1), 0);
-    assertComparison(cluster, Resource.newInstance(2, 1),
-        Resource.newInstance(1, 2), 0);
-    assertComparison(cluster, Resource.newInstance(2, 1),
-        Resource.newInstance(1, 1), 1);
-    assertComparison(cluster, Resource.newInstance(2, 2),
-        Resource.newInstance(2, 1), 1);
-    assertComparison(cluster, Resource.newInstance(2, 2),
-        Resource.newInstance(1, 2), 1);
-    assertComparison(cluster, Resource.newInstance(3, 1),
-        Resource.newInstance(3, 0), 1);
-  }
-
   private void testCompareDefault(Resource cluster) {
     assertComparison(cluster, newResource(1, 1, 2), newResource(1, 1, 1), 0);
     assertComparison(cluster, newResource(1, 2, 1), newResource(1, 1, 1), 0);