test: Refactor test handler

While authoring [0], it was discovered that Armada has duplicate logic
for deciding if Helm test cleanup should be enabled as well as the tests
themselves. Because of this, changes to test logic (e.g. adding pre-test
actions), requires changing all traces of the repeated logic, which can
lead to inconsistent behavior if not properly addressed. This change
moves all test decision logic to a singular Test handler, implemented by
the `Test` class. This change does NOT change the expected behavior of
testing during upgrades; however, tests initiated from the API and CLI
will not execute when testing a manifest if they are disabled in a
chart, unless using the `--enable-all` flag.

[0] https://review.openstack.org/617834

Change-Id: I1530d7637b0eb6a83f048895053a5db80d033046

armada/api/controller/test.py

@@ -21,8 +21,8 @@ from oslo_config import cfg
 from armada import api
 from armada.common import policy
 from armada import const
-from armada.handlers.test import test_release_for_success
 from armada.handlers.manifest import Manifest
+from armada.handlers.test import Test
 from armada.utils.release import release_prefixer
 from armada.utils import validate
 
@@ -36,14 +36,11 @@ class TestReleasesReleaseNameController(api.BaseResource):
 
     @policy.enforce('armada:test_release')
     def on_get(self, req, resp, release):
-        self.logger.info('RUNNING: %s', release)
         with self.get_tiller(req, resp) as tiller:
-
             cleanup = req.get_param_as_bool('cleanup')
-            if cleanup is None:
+
-                cleanup = False
+            test_handler = Test(release, tiller, cleanup=cleanup)
-            success = test_release_for_success(
+            success = test_handler.test_release_for_success()
-                tiller, release, cleanup=cleanup)
 
         if success:
             msg = {
@@ -56,8 +53,6 @@ class TestReleasesReleaseNameController(api.BaseResource):
                 'message': 'MESSAGE: Test Fail'
             }
 
-        self.logger.info(msg)
-
         resp.body = json.dumps(msg)
         resp.status = falcon.HTTP_200
         resp.content_type = 'application/json'
@@ -135,29 +130,29 @@ class TestReleasesManifestController(api.BaseResource):
                 const.KEYWORD_GROUPS):
             for ch in group.get(const.KEYWORD_CHARTS):
                 chart = ch['chart']
+
                 release_name = release_prefixer(prefix, chart.get('release'))
-                cleanup = req.get_param_as_bool('cleanup')
-                if cleanup is None:
-                    test_chart_override = chart.get('test', {})
-                    if isinstance(test_chart_override, bool):
-                        self.logger.warn(
-                            'Boolean value for chart `test` key is deprecated '
-                            'and will be removed. Use `test.enabled` instead.')
-                        # Use old default value.
-                        cleanup = True
-                    else:
-                        cleanup = test_chart_override.get('options', {}).get(
-                            'cleanup', False)
                 if release_name in known_releases:
-                    self.logger.info('RUNNING: %s tests', release_name)
+                    cleanup = req.get_param_as_bool('cleanup')
-                    success = test_release_for_success(
+                    enable_all = req.get_param_as_bool('enable_all')
-                        tiller, release_name, cleanup=cleanup)
+                    cg_test_charts = group.get('test_charts')
-                    if success:
+                    test_values = chart.get('test', {})
-                        self.logger.info("PASSED: %s", release_name)
+
-                        message['test']['passed'].append(release_name)
+                    test_handler = Test(
-                    else:
+                        release_name,
-                        self.logger.info("FAILED: %s", release_name)
+                        tiller,
-                        message['test']['failed'].append(release_name)
+                        cg_test_charts=cg_test_charts,
+                        cleanup=cleanup,
+                        enable_all=enable_all,
+                        test_values=test_values)
+
+                    if test_handler.test_enabled:
+                        success = test_handler.test_release_for_success()
+
+                        if success:
+                            message['test']['passed'].append(release_name)
+                        else:
+                            message['test']['failed'].append(release_name)
                 else:
                     self.logger.info('Release %s not found - SKIPPING',
                                      release_name)

