Issue #830 vcloud-director: fixing live tests (User and Task)

labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/User.java

@@ -348,7 +348,7 @@ public class User extends EntityType {
       this.telephone = builder.telephone;
       isEnabled = builder.isEnabled;
       isLocked = builder.isLocked;
-      this.im = checkNotNull(builder.im, "im");
+      this.im = builder.im;
       this.nameInSource = builder.nameInSource;
       isAlertEnabled = builder.isAlertEnabled;
       this.alertEmailPrefix = builder.alertEmailPrefix;

labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/predicates/TaskStatusEquals.java

@@ -0,0 +1,89 @@
+/*
+ * Licensed to jclouds, Inc. (jclouds) under one or more
+ * contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  jclouds licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.jclouds.vcloud.director.v1_5.predicates;
+
+import java.util.Collection;
+import java.util.Collections;
+
+import javax.annotation.Resource;
+
+import org.jclouds.logging.Logger;
+import org.jclouds.vcloud.director.v1_5.VCloudDirectorException;
+import org.jclouds.vcloud.director.v1_5.domain.Task;
+import org.jclouds.vcloud.director.v1_5.features.TaskClient;
+
+import com.google.common.base.Predicate;
+
+/**
+ * Test a {@link Task} to see if it has succeeded.
+ * 
+ * @author grkvlt@apache.org
+ */
+public class TaskStatusEquals implements Predicate<Task> {
+
+   private final TaskClient taskClient;
+
+   @Resource
+   protected Logger logger = Logger.NULL;
+
+   private Collection<String> expectedStatuses;
+   private Collection<String> failingStatuses;
+
+   // TODO Use Task.Status, once it is turned into an enumeration
+   public TaskStatusEquals(TaskClient taskClient, String expectedStatus, Collection<String> failingStatuses) {
+      this(taskClient, Collections.singleton(expectedStatus), failingStatuses);
+   }
+
+   public TaskStatusEquals(TaskClient taskClient, Collection<String> expectedStatuses, Collection<String> failingStatuses) {
+      this.taskClient = taskClient;
+      this.expectedStatuses = expectedStatuses;
+      this.failingStatuses = failingStatuses;
+   }
+
+   /** @see Predicate#apply(Object) */
+   @Override
+   public boolean apply(Task task) {
+      logger.trace("looking for status on task %s", task);
+
+      // TODO shouldn't we see if it's already done before getting it from API server?
+      task = taskClient.getTask(task.getHref());
+      
+      // perhaps task isn't available, yet
+      if (task == null) return false;
+      logger.trace("%s: looking for status %s: currently: %s", task, expectedStatuses, task.getStatus());
+      
+      for (String failingStatus : failingStatuses) {
+         if (task.getStatus().equals(failingStatus)) {
+            throw new VCloudDirectorException(task);
+         }
+      }
+      
+      for (String expectedStatus : expectedStatuses) {
+         if (task.getStatus().equals(expectedStatus)) {
+            return true;
+         }
+      }
+      return false;
+   }
+
+   @Override
+   public String toString() {
+      return "checkTaskSuccess()";
+   }
+}

labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/predicates/TaskSuccess.java

@@ -60,6 +60,10 @@ public class TaskSuccess implements Predicate<Task> {
       logger.trace("%s: looking for status %s: currently: %s", task, Task.Status.SUCCESS, task.getStatus());
       if (task.getStatus().equals(Task.Status.ERROR))
          throw new VCloudDirectorException(task);
+      if (task.getStatus().equals(Task.Status.CANCELED))
+         throw new VCloudDirectorException(task);
+      if (task.getStatus().equals(Task.Status.ABORTED))
+         throw new VCloudDirectorException(task);
       return task.getStatus().equals(Task.Status.SUCCESS);
    }
 

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/GroupClientLiveTest.java

@@ -19,12 +19,9 @@
 package org.jclouds.vcloud.director.v1_5.features;
 
 import static com.google.common.base.Objects.equal;
-import static org.jclouds.vcloud.director.v1_5.VCloudDirectorLiveTestConstants.OBJ_DEL;
 import static org.jclouds.vcloud.director.v1_5.VCloudDirectorLiveTestConstants.OBJ_FIELD_UPDATABLE;
 import static org.jclouds.vcloud.director.v1_5.VCloudDirectorLiveTestConstants.REF_REQ_LIVE;
-import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
@@ -32,7 +29,6 @@ import java.net.URI;
 
 import org.jclouds.vcloud.director.v1_5.VCloudDirectorException;
 import org.jclouds.vcloud.director.v1_5.domain.Checks;
-import org.jclouds.vcloud.director.v1_5.domain.Error;
 import org.jclouds.vcloud.director.v1_5.domain.Group;
 import org.jclouds.vcloud.director.v1_5.domain.Reference;
 import org.jclouds.vcloud.director.v1_5.internal.BaseVCloudDirectorClientLiveTest;
@@ -78,8 +74,8 @@ public class GroupClientLiveTest extends BaseVCloudDirectorClientLiveTest {
       Checks.checkGroup(group);
    }
    
-   @Test(testName = "PUT /admin/group/{id}") // TODO: depends on?
+   @Test(testName = "PUT /admin/group/{id}", dependsOnMethods = { "testGetGroup" } )
-   public void updateGroup() {
+   public void testUpdateGroup() {
       String oldName = group.getName();
       String newName = "new "+oldName;
       String oldDescription = group.getDescription();
@@ -111,28 +107,23 @@ public class GroupClientLiveTest extends BaseVCloudDirectorClientLiveTest {
       }
    }
    
-   @Test(testName = "DELETE /admin/group/{id}", enabled = false )
+   @Test(testName = "DELETE /admin/group/{id}", dependsOnMethods = { "testUpdateGroup" }, enabled = false )
    public void testDeleteCatalog() {
       groupClient.deleteGroup(groupRef.getHref());
       
-      Error expected = Error.builder()
+      // TODO stronger assertion of error expected
-            .message("???")
+//      Error expected = Error.builder()
-            .majorErrorCode(403)
+//            .message("???")
-            .minorErrorCode("ACCESS_TO_RESOURCE_IS_FORBIDDEN")
+//            .majorErrorCode(403)
-            .build();
+//            .minorErrorCode("ACCESS_TO_RESOURCE_IS_FORBIDDEN")
+//            .build();
       
       try {
          group = groupClient.getGroup(groupRef.getHref());
          fail("Should give HTTP 403 error");
       } catch (VCloudDirectorException vde) {
-         assertEquals(vde.getError(), expected);
+         // success
          group = null;
-      } catch (Exception e) {
-         fail("Should have thrown a VCloudDirectorException");
-      }
-      
-      if (group != null) { // guard against NPE on the .toStrings
-         assertNull(group, String.format(OBJ_DEL, GROUP, group.toString()));
       }
    }
 }

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/TaskClientLiveTest.java

@@ -29,10 +29,13 @@ import org.jclouds.vcloud.director.v1_5.domain.OrgList;
 import org.jclouds.vcloud.director.v1_5.domain.Reference;
 import org.jclouds.vcloud.director.v1_5.domain.Task;
 import org.jclouds.vcloud.director.v1_5.domain.TasksList;
+import org.jclouds.vcloud.director.v1_5.domain.VApp;
 import org.jclouds.vcloud.director.v1_5.internal.BaseVCloudDirectorClientLiveTest;
+import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 
 /**
@@ -49,14 +52,7 @@ public class TaskClientLiveTest extends BaseVCloudDirectorClientLiveTest {
 
    private OrgClient orgClient;
    private TaskClient taskClient;
-
+   
-   @Override
-   @BeforeClass(inheritGroups = true)
-   public void setupRequiredClients() {
-      orgClient = context.getApi().getOrgClient();
-      taskClient = context.getApi().getTaskClient();
-   }
-
    /*
     * Shared state between dependant tests.
     */
@@ -66,6 +62,20 @@ public class TaskClientLiveTest extends BaseVCloudDirectorClientLiveTest {
    private TasksList taskList;
    private Task task;
    private URI taskURI;
+   
+   private VApp vApp;
+
+   @Override
+   @BeforeClass(inheritGroups = true)
+   public void setupRequiredClients() {
+      orgClient = context.getApi().getOrgClient();
+      taskClient = context.getApi().getTaskClient();
+   }
+
+   @AfterClass(groups = { "live" })
+   public void cleanUp() throws Exception {
+      if (vApp != null) cleanUpVApp(vApp);
+   }
 
    @Test(testName = "GET /tasksList/{id}")
    public void testGetTaskList() {
@@ -100,10 +110,19 @@ public class TaskClientLiveTest extends BaseVCloudDirectorClientLiveTest {
       // Check required elements and attributes
       checkTask(task);
    }
-   
+
+   // FIXME cancelTask complains "This task can not be canceled"
+   // However, when I do this through the UI, I can cancel the task for instantiating a vApp.
    @Test(testName = "GET /task/{id}/metadata/", dependsOnMethods = { "testGetTask" })
    public void testCancelTask() {
+      vApp = instantiateVApp();
+      
+      Task task = Iterables.getFirst(vApp.getTasks(), null);
+      assertNotNull(task, "instantiateVApp should contain one long-running task");
+      assertTaskStatusEventually(task, Task.Status.RUNNING, ImmutableSet.of(Task.Status.ERROR, Task.Status.ABORTED));
+
       // Call the method being tested
       taskClient.cancelTask(taskURI);
+      assertTaskStatusEventually(task, Task.Status.CANCELED, ImmutableSet.of(Task.Status.ERROR, Task.Status.ABORTED, Task.Status.SUCCESS));
    }
 }

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/UserClientLiveTest.java

@@ -19,10 +19,8 @@
 package org.jclouds.vcloud.director.v1_5.features;
 
 import static com.google.common.base.Objects.equal;
-import static org.jclouds.vcloud.director.v1_5.VCloudDirectorLiveTestConstants.OBJ_DEL;
 import static org.jclouds.vcloud.director.v1_5.VCloudDirectorLiveTestConstants.OBJ_FIELD_UPDATABLE;
 import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
@@ -34,6 +32,7 @@ import org.jclouds.vcloud.director.v1_5.domain.Error;
 import org.jclouds.vcloud.director.v1_5.domain.Reference;
 import org.jclouds.vcloud.director.v1_5.domain.User;
 import org.jclouds.vcloud.director.v1_5.internal.BaseVCloudDirectorClientLiveTest;
+import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
@@ -67,10 +66,23 @@ public class UserClientLiveTest extends BaseVCloudDirectorClientLiveTest {
       orgRef = Iterables.getFirst(context.getApi().getOrgClient().getOrgList().getOrgs(), null).toAdminReference(endpoint);
    }
    
+   @AfterClass(groups = { "live" })
+   public void cleanUp() throws Exception {
+      if (user != null) {
+         try {
+            userClient.deleteUser(user.getHref());
+         } catch (VCloudDirectorException e) {
+            // ignore; user probably already deleted
+         }
+      }
+   }
+   
    @Test(testName = "POST /admin/org/{id}/users")
    public void testCreateUser() {
+      String name = "a"+random.nextInt();
+      
       User newUser = User.builder()
-         .name("test")
+         .name(name)
          .fullName("testFullName")
          .emailAddress("test@test.com")
          .telephone("555-1234")
@@ -86,7 +98,6 @@ public class UserClientLiveTest extends BaseVCloudDirectorClientLiveTest {
             .href(URI.create("https://vcloudbeta.bluelock.com/api/admin/role/ff1e0c91-1288-3664-82b7-a6fa303af4d1"))
             .build())
          .password("password")
-//       .group()
          .build();
       user = userClient.createUser(orgRef.getHref(), newUser);
       
@@ -104,9 +115,11 @@ public class UserClientLiveTest extends BaseVCloudDirectorClientLiveTest {
    @Test(testName = "PUT /admin/user/{id}",
          dependsOnMethods = { "testGetUser" })
    public void testUpdateUser() {
+      // TODO It seems that role.href doesn't need to be valid! I can run on 
+      // greenhousedata with a bluelock uri!
+      
       User oldUser = user.toBuilder().build();
       User newUser = user.toBuilder()
-//         .name("new"+oldUser.getName())
          .fullName("new"+oldUser.getFullName())
          .emailAddress("new"+oldUser.getEmailAddress())
          .telephone("1-"+oldUser.getTelephone())
@@ -115,88 +128,98 @@ public class UserClientLiveTest extends BaseVCloudDirectorClientLiveTest {
          .isAlertEnabled(true)
          .alertEmailPrefix("new"+oldUser.getAlertEmailPrefix())
          .alertEmail("new"+oldUser.getAlertEmail())
+         .storedVmQuota(1)
+         .deployedVmQuota(1)
+         .password("newPassword")
+         // TODO test setting other fields?
+//         .name("new"+oldUser.getName())
 //         .role(Reference.builder() // FIXME: auto-fetch a role? or inject
 //            .name("vApp Author")
 //            .href(URI.create("https://vcloudbeta.bluelock.com/api/admin/role/1bf4457f-a253-3cf1-b163-f319f1a31802"))
 //            .build())
-         .storedVmQuota(1)
-         .deployedVmQuota(1)
-         .password("newPassword")
          .build();
       
-      try {
+      userClient.updateUser(user.getHref(), newUser);
-         userClient.updateUser(user.getHref(), newUser);
+      user = userClient.getUser(user.getHref());
-         user = userClient.getUser(user.getHref());
+      
-         Checks.checkUser(user);
+      Checks.checkUser(user);
-//         assertTrue(equal(user.getName(), newUser.getName()), 
+      assertTrue(equal(user.getFullName(), newUser.getFullName()), 
-//               String.format(OBJ_FIELD_UPDATABLE, USER, "name"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "fullName"));
-         assertTrue(equal(user.getFullName(), newUser.getFullName()), 
+      assertTrue(equal(user.getEmailAddress(), newUser.getEmailAddress()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "fullName"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "emailAddress"));
-         assertTrue(equal(user.getEmailAddress(), newUser.getEmailAddress()), 
+      assertTrue(equal(user.getTelephone(), newUser.getTelephone()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "emailAddress"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "telephone"));
-         assertTrue(equal(user.getTelephone(), newUser.getTelephone()), 
+      assertTrue(equal(user.isEnabled(), newUser.isEnabled()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "telephone"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "isEnabled"));
-         assertTrue(equal(user.isEnabled(), newUser.isEnabled()), 
+      assertTrue(equal(user.getIM(), newUser.getIM()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "isEnabled"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "im"));
-         assertTrue(equal(user.getIM(), newUser.getIM()), 
+      assertTrue(equal(user.isAlertEnabled(), newUser.isAlertEnabled()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "im"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "isAlertEnabled"));
-         assertTrue(equal(user.isAlertEnabled(), newUser.isAlertEnabled()), 
+      assertTrue(equal(user.getAlertEmailPrefix(), newUser.getAlertEmailPrefix()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "isAlertEnabled"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "alertEmailPrefix"));
-         assertTrue(equal(user.getAlertEmailPrefix(), newUser.getAlertEmailPrefix()), 
+      assertTrue(equal(user.getAlertEmail(), newUser.getAlertEmail()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "alertEmailPrefix"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "alertEmail"));
-         assertTrue(equal(user.getAlertEmail(), newUser.getAlertEmail()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "alertEmail"));
 //         assertTrue(equal(user.getRole(), newUser.getRole()), 
 //               String.format(OBJ_FIELD_UPDATABLE, USER, "role"));
-         assertTrue(equal(user.getStoredVmQuota(), newUser.getStoredVmQuota()), 
+      assertTrue(equal(user.getStoredVmQuota(), newUser.getStoredVmQuota()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "storedVmQuota"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "storedVmQuota"));
-         assertTrue(equal(user.getDeployedVmQuota(), newUser.getDeployedVmQuota()), 
+      assertTrue(equal(user.getDeployedVmQuota(), newUser.getDeployedVmQuota()), 
-               String.format(OBJ_FIELD_UPDATABLE, USER, "deployedVmQuota"));
+            String.format(OBJ_FIELD_UPDATABLE, USER, "deployedVmQuota"));
-         
+      
-         // FIXME: assert password is changed with session client?
+      // TODO: assert password is changed with session client?
-      } finally {
-         userClient.updateUser(user.getHref(), oldUser);
-         user = userClient.getUser(user.getHref());
-      }
    }
  
-// 
-
-   
    @Test(testName = "POST /admin/user/{id}/action/unlock",
-         dependsOnMethods = { "testUpdateUser" } )
+         dependsOnMethods = { "testUpdateUser" }, enabled=false )
    public void testUnlockUser() {
+      // FIXME Need to simulate failed login, to lock the account?
+      //
+      // UserType.isLocked states:
+      //  This flag is set if the user account has been locked due to too many invalid login attempts. 
+      // A locked user account can be re-enabled by updating the user with this flag set to false. 
+      // (However, the account cannot be manually locked by setting it to true - setting this flag is 
+      // only done by the login process).
+      
       //TODO: check previous tests a) enabled lockout, b) set password
       //TODO: attempt too many times with the wrong password
       //TODO: verify access is denied
       //TODO: unlock user
       //TODO: verify access is renewed
+      
+      throw new UnsupportedOperationException("Test not yet implemented; need to first cause account to be locked");
    }
  
    @Test(testName = "DELETE /admin/user/{id}",
-         dependsOnMethods = { "testUnlockUser" } )
+         dependsOnMethods = { "testCreateUser" } )
    public void testDeleteUser() {
-      userClient.deleteUser(user.getHref());
+      // Create a user to be deleted (so we remove dependencies on test ordering)
-      
+      String name = "a"+random.nextInt();
+      User newUser = User.builder()
+         .name(name)
+         .role(Reference.builder() // FIXME: auto-fetch a role? or inject
+                  .name("vApp User")
+                  .href(URI.create("https://vcloudbeta.bluelock.com/api/admin/role/ff1e0c91-1288-3664-82b7-a6fa303af4d1"))
+                  .build())
+         .password("password")
+         .build();
+      User userToBeDeleted = userClient.createUser(orgRef.getHref(), newUser);
+
+      // Delete the user
+      userClient.deleteUser(userToBeDeleted.getHref());
+
+      // Confirm cannot no longer be accessed 
       Error expected = Error.builder()
             .message("No access to entity \"(com.vmware.vcloud.entity.user:"+
-                  user.getId().substring("urn:vcloud:user:".length())+")\".")
+                     userToBeDeleted.getId().substring("urn:vcloud:user:".length())+")\".")
             .majorErrorCode(403)
             .minorErrorCode("ACCESS_TO_RESOURCE_IS_FORBIDDEN")
             .build();
       
       try {
-         user = userClient.getUser(user.getHref());
+         userClient.getUser(userToBeDeleted.getHref());
-         fail("Should give HTTP 403 error");
+         fail("Should give HTTP 403 error for accessing user after deleting it ("+userToBeDeleted+")");
       } catch (VCloudDirectorException vde) {
          assertEquals(vde.getError(), expected);
-         user = null;
-      } catch (Exception e) {
-         fail("Should have thrown a VCloudDirectorException");
-      }
-      
-      if (user != null) { // guard against NPE on the .toStrings
-         assertNull(user, String.format(OBJ_DEL, USER, user.toString()));
       }
    }
 }

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/VAppClientLiveTest.java

@@ -149,7 +149,7 @@ public class VAppClientLiveTest extends BaseVCloudDirectorClientLiveTest {
    }
 
    @BeforeClass(inheritGroups = true)
-   public void setupEnvironment() {
+   public void setupEnvironment() throws Exception {
       vdc = vdcClient.getVdc(vdcURI);
       assertNotNull(vdc, String.format(ENTITY_NON_NULL, VDC));
 
@@ -1160,7 +1160,7 @@ public class VAppClientLiveTest extends BaseVCloudDirectorClientLiveTest {
 
    // NOTE This method is also called by the BeforeClass method setupRequiredClients
    @AfterClass(alwaysRun = true, description = "Clean up the environment by deleting created VApps named 'test-vapp' or 'new-name'")
-   public void cleanUp() {
+   public void cleanUp() throws Exception {
       // Find references in the Vdc with the VApp type and named 'test-vapp' or 'new-name'
       Iterable<Reference> vApps = Iterables.filter(
             vdc.getResourceEntities().getResourceEntities(),
@@ -1176,25 +1176,7 @@ public class VAppClientLiveTest extends BaseVCloudDirectorClientLiveTest {
       // If we found any references, delete the VApp they point to
       if (vApps != null && !Iterables.isEmpty(vApps)) {
          for (Reference each : vApps) {
-            VApp found = vAppClient.getVApp(each.getHref());
+            cleanUpVApp(each.getHref());
-            // debug(found);
-
-            // Shutdown and power off the VApp if necessary
-            if (found.getStatus().equals(Status.POWERED_ON.getValue())) {
-               Task shutdownTask = vAppClient.shutdown(found.getHref());
-               retryTaskSuccess.apply(shutdownTask);
-            }
-
-            // Undeploy the VApp if necessary
-            if (found.isDeployed()) {
-               UndeployVAppParams params = UndeployVAppParams.builder().build();
-               Task undeployTask = vAppClient.undeploy(found.getHref(), params);
-               retryTaskSuccess.apply(undeployTask);
-            }
-
-            // Delete the VApp
-            Task deleteTask = vAppClient.deleteVApp(found.getHref());
-            retryTaskSuccess.apply(deleteTask);
          }
       }
    }

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/VAppTemplateClientLiveTest.java

@@ -69,7 +69,6 @@ import com.google.common.collect.Iterables;
 @Test(groups = {"live", "unit", "user"}, testName = "VAppTemplateClientLiveTest")
 public class VAppTemplateClientLiveTest extends BaseVCloudDirectorClientLiveTest {
 
-   private final Random random = new Random();
    private VAppTemplateClient vappTemplateClient;
    private VdcClient vdcClient;
    private VAppClient vappClient;
@@ -88,35 +87,7 @@ public class VAppTemplateClientLiveTest extends BaseVCloudDirectorClientLiveTest
    // TODO remove duplication from other tests
    @AfterClass(groups = { "live" })
    public void cleanUp() throws Exception {
-      if (vApp != null) {
+      if (vApp != null) cleanUpVApp(vApp);
-         vApp = vappClient.getVApp(vApp.getHref()); // update
-         
-         // Shutdown and power off the VApp if necessary
-         if (vApp.getStatus().equals(Status.POWERED_ON.getValue())) {
-            try {
-               Task shutdownTask = vappClient.shutdown(vApp.getHref());
-               retryTaskSuccess.apply(shutdownTask);
-            } catch (Exception e) {
-               // keep going; cleanup as much as possible
-               logger.warn(e, "Continuing cleanup after error shutting down VApp %s", vApp);
-            }
-         }
-
-         // Undeploy the VApp if necessary
-         if (vApp.isDeployed()) {
-            try {
-               UndeployVAppParams params = UndeployVAppParams.builder().build();
-               Task undeployTask = vappClient.undeploy(vApp.getHref(), params);
-               retryTaskSuccess.apply(undeployTask);
-            } catch (Exception e) {
-               // keep going; cleanup as much as possible
-               logger.warn(e, "Continuing cleanup after error undeploying VApp %s", vApp);
-            }
-         }
-         
-         Task task = vappClient.deleteVApp(vApp.getHref());
-         assertTaskSucceeds(task);
-      }
    }
 
    // FIXME cloneVAppTemplate is giving back 500 error

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/VdcClientLiveTest.java

@@ -85,30 +85,24 @@ public class VdcClientLiveTest extends BaseVCloudDirectorClientLiveTest {
    private VAppTemplate uploadedVAppTemplate;
    
    @AfterClass(groups = { "live" })
-   public void cleanUp() {
+   public void cleanUp() throws Exception {
       if (clonedVAppTemplate != null) {
-         Task task = vappTemplateClient.deleteVappTemplate(clonedVAppTemplate.getHref());
+         cleanUpVAppTemplate(clonedVAppTemplate);
-         assertTaskSucceeds(task);
       }
       if (capturedVAppTemplate != null) {
-         Task task = vappTemplateClient.deleteVappTemplate(capturedVAppTemplate.getHref());
+         cleanUpVAppTemplate(capturedVAppTemplate);
-         assertTaskSucceeds(task);
       }
       if (uploadedVAppTemplate != null) {
-         Task task = vappTemplateClient.deleteVappTemplate(uploadedVAppTemplate.getHref());
+         cleanUpVAppTemplate(uploadedVAppTemplate);
-         assertTaskSucceeds(task);
       }
       if (instantiatedVApp != null) {
-         Task task = vappClient.deleteVApp(instantiatedVApp.getHref());
+         cleanUpVApp(instantiatedVApp);
-         assertTaskSucceeds(task);
       }
       if (clonedVApp != null) {
-         Task task = vappClient.deleteVApp(clonedVApp.getHref());
+         cleanUpVApp(clonedVApp);
-         assertTaskSucceeds(task);
       }
       if (composedVApp != null) {
-         Task task = vappClient.deleteVApp(composedVApp.getHref());
+         cleanUpVApp(composedVApp);
-         assertTaskSucceeds(task);
       }
    }
 

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/internal/BaseVCloudDirectorClientLiveTest.java

@@ -25,16 +25,16 @@ import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
 import java.net.URI;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.Properties;
 import java.util.Random;
 import java.util.Set;
 
 import javax.annotation.Resource;
 import javax.inject.Inject;
-import javax.inject.Named;
 
 import org.jclouds.compute.BaseVersionedServiceLiveTest;
-import org.jclouds.compute.reference.ComputeServiceConstants;
 import org.jclouds.date.DateService;
 import org.jclouds.logging.Logger;
 import org.jclouds.logging.log4j.config.Log4JLoggingModule;
@@ -45,6 +45,7 @@ import org.jclouds.sshj.config.SshjSshClientModule;
 import org.jclouds.vcloud.director.testng.FormatApiResultsListener;
 import org.jclouds.vcloud.director.v1_5.VCloudDirectorAsyncClient;
 import org.jclouds.vcloud.director.v1_5.VCloudDirectorClient;
+import org.jclouds.vcloud.director.v1_5.VCloudDirectorException;
 import org.jclouds.vcloud.director.v1_5.VCloudDirectorMediaType;
 import org.jclouds.vcloud.director.v1_5.domain.InstantiateVAppTemplateParams;
 import org.jclouds.vcloud.director.v1_5.domain.InstantiationParams;
@@ -53,14 +54,20 @@ import org.jclouds.vcloud.director.v1_5.domain.NetworkConfigSection;
 import org.jclouds.vcloud.director.v1_5.domain.NetworkConfiguration;
 import org.jclouds.vcloud.director.v1_5.domain.Org;
 import org.jclouds.vcloud.director.v1_5.domain.Reference;
+import org.jclouds.vcloud.director.v1_5.domain.ResourceEntityType.Status;
 import org.jclouds.vcloud.director.v1_5.domain.Session;
 import org.jclouds.vcloud.director.v1_5.domain.Task;
+import org.jclouds.vcloud.director.v1_5.domain.UndeployVAppParams;
 import org.jclouds.vcloud.director.v1_5.domain.VApp;
 import org.jclouds.vcloud.director.v1_5.domain.VAppNetworkConfiguration;
 import org.jclouds.vcloud.director.v1_5.domain.VAppTemplate;
 import org.jclouds.vcloud.director.v1_5.domain.Vdc;
+import org.jclouds.vcloud.director.v1_5.features.TaskClient;
+import org.jclouds.vcloud.director.v1_5.features.VAppClient;
+import org.jclouds.vcloud.director.v1_5.features.VAppTemplateClient;
 import org.jclouds.vcloud.director.v1_5.features.VdcClient;
 import org.jclouds.vcloud.director.v1_5.predicates.ReferenceTypePredicates;
+import org.jclouds.vcloud.director.v1_5.predicates.TaskStatusEquals;
 import org.jclouds.vcloud.director.v1_5.predicates.TaskSuccess;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeGroups;
@@ -90,7 +97,7 @@ public abstract class BaseVCloudDirectorClientLiveTest extends BaseVersionedServ
    @Resource
    protected Logger logger = Logger.NULL;
 
-   protected static final long TASK_TIMEOUT_SECONDS = 10L;
+   protected static final long TASK_TIMEOUT_SECONDS = 100L;
    protected static final long LONG_TASK_TIMEOUT_SECONDS = 300L;
 
    public static final String VAPP = "vApp";
@@ -132,7 +139,7 @@ public abstract class BaseVCloudDirectorClientLiveTest extends BaseVersionedServ
 
    @Inject
    protected void initTaskSuccess(TaskSuccess taskSuccess) {
-      retryTaskSuccess = new RetryablePredicate<Task>(taskSuccess, TASK_TIMEOUT_SECONDS * 10000L);
+      retryTaskSuccess = new RetryablePredicate<Task>(taskSuccess, TASK_TIMEOUT_SECONDS * 1000L);
    }
 
    @Inject
@@ -219,9 +226,25 @@ public abstract class BaseVCloudDirectorClientLiveTest extends BaseVersionedServ
    
    protected void assertTaskSucceedsLong(Task task) {
       assertTrue(retryTaskSuccessLong.apply(task), String.format(TASK_COMPLETE_TIMELY, task));
+   }
 
+   protected void assertTaskStatusEventually(Task task, String expectedStatus, Collection<String> failingStatuses) {
+      TaskClient taskClient = context.getApi().getTaskClient();
+      TaskStatusEquals predicate = new TaskStatusEquals(taskClient, expectedStatus, failingStatuses);
+      RetryablePredicate<Task> retryablePredicate = new RetryablePredicate<Task>(predicate, TASK_TIMEOUT_SECONDS * 1000L);
+      assertTrue(retryablePredicate.apply(task), "Task must enter status "+expectedStatus);
    }
    
+   protected void assertTaskDoneEventually(Task task) {
+      TaskClient taskClient = context.getApi().getTaskClient();
+      TaskStatusEquals predicate = new TaskStatusEquals(
+               taskClient, 
+               ImmutableSet.of(Task.Status.ABORTED, Task.Status.CANCELED, Task.Status.ERROR, Task.Status.SUCCESS), 
+               Collections.<String>emptySet());
+      RetryablePredicate<Task> retryablePredicate = new RetryablePredicate<Task>(predicate, LONG_TASK_TIMEOUT_SECONDS * 1000L);
+      assertTrue(retryablePredicate.apply(task), "Task must be done");
+   }
+
    /**
     * Instantiate a {@link VApp} in a {@link Vdc} using the {@link VAppTemplate} we have configured for the tests.
     * 
@@ -300,4 +323,73 @@ public abstract class BaseVCloudDirectorClientLiveTest extends BaseVersionedServ
 
       return networkConfiguration;
    }
+   
+   protected void cleanUpVAppTemplate(VAppTemplate vAppTemplate) {
+      VAppTemplateClient vappTemplateClient = context.getApi().getVAppTemplateClient();
+      
+      Task task = vappTemplateClient.deleteVappTemplate(vAppTemplate.getHref());
+      assertTaskSucceeds(task);
+   }
+
+   protected void cleanUpVApp(VApp vApp) throws Exception {
+      cleanUpVApp(vApp.getHref());
+   }
+   
+   // TODO code tidy for cleanUpVApp? Seems extremely verbose!
+   protected void cleanUpVApp(URI vAppUri) throws Exception {
+      VAppClient vappClient = context.getApi().getVAppClient();
+
+      VApp vApp;
+      try {
+         vApp = vappClient.getVApp(vAppUri); // update
+      } catch (VCloudDirectorException e) {
+         // presumably vApp has already been deleted; ignore
+         return;
+      }
+      
+      // Wait for busy tasks to complete (don't care if it's failed or successful)
+      // Otherwise, get error on delete "entity is busy completing an operation.
+      if (vApp.getTasks() != null) {
+         for (Task task : vApp.getTasks()) {
+            assertTaskDoneEventually(task);
+         }
+      }
+      
+      // Shutdown and power off the VApp if necessary
+      if (vApp.getStatus().equals(Status.POWERED_ON.getValue())) {
+         try {
+            Task shutdownTask = vappClient.shutdown(vAppUri);
+            retryTaskSuccess.apply(shutdownTask);
+         } catch (Exception e) {
+            // keep going; cleanup as much as possible
+            logger.warn(e, "Continuing cleanup after error shutting down VApp %s", vApp);
+         }
+      }
+
+      // Undeploy the VApp if necessary
+      if (vApp.isDeployed()) {
+         try {
+            UndeployVAppParams params = UndeployVAppParams.builder().build();
+            Task undeployTask = vappClient.undeploy(vAppUri, params);
+            retryTaskSuccess.apply(undeployTask);
+         } catch (Exception e) {
+            // keep going; cleanup as much as possible
+            logger.warn(e, "Continuing cleanup after error undeploying VApp %s", vApp);
+         }
+      }
+      
+      try {
+         Task task = vappClient.deleteVApp(vAppUri);
+         assertTaskSucceeds(task);
+      } catch (Exception e) {
+         try {
+            vApp = vappClient.getVApp(vAppUri); // refresh
+         } catch (Exception e2) {
+            // ignore
+         }
+
+         logger.warn(e, "Deleting vApp failed: vApp="+vApp);
+         throw e;
+      }
+   }
 }