From 3b409e4d0e136380042a59a421ec4c4dc5b95c18 Mon Sep 17 00:00:00 2001
From: Eric Fried <openstack@fried.cc>
Date: Tue, 5 Nov 2019 13:26:19 -0600
Subject: [PATCH] Refactor AggregateTests

While investigating the referenced story/bug I noticed that
wait_for_status in
openstackclient.tests.functional.compute.v2.test_aggregate.AggregateTests
was doing a lot more than it should ever need to (it probably got copied
in from somewhere). The two places calling it only need to a) check the
output of `openstack aggregate show`, and b) try once -- since they just
got done creating the aggregate synchronously, there should never be a
need to delay/retry. So this commit removes the helper method and just
inlines the check.

At the same time, the addCleanup(aggregate delete) directives are moved
above their respective creates. This is a defensive best practice which
makes sure cleanup happens even if something fails very soon after the
actual back-end create (as was in fact the case with the referenced
bug/story).

It is unknown whether this will impact the referenced bug.

Change-Id: I0d7432f13642fbccd5ca79da9c76adfcbabb5fa9
Story: 2006811
Related-Bug: #1851391
---
 .../functional/compute/v2/test_aggregate.py   | 66 +++++++------------
 1 file changed, 22 insertions(+), 44 deletions(-)

diff --git a/openstackclient/tests/functional/compute/v2/test_aggregate.py b/openstackclient/tests/functional/compute/v2/test_aggregate.py
index be318ae5e3..1de5309921 100644
--- a/openstackclient/tests/functional/compute/v2/test_aggregate.py
+++ b/openstackclient/tests/functional/compute/v2/test_aggregate.py
@@ -11,7 +11,6 @@
 #    under the License.
 
 import json
-import time
 import uuid
 
 from openstackclient.tests.functional import base
@@ -20,34 +19,14 @@ from openstackclient.tests.functional import base
 class AggregateTests(base.TestCase):
     """Functional tests for aggregate"""
 
-    def wait_for_status(self, check_type, check_name, desired_status,
-                        wait=120, interval=5, failures=None):
-        current_status = "notset"
-        if failures is None:
-            failures = ['error']
-        total_sleep = 0
-        while total_sleep < wait:
-            output = json.loads(self.openstack(
-                check_type + ' show -f json ' + check_name))
-            current_status = output['name']
-            if (current_status == desired_status):
-                print('{} {} now has status {}'
-                      .format(check_type, check_name, current_status))
-                return
-            print('Checking {} {} Waiting for {} current status: {}'
-                  .format(check_type, check_name,
-                          desired_status, current_status))
-            if current_status in failures:
-                raise Exception(
-                    'Current status {} of {} {} is one of failures {}'
-                    .format(current_status, check_type, check_name, failures))
-            time.sleep(interval)
-            total_sleep += interval
-        self.assertOutput(desired_status, current_status)
-
     def test_aggregate_crud(self):
         """Test create, delete multiple"""
         name1 = uuid.uuid4().hex
+        self.addCleanup(
+            self.openstack,
+            'aggregate delete ' + name1,
+            fail_ok=True,
+        )
         cmd_output = json.loads(self.openstack(
             'aggregate create -f json ' +
             '--zone nova ' +
@@ -66,14 +45,16 @@ class AggregateTests(base.TestCase):
             'a',
             cmd_output['properties']
         )
-        self.wait_for_status('aggregate', name1, name1)
-        self.addCleanup(
-            self.openstack,
-            'aggregate delete ' + name1,
-            fail_ok=True,
-        )
+        cmd_output = json.loads(self.openstack(
+            'aggregate show -f json ' + name1))
+        self.assertEqual(name1, cmd_output['name'])
 
         name2 = uuid.uuid4().hex
+        self.addCleanup(
+            self.openstack,
+            'aggregate delete ' + name2,
+            fail_ok=True,
+        )
         cmd_output = json.loads(self.openstack(
             'aggregate create -f json ' +
             '--zone external ' +
@@ -87,15 +68,17 @@ class AggregateTests(base.TestCase):
             'external',
             cmd_output['availability_zone']
         )
-        self.wait_for_status('aggregate', name2, name2)
-        self.addCleanup(
-            self.openstack,
-            'aggregate delete ' + name2,
-            fail_ok=True,
-        )
+        cmd_output = json.loads(self.openstack(
+            'aggregate show -f json ' + name2))
+        self.assertEqual(name2, cmd_output['name'])
 
         # Test aggregate set
         name3 = uuid.uuid4().hex
+        self.addCleanup(
+            self.openstack,
+            'aggregate delete ' + name3,
+            fail_ok=True,
+        )
         raw_output = self.openstack(
             'aggregate set ' +
             '--name ' + name3 + ' ' +
@@ -105,11 +88,6 @@ class AggregateTests(base.TestCase):
             name1
         )
         self.assertOutput('', raw_output)
-        self.addCleanup(
-            self.openstack,
-            'aggregate delete ' + name3,
-            fail_ok=True,
-        )
 
         cmd_output = json.loads(self.openstack(
             'aggregate show -f json ' +
@@ -196,11 +174,11 @@ class AggregateTests(base.TestCase):
             self.skipTest("Skip aggregates in a Nova cells v1 configuration")
 
         name = uuid.uuid4().hex
+        self.addCleanup(self.openstack, 'aggregate delete ' + name)
         self.openstack(
             'aggregate create ' +
             name
         )
-        self.addCleanup(self.openstack, 'aggregate delete ' + name)
 
         # Test add host
         cmd_output = json.loads(self.openstack(