armada/cli/test.py

@@ -19,8 +19,8 @@ from oslo_config import cfg
 
 from armada.cli import CliAction
 from armada import const
-from armada.handlers.test import test_release_for_success
 from armada.handlers.manifest import Manifest
+from armada.handlers.test import Test
 from armada.handlers.tiller import Tiller
 from armada.utils.release import release_prefixer
 
@@ -79,20 +79,26 @@ SHORT_DESC = "Command tests releases."
     help=("Delete test pods upon completion."),
     is_flag=True,
     default=None)
+@click.option(
+    '--enable-all',
+    help=("Run all tests for all releases regardless of any disabled chart "
+          "tests."),
+    is_flag=True,
+    default=False)
 @click.option('--debug', help="Enable debug logging.", is_flag=True)
 @click.pass_context
 def test_charts(ctx, file, release, tiller_host, tiller_port, tiller_namespace,
-                target_manifest, cleanup, debug):
+                target_manifest, cleanup, enable_all, debug):
     CONF.debug = debug
     TestChartManifest(ctx, file, release, tiller_host, tiller_port,
-                      tiller_namespace, target_manifest,
+                      tiller_namespace, target_manifest, cleanup,
-                      cleanup).safe_invoke()
+                      enable_all).safe_invoke()
 
 
 class TestChartManifest(CliAction):
 
     def __init__(self, ctx, file, release, tiller_host, tiller_port,
-                 tiller_namespace, target_manifest, cleanup):
+                 tiller_namespace, target_manifest, cleanup, enable_all):
 
         super(TestChartManifest, self).__init__()
         self.ctx = ctx
@@ -103,6 +109,7 @@ class TestChartManifest(CliAction):
         self.tiller_namespace = tiller_namespace
         self.target_manifest = target_manifest
         self.cleanup = cleanup
