From 5dabc227657c29ea21528674d9ca1aa0e4c2bdd4 Mon Sep 17 00:00:00 2001 From: Sanford Ryza Date: Tue, 3 Dec 2013 23:21:30 +0000 Subject: [PATCH] YARN-1332. In TestAMRMClient, replace assertTrue with assertEquals where possible (Sebastian Wong via Sandy Ryza) git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1547640 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-yarn-project/CHANGES.txt | 3 + .../yarn/client/api/impl/TestAMRMClient.java | 122 +++++++++--------- 2 files changed, 64 insertions(+), 61 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index adbcdd00500..6838b8e9fa6 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -132,6 +132,9 @@ Release 2.4.0 - UNRELEASED YARN-1318. Promoted AdminService to an Always-On service and merged it into RMHAProtocolService. (Karthik Kambatla via vinodkv) + YARN-1332. In TestAMRMClient, replace assertTrue with assertEquals where + possible (Sebastian Wong via Sandy Ryza) + OPTIMIZATIONS BUG FIXES diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClient.java index 1f7565be181..08e71c1cb84 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClient.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClient.java @@ -248,7 +248,7 @@ public void testAMRMClientMatchingFit() throws YarnException, IOException { matches = amClient.getMatchingRequests(priority, node, testCapability1); verifyMatches(matches, 1); storedRequest = matches.get(0).iterator().next(); - assertTrue(storedContainer1 == storedRequest); + assertEquals(storedContainer1, storedRequest); amClient.removeContainerRequest(storedContainer1); // exact matching with order maintained @@ -259,9 +259,9 @@ public void testAMRMClientMatchingFit() throws YarnException, IOException { int i = 0; for(ContainerRequest storedRequest1 : matches.get(0)) { if(i++ == 0) { - assertTrue(storedContainer4 == storedRequest1); + assertEquals(storedContainer4, storedRequest1); } else { - assertTrue(storedContainer6 == storedRequest1); + assertEquals(storedContainer6, storedRequest1); } } amClient.removeContainerRequest(storedContainer6); @@ -276,7 +276,7 @@ public void testAMRMClientMatchingFit() throws YarnException, IOException { assert(matches.size() == 2); // verify non-fitting containers are not returned and fitting ones are for(Collection testSet : matches) { - assertTrue(testSet.size() == 1); + assertEquals(1, testSet.size()); ContainerRequest testRequest = testSet.iterator().next(); assertTrue(testRequest != storedContainer4); assertTrue(testRequest != storedContainer5); @@ -310,8 +310,8 @@ public void testAMRMClientMatchingFit() throws YarnException, IOException { private void verifyMatches( List> matches, int matchSize) { - assertTrue(matches.size() == 1); - assertTrue(matches.get(0).size() == matchSize); + assertEquals(1, matches.size()); + assertEquals(matches.get(0).size(), matchSize); } @Test (timeout=60000) @@ -337,12 +337,12 @@ public void testAMRMClientMatchingFitInferredRack() throws YarnException, IOExce matches = amClient.getMatchingRequests(priority, node, capability); verifyMatches(matches, 1); storedRequest = matches.get(0).iterator().next(); - assertTrue(storedContainer1 == storedRequest); + assertEquals(storedContainer1, storedRequest); // inferred match rack matches = amClient.getMatchingRequests(priority, rack, capability); verifyMatches(matches, 1); storedRequest = matches.get(0).iterator().next(); - assertTrue(storedContainer1 == storedRequest); + assertEquals(storedContainer1, storedRequest); // inferred rack match no longer valid after request is removed amClient.removeContainerRequest(storedContainer1); @@ -387,10 +387,10 @@ public void testAMRMClientMatchStorage() throws YarnException, IOException { // test addition and storage int containersRequestedAny = amClient.remoteRequestsTable.get(priority) .get(ResourceRequest.ANY).get(capability).remoteRequest.getNumContainers(); - assertTrue(containersRequestedAny == 2); + assertEquals(2, containersRequestedAny); containersRequestedAny = amClient.remoteRequestsTable.get(priority1) .get(ResourceRequest.ANY).get(capability).remoteRequest.getNumContainers(); - assertTrue(containersRequestedAny == 1); + assertEquals(1, containersRequestedAny); List> matches = amClient.getMatchingRequests(priority, node, capability); verifyMatches(matches, 2); @@ -417,7 +417,7 @@ public void testAMRMClientMatchStorage() throws YarnException, IOException { // test matching of containers ContainerRequest storedRequest = matches.get(0).iterator().next(); - assertTrue(storedContainer1 == storedRequest); + assertEquals(storedContainer1, storedRequest); amClient.removeContainerRequest(storedContainer1); matches = amClient.getMatchingRequests(priority, ResourceRequest.ANY, capability); @@ -438,10 +438,10 @@ public void testAMRMClientMatchStorage() throws YarnException, IOException { && iterationsLeft-- > 0) { Log.info(" == alloc " + allocatedContainerCount + " it left " + iterationsLeft); AllocateResponse allocResponse = amClient.allocate(0.1f); - assertTrue(amClient.ask.size() == 0); - assertTrue(amClient.release.size() == 0); + assertEquals(0, amClient.ask.size()); + assertEquals(0, amClient.release.size()); - assertTrue(nodeCount == amClient.getClusterNodeCount()); + assertEquals(nodeCount, amClient.getClusterNodeCount()); allocatedContainerCount += allocResponse.getAllocatedContainers().size(); for(Container container : allocResponse.getAllocatedContainers()) { ContainerRequest expectedRequest = @@ -453,7 +453,7 @@ public void testAMRMClientMatchStorage() throws YarnException, IOException { // test correct matched container is returned verifyMatches(matches, 1); ContainerRequest matchedRequest = matches.get(0).iterator().next(); - assertTrue(matchedRequest == expectedRequest); + assertEquals(matchedRequest, expectedRequest); amClient.removeContainerRequest(matchedRequest); // assign this container, use it and release it amClient.releaseAssignedContainer(container.getId()); @@ -464,11 +464,11 @@ public void testAMRMClientMatchStorage() throws YarnException, IOException { } } - assertTrue(allocatedContainerCount == 2); + assertEquals(2, allocatedContainerCount); AllocateResponse allocResponse = amClient.allocate(0.1f); - assertTrue(amClient.release.size() == 0); - assertTrue(amClient.ask.size() == 0); - assertTrue(allocResponse.getAllocatedContainers().size() == 0); + assertEquals(0, amClient.release.size()); + assertEquals(0, amClient.ask.size()); + assertEquals(0, allocResponse.getAllocatedContainers().size()); // 0 requests left. everything got cleaned up assertTrue(amClient.remoteRequestsTable.isEmpty()); @@ -494,14 +494,14 @@ public void testAllocationWithBlacklist() throws YarnException, IOException { amClient.start(); amClient.registerApplicationMaster("Host", 10000, ""); - assertTrue(amClient.ask.size() == 0); - assertTrue(amClient.release.size() == 0); + assertEquals(0, amClient.ask.size()); + assertEquals(0, amClient.release.size()); ContainerRequest storedContainer1 = new ContainerRequest(capability, nodes, racks, priority); amClient.addContainerRequest(storedContainer1); - assertTrue(amClient.ask.size() == 3); - assertTrue(amClient.release.size() == 0); + assertEquals(3, amClient.ask.size()); + assertEquals(0, amClient.release.size()); List localNodeBlacklist = new ArrayList(); localNodeBlacklist.add(node); @@ -512,7 +512,7 @@ public void testAllocationWithBlacklist() throws YarnException, IOException { int allocatedContainerCount = getAllocatedContainersNumber(amClient, DEFAULT_ITERATION); // the only node is in blacklist, so no allocation - assertTrue(allocatedContainerCount == 0); + assertEquals(0, allocatedContainerCount); // Remove node from blacklist, so get assigned with 2 amClient.updateBlacklist(null, localNodeBlacklist); @@ -521,7 +521,7 @@ public void testAllocationWithBlacklist() throws YarnException, IOException { amClient.addContainerRequest(storedContainer2); allocatedContainerCount = getAllocatedContainersNumber(amClient, DEFAULT_ITERATION); - assertEquals(allocatedContainerCount, 2); + assertEquals(2, allocatedContainerCount); // Test in case exception in allocate(), blacklist is kept assertTrue(amClient.blacklistAdditions.isEmpty()); @@ -538,7 +538,7 @@ public void testAllocationWithBlacklist() throws YarnException, IOException { amClient.allocate(0.1f); fail("there should be an exception here."); } catch (Exception e) { - assertEquals(amClient.blacklistAdditions.size(), 1); + assertEquals(1, amClient.blacklistAdditions.size()); } } finally { if (amClient != null && amClient.getServiceState() == STATE.STARTED) { @@ -565,16 +565,16 @@ public void testAMRMClientWithBlacklist() throws YarnException, IOException { nodeList01.add(nodes[0]); nodeList01.add(nodes[1]); amClient.updateBlacklist(nodeList01, null); - assertEquals(amClient.blacklistAdditions.size(),2); - assertEquals(amClient.blacklistRemovals.size(),0); + assertEquals(2, amClient.blacklistAdditions.size()); + assertEquals(0, amClient.blacklistRemovals.size()); // Add nodes[0] again, verify it is not added duplicated. List nodeList02 = new ArrayList(); nodeList02.add(nodes[0]); nodeList02.add(nodes[2]); amClient.updateBlacklist(nodeList02, null); - assertEquals(amClient.blacklistAdditions.size(),3); - assertEquals(amClient.blacklistRemovals.size(),0); + assertEquals(3, amClient.blacklistAdditions.size()); + assertEquals(0, amClient.blacklistRemovals.size()); // Add nodes[1] and nodes[2] to removal list, // Verify addition list remove these two nodes. @@ -582,16 +582,16 @@ public void testAMRMClientWithBlacklist() throws YarnException, IOException { nodeList12.add(nodes[1]); nodeList12.add(nodes[2]); amClient.updateBlacklist(null, nodeList12); - assertEquals(amClient.blacklistAdditions.size(),1); - assertEquals(amClient.blacklistRemovals.size(),2); + assertEquals(1, amClient.blacklistAdditions.size()); + assertEquals(2, amClient.blacklistRemovals.size()); // Add nodes[1] again to addition list, // Verify removal list will remove this node. List nodeList1 = new ArrayList(); nodeList1.add(nodes[1]); amClient.updateBlacklist(nodeList1, null); - assertEquals(amClient.blacklistAdditions.size(),2); - assertEquals(amClient.blacklistRemovals.size(),1); + assertEquals(2, amClient.blacklistAdditions.size()); + assertEquals(1, amClient.blacklistRemovals.size()); } finally { if (amClient != null && amClient.getServiceState() == STATE.STARTED) { amClient.stop(); @@ -606,10 +606,10 @@ private int getAllocatedContainersNumber( while (iterationsLeft-- > 0) { Log.info(" == alloc " + allocatedContainerCount + " it left " + iterationsLeft); AllocateResponse allocResponse = amClient.allocate(0.1f); - assertTrue(amClient.ask.size() == 0); - assertTrue(amClient.release.size() == 0); + assertEquals(0, amClient.ask.size()); + assertEquals(0, amClient.release.size()); - assertTrue(nodeCount == amClient.getClusterNodeCount()); + assertEquals(nodeCount, amClient.getClusterNodeCount()); allocatedContainerCount += allocResponse.getAllocatedContainers().size(); if(allocatedContainerCount == 0) { @@ -654,8 +654,8 @@ private void testAllocation(final AMRMClientImpl amClient) throws YarnException, IOException { // setup container request - assertTrue(amClient.ask.size() == 0); - assertTrue(amClient.release.size() == 0); + assertEquals(0, amClient.ask.size()); + assertEquals(0, amClient.release.size()); amClient.addContainerRequest( new ContainerRequest(capability, nodes, racks, priority)); @@ -677,11 +677,11 @@ private void testAllocation(final AMRMClientImpl amClient) int containersRequestedAny = amClient.remoteRequestsTable.get(priority) .get(ResourceRequest.ANY).get(capability).remoteRequest.getNumContainers(); - assertTrue(containersRequestedNode == 2); - assertTrue(containersRequestedRack == 2); - assertTrue(containersRequestedAny == 2); - assertTrue(amClient.ask.size() == 3); - assertTrue(amClient.release.size() == 0); + assertEquals(2, containersRequestedNode); + assertEquals(2, containersRequestedRack); + assertEquals(2, containersRequestedAny); + assertEquals(3, amClient.ask.size()); + assertEquals(0, amClient.release.size()); // RM should allocate container within 2 calls to allocate() int allocatedContainerCount = 0; @@ -695,10 +695,10 @@ private void testAllocation(final AMRMClientImpl amClient) while (allocatedContainerCount < containersRequestedAny && iterationsLeft-- > 0) { AllocateResponse allocResponse = amClient.allocate(0.1f); - assertTrue(amClient.ask.size() == 0); - assertTrue(amClient.release.size() == 0); + assertEquals(0, amClient.ask.size()); + assertEquals(0, amClient.release.size()); - assertTrue(nodeCount == amClient.getClusterNodeCount()); + assertEquals(nodeCount, amClient.getClusterNodeCount()); allocatedContainerCount += allocResponse.getAllocatedContainers().size(); for(Container container : allocResponse.getAllocatedContainers()) { ContainerId rejectContainerId = container.getId(); @@ -724,19 +724,19 @@ private void testAllocation(final AMRMClientImpl amClient) Assert.assertTrue(receivedNMTokens.size() > 0 && receivedNMTokens.size() <= nodeCount); - assertTrue(allocatedContainerCount == containersRequestedAny); - assertTrue(amClient.release.size() == 2); - assertTrue(amClient.ask.size() == 0); + assertEquals(allocatedContainerCount, containersRequestedAny); + assertEquals(2, amClient.release.size()); + assertEquals(0, amClient.ask.size()); // need to tell the AMRMClient that we dont need these resources anymore amClient.removeContainerRequest( new ContainerRequest(capability, nodes, racks, priority)); amClient.removeContainerRequest( new ContainerRequest(capability, nodes, racks, priority)); - assertTrue(amClient.ask.size() == 3); + assertEquals(3, amClient.ask.size()); // send 0 container count request for resources that are no longer needed ResourceRequest snoopRequest = amClient.ask.iterator().next(); - assertTrue(snoopRequest.getNumContainers() == 0); + assertEquals(0, snoopRequest.getNumContainers()); // test RPC exception handling amClient.addContainerRequest(new ContainerRequest(capability, nodes, @@ -744,7 +744,7 @@ private void testAllocation(final AMRMClientImpl amClient) amClient.addContainerRequest(new ContainerRequest(capability, nodes, racks, priority)); snoopRequest = amClient.ask.iterator().next(); - assertTrue(snoopRequest.getNumContainers() == 2); + assertEquals(2, snoopRequest.getNumContainers()); ApplicationMasterProtocol realRM = amClient.rmClient; try { @@ -768,12 +768,12 @@ public AllocateResponse answer(InvocationOnMock invocation) amClient.rmClient = realRM; } - assertTrue(amClient.release.size() == 2); - assertTrue(amClient.ask.size() == 3); + assertEquals(2, amClient.release.size()); + assertEquals(3, amClient.ask.size()); snoopRequest = amClient.ask.iterator().next(); // verify that the remove request made in between makeRequest and allocate // has not been lost - assertTrue(snoopRequest.getNumContainers() == 0); + assertEquals(0, snoopRequest.getNumContainers()); iterationsLeft = 3; // do a few iterations to ensure RM is not going send new containers @@ -781,13 +781,13 @@ public AllocateResponse answer(InvocationOnMock invocation) // inform RM of rejection AllocateResponse allocResponse = amClient.allocate(0.1f); // RM did not send new containers because AM does not need any - assertTrue(allocResponse.getAllocatedContainers().size() == 0); + assertEquals(0, allocResponse.getAllocatedContainers().size()); if(allocResponse.getCompletedContainersStatuses().size() > 0) { for(ContainerStatus cStatus :allocResponse .getCompletedContainersStatuses()) { if(releases.contains(cStatus.getContainerId())) { - assertTrue(cStatus.getState() == ContainerState.COMPLETE); - assertTrue(cStatus.getExitStatus() == -100); + assertEquals(cStatus.getState(), ContainerState.COMPLETE); + assertEquals(-100, cStatus.getExitStatus()); releases.remove(cStatus.getContainerId()); } } @@ -797,8 +797,8 @@ public AllocateResponse answer(InvocationOnMock invocation) sleep(100); } } - assertTrue(amClient.ask.size() == 0); - assertTrue(amClient.release.size() == 0); + assertEquals(0, amClient.ask.size()); + assertEquals(0, amClient.release.size()); } private void sleep(int sleepTime) {