+        self.enable_all = enable_all
 
     def invoke(self):
         with Tiller(
@@ -117,13 +124,8 @@ class TestChartManifest(CliAction):
 
         if self.release:
             if not self.ctx.obj.get('api', False):
-                self.logger.info("RUNNING: %s tests", self.release)
+                test_handler = Test(self.release, tiller, cleanup=self.cleanup)
-                success = test_release_for_success(
+                test_handler.test_release_for_success()
-                    tiller, self.release, cleanup=self.cleanup)
-                if success:
-                    self.logger.info("PASSED: %s", self.release)
-                else:
-                    self.logger.info("FAILED: %s", self.release)
             else:
                 client = self.ctx.obj.get('CLIENT')
                 query = {
@@ -150,32 +152,21 @@ class TestChartManifest(CliAction):
                         const.KEYWORD_GROUPS):
                     for ch in group.get(const.KEYWORD_CHARTS):
                         chart = ch['chart']
+
                         release_name = release_prefixer(
                             prefix, chart.get('release'))
-
                         if release_name in known_release_names:
-                            cleanup = self.cleanup
+                            test_values = chart.get('test', {})
-                            if cleanup is None:
-                                test_chart_override = chart.get('test', {})
-                                if isinstance(test_chart_override, bool):
-                                    self.logger.warn(
-                                        'Boolean value for chart `test` key is'
-                                        ' deprecated and support for this will'
-                                        ' be removed. Use `test.enabled` '
-                                        'instead.')
-                                    # Use old default value.
-                                    cleanup = True
-                                else:
-                                    cleanup = test_chart_override.get(
-                                        'options', {}).get('cleanup', False)
-                            self.logger.info('RUNNING: %s tests', release_name)
-                            success = test_release_for_success(
-                                tiller, release_name, cleanup=cleanup)
-                            if success:
-                                self.logger.info("PASSED: %s", release_name)
-                            else:
-                                self.logger.info("FAILED: %s", release_name)
 
+                            test_handler = Test(
+                                release_name,
+                                tiller,
+                                cleanup=self.cleanup,
+                                enable_all=self.enable_all,
+                                test_values=test_values)
+
+                            if test_handler.test_enabled:
+                                test_handler.test_release_for_success()
                         else:
                             self.logger.info('Release %s not found - SKIPPING',
                                              release_name)

armada/handlers/armada.py

@@ -200,14 +200,6 @@ class Armada(object):
 
             # TODO(MarshM): Deprecate the `test_charts` key
             cg_test_all_charts = chartgroup.get('test_charts')
-            if isinstance(cg_test_all_charts, bool):
-                LOG.warn('The ChartGroup `test_charts` key is deprecated, '
-                         'and support for this will be removed. See the '
-                         'Chart `test` key for more information.')
-            else:
-                # This key defaults to True. Individual charts must
-                # explicitly disable helm tests if they choose
-                cg_test_all_charts = True
 
             cg_charts = chartgroup.get(const.KEYWORD_CHARTS, [])
             charts = map(lambda x: x.get('chart', {}), cg_charts)

armada/handlers/chart_deploy.py

@@ -19,8 +19,8 @@ import yaml
 from armada import const
 from armada.exceptions import armada_exceptions
 from armada.handlers.chartbuilder import ChartBuilder
-from armada.handlers.test import test_release_for_success
 from armada.handlers.release_diff import ReleaseDiff
+from armada.handlers.test import Test
 from armada.handlers.wait import ChartWait
 from armada.exceptions import tiller_exceptions
 import armada.utils.release as r
@@ -202,34 +202,25 @@ class ChartDeploy(object):
         chart_wait.wait(timer)
 
         # Test
-        test_chart_override = chart.get('test')
-        # Use old default value when not using newer `test` key
-        test_cleanup = True
-        if test_chart_override is None:
-            test_enabled = cg_test_all_charts
-        elif isinstance(test_chart_override, bool):
-            LOG.warn('Boolean value for chart `test` key is'
-                     ' deprecated and support for this will'
-                     ' be removed. Use `test.enabled` '
-                     'instead.')
-            test_enabled = test_chart_override
-        else:
-            # NOTE: helm tests are enabled by default
-            test_enabled = test_chart_override.get('enabled', True)
-            test_cleanup = test_chart_override.get('options', {}).get(
-                'cleanup', False)
-
         just_deployed = ('install' in result) or ('upgrade' in result)
         last_test_passed = old_release and r.get_last_test_result(old_release)
-        run_test = test_enabled and (just_deployed or not last_test_passed)
 
+        test_values = chart.get('test')
+        test_handler = Test(
+            release_name,
+            self.tiller,
+            cg_test_charts=cg_test_all_charts,
+            test_values=test_values)
+
+        run_test = test_handler.test_enabled and (just_deployed or
+                                                  not last_test_passed)
         if run_test:
             timer = int(round(deadline - time.time()))
-            self._test_chart(release_name, timer, test_cleanup)
+            self._test_chart(release_name, timer, test_handler)
 
         return result
 
-    def _test_chart(self, release_name, timeout, cleanup):
+    def _test_chart(self, release_name, timeout, test_handler):
         if self.dry_run:
             LOG.info(
                 'Skipping test during `dry-run`, would have tested '
@@ -242,12 +233,8 @@ class ChartDeploy(object):
             LOG.error(reason)
             raise armada_exceptions.ArmadaTimeoutException(reason)
 
-        success = test_release_for_success(
+        success = test_handler.test_release_for_success(timeout=timeout)
-            self.tiller, release_name, timeout=timeout, cleanup=cleanup)
+        if not success:
-        if success:
-            LOG.info("Test passed for release: %s", release_name)
-        else:
-            LOG.info("Test failed for release: %s", release_name)
             raise tiller_exceptions.TestFailedException(release_name)
 
     def get_diff(self, old_chart, old_values, new_chart, values):

armada/handlers/test.py

@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from oslo_log import log as logging
+
 from armada import const
 
 TESTRUN_STATUS_UNKNOWN = 0
@@ -19,16 +21,103 @@ TESTRUN_STATUS_SUCCESS = 1
 TESTRUN_STATUS_FAILURE = 2
 TESTRUN_STATUS_RUNNING = 3
 
-
+LOG = logging.getLogger(__name__)
-def test_release_for_success(tiller,
-                             release,
-                             timeout=const.DEFAULT_TILLER_TIMEOUT,
-                             cleanup=False):
-    test_suite_run = tiller.test_release(
-        release, timeout=timeout, cleanup=cleanup)
-    return get_test_suite_run_success(test_suite_run)
 
 
-def get_test_suite_run_success(test_suite_run):
+class Test(object):
-    return all(
+
-        r.status == TESTRUN_STATUS_SUCCESS for r in test_suite_run.results)
+    def __init__(self,
+                 release_name,
+                 tiller,
+                 cg_test_charts=None,
+                 cleanup=None,
+                 enable_all=False,
+                 test_values=None):
+        """Initialize a test handler to run Helm tests corresponding to a
+        release.
+
+        :param release_name: Name of a Helm release
+        :param tiller: Tiller object
+        :param cg_test_charts: Chart group `test_charts` key
+        :param cleanup: Triggers cleanup; overrides `test.options.cleanup`
+        :param enable_all: Run tests regardless of the value of `test.enabled`
+        :param test_values: Test values retrieved from a chart's `test` key
+
+        :type release_name: str
+        :type tiller: Tiller object
+        :type cg_test_charts: bool
+        :type cleanup: bool
+        :type enable_all: bool
+        :type test_values: dict or bool (deprecated)
+        """
+
+        self.release_name = release_name
+        self.tiller = tiller
+        self.cleanup = cleanup
+
+        # NOTE(drewwalters96): Support the chart_group `test_charts` key until
+        # its deprecation period ends. The `test.enabled`, `enable_all` flag,
+        # and deprecated, boolean `test` key override this value if provided.
+        if cg_test_charts is not None:
+            LOG.warn('Chart group key `test_charts` is deprecated and will be '
+                     'removed. Use `test.enabled` instead.')
+            self.test_enabled = cg_test_charts
+        else:
+            self.test_enabled = True
+
+        # NOTE: Support old, boolean `test` key until deprecation period ends.
+        if (type(test_values) == bool):
+            LOG.warn('Boolean value for chart `test` key is deprecated and '
+                     'will be removed. Use `test.enabled` instead.')
+
+            self.test_enabled = test_values
+
+            # NOTE: Use old, default cleanup value (i.e. True) if none is
+            # provided.
+            if self.cleanup is None:
+                self.cleanup = True
+        elif test_values:
+            test_enabled_opt = test_values.get('enabled')
+            if test_enabled_opt is not None:
+                self.test_enabled = test_enabled_opt
+
+            # NOTE(drewwalters96): `self.cleanup`, the cleanup value provided
+            # by the API/CLI, takes precedence over the chart value
+            # `test.cleanup`.
+            if self.cleanup is None:
+                test_options = test_values.get('options', {})
+                self.cleanup = test_options.get('cleanup', False)
+        else:
+            # Default cleanup value
+            if self.cleanup is None:
+                self.cleanup = False
+
+        if enable_all:
+            self.test_enabled = True
+
+    def test_release_for_success(self, timeout=const.DEFAULT_TILLER_TIMEOUT):
+        """Run the Helm tests corresponding to a release for success (i.e. exit
+        code 0).
+
+        :param timeout: Timeout value for a release's tests completion
+        :type timeout: int
+
+        :rtype: Helm test suite run result
+        """
+        LOG.info('RUNNING: %s tests', self.release_name)
+
+        test_suite_run = self.tiller.test_release(
+            self.release_name, timeout=timeout, cleanup=self.cleanup)
+
+        success = self.get_test_suite_run_success(test_suite_run)
+        if success:
+            LOG.info('PASSED: %s', self.release_name)
+        else:
+            LOG.info('FAILED: %s', self.release_name)
+
+        return success
+
+    @classmethod
+    def get_test_suite_run_success(self, test_suite_run):
+        return all(
+            r.status == TESTRUN_STATUS_SUCCESS for r in test_suite_run.results)

armada/tests/unit/api/test_test_controller.py

@@ -59,7 +59,7 @@ class TestReleasesManifestControllerTest(base.BaseControllerTest):
 
 class TestReleasesReleaseNameControllerTest(base.BaseControllerTest):
 
-    @mock.patch.object(test, 'test_release_for_success')
+    @mock.patch.object(test.Test, 'test_release_for_success')
     @mock.patch.object(api, 'Tiller')
     def test_test_controller_test_pass(self, mock_tiller,
                                        mock_test_release_for_success):
@@ -73,14 +73,13 @@ class TestReleasesReleaseNameControllerTest(base.BaseControllerTest):
 
         release = 'fake-release'
         resp = self.app.simulate_get('/api/v1.0/test/{}'.format(release))
-        mock_test_release_for_success.assert_has_calls(
+        mock_test_release_for_success.assert_called_once()
-            [mock.call(mock_tiller.return_value, release, cleanup=False)])
         self.assertEqual(200, resp.status_code)
         self.assertEqual('MESSAGE: Test Pass',
                          json.loads(resp.text)['message'])
         m_tiller.__exit__.assert_called()
 
-    @mock.patch.object(test, 'test_release_for_success')
+    @mock.patch.object(test.Test, 'test_release_for_success')
     @mock.patch.object(api, 'Tiller')
     def test_test_controller_test_fail(self, mock_tiller,
                                        mock_test_release_for_success):
@@ -98,7 +97,7 @@ class TestReleasesReleaseNameControllerTest(base.BaseControllerTest):
                          json.loads(resp.text)['message'])
         m_tiller.__exit__.assert_called()
 
-    @mock.patch.object(test, 'test_release_for_success')
+    @mock.patch.object(test.Test, 'test_release_for_success')
     @mock.patch.object(api, 'Tiller')
     def test_test_controller_cleanup(self, mock_tiller,
                                      mock_test_release_for_success):
@@ -112,8 +111,7 @@ class TestReleasesReleaseNameControllerTest(base.BaseControllerTest):
         release = 'fake-release'
         resp = self.app.simulate_get(
             '/api/v1.0/test/{}'.format(release), query_string='cleanup=true')
-        mock_test_release_for_success.assert_has_calls(
+        mock_test_release_for_success.assert_called_once()
-            [mock.call(m_tiller, release, cleanup=True)])
         self.assertEqual(200, resp.status_code)
         self.assertEqual('MESSAGE: Test Pass',
                          json.loads(resp.text)['message'])
@@ -125,7 +123,7 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest):
 
     @mock.patch.object(test, 'Manifest')
     @mock.patch.object(api, 'Tiller')
-    @mock.patch.object(test, 'test_release_for_success')
+    @mock.patch.object(test.Test, 'test_release_for_success')
     def test_test_controller_tiller_exc_returns_500(
             self, mock_test_release_for_success, mock_tiller, _):
         rules = {'armada:test_manifest': '@'}
@@ -227,7 +225,7 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest):
 class TestReleasesReleaseNameControllerNegativeTest(base.BaseControllerTest):
 
     @mock.patch.object(api, 'Tiller')
-    @mock.patch.object(test, 'test_release_for_success')
+    @mock.patch.object(test.Test, 'test_release_for_success')
     def test_test_controller_tiller_exc_returns_500(
             self, mock_test_release_for_success, mock_tiller):
         rules = {'armada:test_release': '@'}

armada/tests/unit/handlers/test_armada.py

@@ -17,7 +17,6 @@ import yaml
 
 from armada import const
 from armada.handlers import armada
-from armada.handlers import chart_deploy
 from armada.handlers.test import TESTRUN_STATUS_SUCCESS, TESTRUN_STATUS_FAILURE
 from armada.tests.unit import base
 from armada.tests.test_utils import AttrDict, makeMockThreadSafe
@@ -337,9 +336,9 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
         @mock.patch.object(armada.Armada, 'post_flight_ops')
         @mock.patch.object(armada.Armada, 'pre_flight_ops')
         @mock.patch('armada.handlers.chart_deploy.ChartBuilder')
-        @mock.patch.object(chart_deploy, 'test_release_for_success')
+        @mock.patch('armada.handlers.chart_deploy.Test')
-        def _do_test(mock_test_release_for_success, mock_chartbuilder,
+        def _do_test(mock_test, mock_chartbuilder, mock_pre_flight,
-                     mock_pre_flight, mock_post_flight):
+                     mock_post_flight):
             # Instantiate Armada object.
             yaml_documents = list(yaml.safe_load_all(TEST_YAML))
 
@@ -351,8 +350,9 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
 
             chart_group = armada_obj.manifest['armada']['chart_groups'][0]
             charts = chart_group['chart_group']
-            cg_test_all_charts = chart_group.get('test_charts', True)
+            cg_test_all_charts = chart_group.get('test_charts')
 
+            mock_test_release = mock_test.return_value.test_release_for_success
             if test_failure_to_run:
 
                 def fail(tiller, release, timeout=None, cleanup=False):
@@ -361,9 +361,9 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
                     raise tiller_exceptions.ReleaseException(
                         release, status, 'Test')
 
-                mock_test_release_for_success.side_effect = fail
+                mock_test_release.side_effect = fail
             else:
-                mock_test_release_for_success.return_value = test_success
+                mock_test_release.return_value = test_success
 
             # Stub out irrelevant methods called by `armada.sync()`.
             mock_chartbuilder.get_source_path.return_value = None
@@ -377,7 +377,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
             expected_install_release_calls = []
             expected_update_release_calls = []
             expected_uninstall_release_calls = []
-            expected_test_release_for_success_calls = []
+            expected_test_constructor_calls = []
 
             for c in charts:
                 chart = c['chart']
@@ -389,7 +389,6 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
                 native_wait_enabled = (chart['wait'].get('native', {}).get(
                     'enabled', True))
 
-                expected_apply = True
                 if release_name not in [x.name for x in known_releases]:
                     expected_install_release_calls.append(
                         mock.call(
@@ -435,9 +434,7 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
                                         break
 
                         if status == const.STATUS_DEPLOYED:
-                            if not diff:
+                            if diff:
-                                expected_apply = False
-                            else:
                                 upgrade = chart.get('upgrade', {})
                                 disable_hooks = upgrade.get('no_hooks', False)
                                 force = upgrade.get('force', False)
@@ -462,25 +459,12 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
                                         timeout=mock.ANY))
 
                 test_chart_override = chart.get('test')
-                # Use old default value when not using newer `test` key
+                expected_test_constructor_calls.append(
-                test_cleanup = True
+                    mock.call(
-                if test_chart_override is None:
+                        release_name,
-                    test_this_chart = cg_test_all_charts
+                        m_tiller,
-                elif isinstance(test_chart_override, bool):
+                        cg_test_charts=cg_test_all_charts,
-                    test_this_chart = test_chart_override
+                        test_values=test_chart_override))
-                else:
-                    test_this_chart = test_chart_override.get('enabled', True)
-                    test_cleanup = test_chart_override.get('options', {}).get(
-                        'cleanup', False)
-
-                if test_this_chart and (expected_apply or
-                                        not expected_last_test_result):
-                    expected_test_release_for_success_calls.append(
-                        mock.call(
-                            m_tiller,
-                            release_name,
-                            timeout=mock.ANY,
-                            cleanup=test_cleanup))
 
             any_order = not chart_group['sequenced']
             # Verify that at least 1 release is either installed or updated.
@@ -511,10 +495,10 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
             # Verify that the expected number of deployed releases are
             # tested with expected arguments.
             self.assertEqual(
-                len(expected_test_release_for_success_calls),
+                len(expected_test_constructor_calls), mock_test.call_count)
-                mock_test_release_for_success.call_count)
+
-            mock_test_release_for_success.assert_has_calls(
+            mock_test.assert_has_calls(
-                expected_test_release_for_success_calls, any_order=any_order)
+                expected_test_constructor_calls, any_order=True)
 
         _do_test()
 

armada/tests/unit/handlers/test_test.py

@@ -14,8 +14,8 @@
 
 import mock
 
-from armada.handlers import tiller
 from armada.handlers import test
+from armada.handlers import tiller
 from armada.tests.unit import base
 from armada.tests.test_utils import AttrDict
 
@@ -32,7 +32,9 @@ class TestHandlerTestCase(base.ArmadaTestCase):
             tiller_obj.test_release = mock.Mock()
             tiller_obj.test_release.return_value = AttrDict(
                 **{'results': results})
-            success = test.test_release_for_success(tiller_obj, release)
+
+            test_handler = test.Test(release, tiller_obj)
+            success = test_handler.test_release_for_success()
 
             self.assertEqual(expected_success, success)
 
@@ -62,3 +64,198 @@ class TestHandlerTestCase(base.ArmadaTestCase):
             AttrDict(**{'status': test.TESTRUN_STATUS_SUCCESS}),
             AttrDict(**{'status': test.TESTRUN_STATUS_RUNNING})
         ])
+
+    def test_cg_disabled(self):
+        """Test that tests are disabled when a chart group disables all
+        tests.
+        """
+        test_handler = test.Test(
+            release_name='release', tiller=mock.Mock(), cg_test_charts=False)
+
+        assert test_handler.test_enabled is False
+
+    def test_cg_disabled_test_key_enabled(self):
+        """Test that tests are enabled when a chart group disables all
+        tests and the deprecated, boolean `test` key is enabled.
+        """
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            cg_test_charts=False,
+            test_values=True)
+
+        assert test_handler.test_enabled is True
+
+    def test_cg_disabled_test_values_enabled(self):
+        """Test that tests are enabled when a chart group disables all
+        tests and the `test.enabled` key is False.
+        """
+        test_values = {'enabled': True}
+
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            cg_test_charts=False,
+            test_values=test_values)
+
+        assert test_handler.test_enabled is True
+
+    def test_cg_enabled_test_key_disabled(self):
+        """Test that tests are disabled when a chart group enables all
+        tests and the deprecated, boolean `test` key is disabled.
+        """
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            cg_test_charts=True,
+            test_values=False)
+
+        assert test_handler.test_enabled is False
+
+    def test_cg_enabled_test_values_disabled(self):
+        """Test that tests are disabled when a chart group enables all
+        tests and the deprecated, boolean `test` key is disabled.
+        """
+        test_values = {'enabled': False}
+
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            cg_test_charts=True,
+            test_values=test_values)
+
+        assert test_handler.test_enabled is False
+
+    def test_enable_all_cg_disabled(self):
+        """Test that tests are enabled when the `enable_all` parameter is
+        True and the chart group `test_enabled` key is disabled.
+        """
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            cg_test_charts=False,
+            enable_all=True)
+
+        assert test_handler.test_enabled is True
+
+    def test_enable_all_test_key_disabled(self):
+        """Test that tests are enabled when the `enable_all` parameter is
+        True and the deprecated, boolean `test` key is disabled.
+        """
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            enable_all=True,
+            test_values=False)
+
+        assert test_handler.test_enabled is True
+
+    def test_enable_all_test_values_disabled(self):
+        """Test that tests are enabled when the `enable_all` parameter is
+        True and the `test.enabled` key is False.
+        """
+        test_values = {'enabled': False}
+
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            enable_all=True,
+            test_values=test_values)
+
+        assert test_handler.test_enabled is True
+
+    def test_deprecated_test_key_false(self):
+        """Test that tests can be disabled using the deprecated, boolean value
+        for a chart's test key.
+        """
+        test_handler = test.Test(
+            release_name='release', tiller=mock.Mock(), test_values=True)
+
+        assert test_handler.test_enabled
+
+    def test_deprecated_test_key_true(self):
+        """Test that cleanup is enabled by default when tests are enabled using
+        the deprecated, boolean value for a chart's `test` key.
+        """
+        test_handler = test.Test(
+            release_name='release', tiller=mock.Mock(), test_values=True)
+
+        assert test_handler.test_enabled is True
+        assert test_handler.cleanup is True
+
+    def test_tests_disabled(self):
+        """Test that tests are disabled by a chart's values using the
+        `test.enabled` path.
+        """
+        test_values = {'enabled': False}
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            test_values=test_values)
+
+        assert test_handler.test_enabled is False
+
+    def test_tests_enabled(self):
+        """Test that cleanup is disabled (by default) when tests are enabled by
+        a chart's values using the `test.enabled` path.
+        """
+        test_values = {'enabled': True}
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            test_values=test_values)
+
+        assert test_handler.test_enabled is True
+        assert test_handler.cleanup is False
+
+    def test_tests_enabled_cleanup_enabled(self):
+        """Test that the test handler uses the values provided by a chart's
+        `test` key.
+        """
+        test_values = {'enabled': True, 'options': {'cleanup': True}}
+
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            test_values=test_values)
+
+        assert test_handler.test_enabled is True
+        assert test_handler.cleanup is True
+
+    def test_tests_enabled_cleanup_disabled(self):
+        """Test that the test handler uses the values provided by a chart's
+        `test` key.
+        """
+        test_values = {'enabled': True, 'options': {'cleanup': False}}
+
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            test_values=test_values)
+
+        assert test_handler.test_enabled is True
+        assert test_handler.cleanup is False
+
+    def test_no_test_values(self):
+        """Test that the default values are enforced when no chart `test`
+        values are provided (i.e. tests are enabled and cleanup is disabled).
+        """
+        test_handler = test.Test(release_name='release', tiller=mock.Mock())
+
+        assert test_handler.test_enabled is True
+        assert test_handler.cleanup is False
+
+    def test_override_cleanup(self):
+        """Test that a cleanup value passed to the Test handler (i.e. from the
+        API/CLI) takes precedence over a chart's `test.cleanup` value.
+        """
+        test_values = {'enabled': True, 'options': {'cleanup': False}}
+
+        test_handler = test.Test(
+            release_name='release',
+            tiller=mock.Mock(),
+            cleanup=True,
+            test_values=test_values)
+
+        assert test_handler.test_enabled is True
+        assert test_handler.cleanup is True

armada/utils/release.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from armada.handlers.test import get_test_suite_run_success
+from armada.handlers.test import Test
 
 
 def release_prefixer(prefix, release):
@@ -52,4 +52,4 @@ def get_last_test_result(release):
     status = release.info.status
     if not status.HasField('last_test_suite_run'):
         return None
-    return get_test_suite_run_success(status.last_test_suite_run)
+    return Test.get_test_suite_run_success(status.last_test_suite_run)

doc/source/commands/test.rst

@@ -24,6 +24,8 @@ Commands
           $ armada test --release blog-1
 
     Options:
+      --cleanup                     Delete test pods after test completion
+      --enable-all                  Run disabled chart tests
       --file TEXT                   armada manifest
       --release TEXT                helm release
       --tiller-host TEXT            Tiller Host IP

swagger/swaggerV3-api.yaml

@@ -155,6 +155,7 @@ paths:
         - $ref: "#/components/parameters/tiller-port"
         - $ref: "#/components/parameters/tiller-namespace"
         - $ref: "#/components/parameters/target-manifest"
+        - $ref: "#/components/parameters/enable-all"
       requestBody:
         $ref: "#/components/requestBodies/manifest-body"
       responses:
@@ -337,6 +338,13 @@ components:
       description: Flag to allow for chart cleanup
       schema:
         type: boolean
+    enable-all:
+      in: query
+      name: enable_all
+      required: false
+      description: Flag to test disabled tests
+      schema:
+        type: boolean
     dry-run:
       in: query
       name: dry_run