[Verify] Adding '--skip-list' arg to `rally verify start` cmd

Sometimes we need to run the full test suite without some tests.
Now the new '--skip-list' argument allows us to do it. In order to skip
the desired tests we just need to list the tests in some file and provide
the '--skip-list' argument with the path to the file:

    rally verify start --skip-list /path/to/the/file

Also, missing messages about incompatibility of arguments were added.

Change-Id: I882bfacd2e39bed2b54af4092752f642c964975a

etc/rally.bash_completion

@@ -58,7 +58,7 @@ _rally()
     OPTS["verify_results"]="--uuid --html --json --output-file"
     OPTS["verify_show"]="--uuid --sort-by --detailed"
     OPTS["verify_showconfig"]="--deployment"
-    OPTS["verify_start"]="--deployment --set --regex --tests-file --tempest-config --xfails-file --no-use --system-wide --concurrency --failing"
+    OPTS["verify_start"]="--deployment --set --regex --tests-file --skip-list --tempest-config --xfails-file --no-use --system-wide --concurrency --failing"
     OPTS["verify_uninstall"]="--deployment"
     OPTS["verify_use"]="--uuid"
 

rally/api.py

@@ -381,15 +381,18 @@ class Verification(object):
 
     @classmethod
     def verify(cls, deployment, set_name="", regex=None, tests_file=None,
-               tempest_config=None, expected_failures=None, system_wide=False,
-               concur=0, failing=False):
+               tests_file_to_skip=None, tempest_config=None,
+               expected_failures=None, system_wide=False, concur=0,
+               failing=False):
         """Start verification.
 
-        :param deployment: UUID or name of a deployment
         :param deployment: UUID or name of a deployment
         :param set_name: Name of a Tempest test set
         :param regex: Regular expression of test
         :param tests_file: Path to a file with a list of Tempest tests
+                           to run them
+        :param tests_file_to_skip: Path to a file with a list of Tempest tests
+                                   to skip them
         :param tempest_config: User specified Tempest config file location
         :param expected_failures: Dictionary with Tempest tests that are
                                   expected to fail. Keys are test names;
@@ -400,8 +403,7 @@ class Verification(object):
                             env when running the tests
         :param concur: How many processes to use to run Tempest tests.
                        The default value (0) auto-detects CPU count
-        :param failing: Re-run tests that failed during the last
-                        execution
+        :param failing: Re-run tests that failed during the last execution
         :returns: Verification object
         """
 
@@ -417,6 +419,7 @@ class Verification(object):
         LOG.info("Starting verification of deployment: %s" % deployment_uuid)
         verification.set_running()
         verifier.verify(set_name=set_name, regex=regex, tests_file=tests_file,
+                        tests_file_to_skip=tests_file_to_skip,
                         expected_failures=expected_failures, concur=concur,
                         failing=failing)
 

rally/cli/commands/verify.py

@@ -54,14 +54,19 @@ class VerifyCommands(object):
                    help="Test name regular expression")
     @cliutils.args("--tests-file", metavar="<path>", dest="tests_file",
                    type=str, required=False,
-                   help="Path to a file with a list of Tempest tests")
+                   help="Path to a file with a list of Tempest tests "
+                        "to run them")
+    @cliutils.args("--skip-list", metavar="<path>", dest="tests_file_to_skip",
+                   type=str, required=False,
+                   help="Path to a file with a list of Tempest tests "
+                        "to skip them")
     @cliutils.args("--tempest-config", dest="tempest_config", type=str,
                    required=False, metavar="<path>",
                    help="User-specified Tempest config file location")
     @cliutils.args("--xfails-file", dest="xfails_file", type=str,
                    required=False, metavar="<path>",
-                   help="Path to a YAML file with a list of Tempest "
-                        "tests that are expected to fail")
+                   help="Path to a YAML file with a list of Tempest tests "
+                        "that are expected to fail")
     @cliutils.args("--no-use", action="store_false", dest="do_use",
                    help="Don't set the task as default for future operations")
     @cliutils.args("--system-wide", dest="system_wide",
@@ -79,8 +84,8 @@ class VerifyCommands(object):
                    help="Re-run the tests that failed in the last execution",
                    action="store_true")
     @envutils.with_default_deployment(cli_arg_name="deployment")
-    def start(self, deployment=None, set_name="", regex=None,
-              tests_file=None, tempest_config=None, xfails_file=None,
+    def start(self, deployment=None, set_name="", regex=None, tests_file=None,
+              tests_file_to_skip=None, tempest_config=None, xfails_file=None,
               do_use=True, system_wide=False, concur=0, failing=False):
         """Start verification (run Tempest tests).
 
@@ -88,9 +93,12 @@ class VerifyCommands(object):
         :param set_name: Name of a Tempest test set
         :param regex: Regular expression of test
         :param tests_file: Path to a file with a list of Tempest tests
+                           to run them
+        :param tests_file_to_skip: Path to a file with a list of Tempest tests
+                                   to skip them
         :param tempest_config: User specified Tempest config file location
-        :param xfails_file: Path to a file in YAML format with a list of
-                            Tempest tests that are expected to fail
+        :param xfails_file: Path to a YAML file with a list of Tempest tests
+                            that are expected to fail
         :param do_use: Use new task as default for future operations
         :param system_wide: Whether or not to create a virtual env when
                             installing Tempest; whether or not to use
@@ -101,21 +109,26 @@ class VerifyCommands(object):
         :param failing: Re-run tests that failed during the last execution
         """
 
-        msg = _("Arguments '%s' and '%s' are not compatible. "
+        msg = _("Arguments '%s' and '%s' are incompatible. "
                 "You can use only one of the mentioned arguments.")
-        if regex and set_name:
-            print(msg % ("regex", "set"))
-            return 1
-        if tests_file and set_name:
-            print(msg % ("tests_file", "set"))
-            return 1
-        if tests_file and regex:
-            print(msg % ("tests_file", "regex"))
-            return 1
+        incompatible_args_map = [
+            {"regex": regex, "set": set_name},
+            {"tests-file": tests_file, "set": set_name},
+            {"tests-file": tests_file, "regex": regex},
+            {"tests-file": tests_file, "skip-list": tests_file_to_skip},
+            {"failing": failing, "set": set_name},
+            {"failing": failing, "regex": regex},
+            {"failing": failing, "tests-file": tests_file},
+            {"failing": failing, "skip-list": tests_file_to_skip}
+        ]
+        for args in incompatible_args_map:
+            arg_keys = list(args)
+            if args[arg_keys[0]] and args[arg_keys[1]]:
+                print(msg % (arg_keys[0], arg_keys[1]))
+                return 1
 
         if not (regex or set_name or tests_file or failing):
             set_name = "full"
-
         if set_name and set_name not in AVAILABLE_SETS:
             print(_("Tempest test set '%s' not found "
                     "in available test sets. Available sets are %s.")
@@ -125,13 +138,8 @@ class VerifyCommands(object):
         if tests_file and not os.path.exists(tests_file):
             print(_("File '%s' not found.") % tests_file)
             return 1
-
-        if failing and set_name:
-            print(msg % ("failing", "set"))
-            return 1
-
-        if failing and tests_file:
-            print(msg % ("failing", "tests_file"))
+        if tests_file_to_skip and not os.path.exists(tests_file_to_skip):
+            print(_("File '%s' not found.") % tests_file_to_skip)
             return 1
 
         expected_failures = None
@@ -144,7 +152,8 @@ class VerifyCommands(object):
                 return 1
 
         verification = api.Verification.verify(
-            deployment, set_name=set_name, regex=regex, tests_file=tests_file,
+            deployment, set_name=set_name, regex=regex,
+            tests_file=tests_file, tests_file_to_skip=tests_file_to_skip,
             tempest_config=tempest_config, expected_failures=expected_failures,
             system_wide=system_wide, concur=concur, failing=failing)
 

rally/verification/tempest/tempest.py

@@ -312,7 +312,8 @@ class Tempest(object):
         LOG.info(_("Tempest plugin has been successfully installed!"))
 
     @logging.log_verification_wrapper(LOG.info, _("Run verification."))
-    def _prepare_and_run(self, set_name, regex, tests_file, concur, failing):
+    def _prepare_and_run(self, set_name, regex, tests_file,
+                         tests_file_to_skip, concur, failing):
         if not self.is_configured():
             self.generate_config_file()
 
@@ -333,6 +334,18 @@ class Tempest(object):
         elif tests_file:
             testr_args = "--load-list %s" % os.path.abspath(tests_file)
 
+        if tests_file_to_skip and not tests_file:
+            tests_to_run = set(self.discover_tests(testr_args))
+            with open(os.path.abspath(tests_file_to_skip), "rb") as f:
+                tests_to_skip = set([line.strip() for line in f])
+            tests_to_run -= tests_to_skip
+
+            temp_file = tempfile.NamedTemporaryFile()
+            with open(temp_file.name, "wb") as f:
+                f.writelines("\n".join(tests_to_run))
+
+            testr_args = "--load-list %s" % temp_file.name
+
         self.verification.start_verifying(set_name)
         try:
             self.run(testr_args, concur=concur)
@@ -415,9 +428,10 @@ class Tempest(object):
         else:
             self.verification.set_failed()
 
-    def verify(self, set_name, regex, tests_file, expected_failures, concur,
-               failing):
-        self._prepare_and_run(set_name, regex, tests_file, concur, failing)
+    def verify(self, set_name, regex, tests_file,
+               tests_file_to_skip, expected_failures, concur, failing):
+        self._prepare_and_run(set_name, regex, tests_file,
+                              tests_file_to_skip, concur, failing)
         self._save_results(expected_failures=expected_failures)
 
     def import_results(self, set_name, log_file):

tests/unit/cli/commands/test_verify.py

@@ -57,8 +57,8 @@ class VerifyCommandsTestCase(test.TestCase):
 
         mock_verification_verify.assert_called_once_with(
             deployment_id, set_name="full", regex=None, tests_file=None,
-            tempest_config=None, expected_failures=None,
-            system_wide=False, concur=0, failing=False)
+            tests_file_to_skip=None, tempest_config=None,
+            expected_failures=None, system_wide=False, concur=0, failing=False)
 
     @mock.patch("rally.osclients.Clients")
     @mock.patch("rally.api.Verification.verify")
@@ -75,8 +75,8 @@ class VerifyCommandsTestCase(test.TestCase):
 
         mock_verification_verify.assert_called_once_with(
             deployment_id, set_name="full", regex=None, tests_file=None,
-            tempest_config=tempest_config.name, expected_failures=None,
-            system_wide=False, concur=0, failing=False)
+            tests_file_to_skip=None, tempest_config=tempest_config.name,
+            expected_failures=None, system_wide=False, concur=0, failing=False)
         tempest_config.close()
 
     @mock.patch("rally.api.Verification.verify")
@@ -90,8 +90,8 @@ class VerifyCommandsTestCase(test.TestCase):
 
         mock_verification_verify.assert_called_once_with(
             deployment_id, set_name="", regex=None, tests_file=tests_file,
-            tempest_config=None, expected_failures=None, system_wide=False,
-            concur=0, failing=False)
+            tests_file_to_skip=None, tempest_config=None,
+            expected_failures=None, system_wide=False, concur=0, failing=False)
 
     @mock.patch("rally.api.Verification.verify")
     @mock.patch("six.moves.builtins.open",
@@ -106,8 +106,9 @@ class VerifyCommandsTestCase(test.TestCase):
 
         mock_verification_verify.assert_called_once_with(
             deployment_id, set_name="full", regex=None, tests_file=None,
-            tempest_config=None, expected_failures={"test": "reason of fail"},
-            system_wide=False, concur=0, failing=False)
+            tests_file_to_skip=None, tempest_config=None,
+            expected_failures={"test": "reason of fail"}, system_wide=False,
+            concur=0, failing=False)
 
     @mock.patch("rally.api.Verification.verify")
     def test_start_with_wrong_set_name(self, mock_verification_verify):
@@ -121,6 +122,30 @@ class VerifyCommandsTestCase(test.TestCase):
                          consts.TempestTestsAPI)
         self.assertFalse(mock_verification_verify.called)
 
+    @mock.patch("rally.api.Verification.verify")
+    def test_start_with_set_name_and_regex(self, mock_verification_verify):
+        deployment_id = "2856e214-90d1-4d82-9402-dd13973ca0f6"
+
+        set_name = "identity"
+        regex = "tempest.api.compute"
+        self.verify.start(set_name=set_name, regex=regex,
+                          deployment=deployment_id, do_use=False)
+
+        self.assertFalse(mock_verification_verify.called)
+
+    @mock.patch("rally.api.Verification.verify")
+    def test_start_with_tests_file_and_tests_file_to_skip(
+            self, mock_verification_verify):
+        deployment_id = "3580e214-90d1-4d82-9402-dd13973ca0f6"
+
+        tests_file = "/path/to/tests/file-1"
+        tests_file_to_skip = "/path/to/tests/file-2"
+        self.verify.start(tests_file=tests_file,
+                          tests_file_to_skip=tests_file_to_skip,
+                          deployment=deployment_id, do_use=False)
+
+        self.assertFalse(mock_verification_verify.called)
+
     @mock.patch("rally.api.Verification.verify")
     def test_start_with_failing_and_set_name(self, mock_verification_verify):
         deployment_id = "f2009aae-6ef3-468e-96b2-3c987d584010"
@@ -131,6 +156,16 @@ class VerifyCommandsTestCase(test.TestCase):
 
         self.assertFalse(mock_verification_verify.called)
 
+    @mock.patch("rally.api.Verification.verify")
+    def test_start_with_failing_and_regex(self, mock_verification_verify):
+        deployment_id = "25d19aec-f39e-459e-b3d5-24a718e92233"
+
+        regex = "tempest.api.compute"
+        self.verify.start(regex=regex, deployment=deployment_id, do_use=False,
+                          failing=True)
+
+        self.assertFalse(mock_verification_verify.called)
+
     @mock.patch("rally.api.Verification.verify")
     @mock.patch("os.path.exists", return_value=True)
     def test_start_with_failing_and_test_files(self, mock_exists,
@@ -143,6 +178,18 @@ class VerifyCommandsTestCase(test.TestCase):
 
         self.assertFalse(mock_verification_verify.called)
 
+    @mock.patch("rally.api.Verification.verify")
+    @mock.patch("os.path.exists", return_value=True)
+    def test_start_with_failing_and_test_files_to_skip(
+            self, mock_exists, mock_verification_verify):
+        deployment_id = "7902051d-8286-4cfc-aec5-addde73b3a1f"
+
+        tests_file = "/path/to/tests/file"
+        self.verify.start(tests_file_to_skip=tests_file,
+                          deployment=deployment_id, do_use=False, failing=True)
+
+        self.assertFalse(mock_verification_verify.called)
+
     @mock.patch("rally.api.Verification.import_results")
     def test_import_results(self, mock_verification_import_results):
         deployment_id = "fake_deployment_uuid"

tests/unit/test_api.py

@@ -427,7 +427,8 @@ class VerificationAPITestCase(BaseDeploymentTestCase):
 
         self.tempest.verify.assert_called_once_with(
             set_name="smoke", regex=None, tests_file=None,
-            expected_failures=None, concur=0, failing=False)
+            tests_file_to_skip=None, expected_failures=None,
+            concur=0, failing=False)
 
     @mock.patch("os.path.exists", return_value=True)
     @mock.patch("rally.api.objects.Deployment.get")
@@ -444,7 +445,8 @@ class VerificationAPITestCase(BaseDeploymentTestCase):
 
         self.tempest.verify.assert_called_once_with(
             set_name="", regex=None, tests_file=tests_file,
-            expected_failures=None, concur=0, failing=False)
+            tests_file_to_skip=None, expected_failures=None,
+            concur=0, failing=False)
 
     @mock.patch("rally.api.objects.Deployment.get")
     @mock.patch("rally.api.objects.Verification")

tests/unit/verification/test_tempest.py

@@ -358,7 +358,7 @@ class TempestVerifyTestCase(BaseTestCase):
     @mock.patch(TEMPEST_PATH + ".config.TempestConfig")
     def test_verify_no_tempest_config_exists(self, mock_tempest_config):
         self.assertRaises(exceptions.NotFoundException, self.verifier.verify,
-                          "compute", None, None, None, 0, False)
+                          "compute", None, None, None, None, 0, False)
 
     @mock.patch(TEMPEST_PATH + ".tempest.Tempest.parse_results",
                 return_value=None)
@@ -372,7 +372,7 @@ class TempestVerifyTestCase(BaseTestCase):
         set_name = "identity"
         fake_call = self._get_fake_call("tempest.api.%s" % set_name)
 
-        self.verifier.verify(set_name, None, None, None, 0, False)
+        self.verifier.verify(set_name, None, None, None, None, 0, False)
         self.verifier.verification.start_verifying.assert_called_once_with(
             set_name)
 
@@ -394,7 +394,7 @@ class TempestVerifyTestCase(BaseTestCase):
         fake_call = self._get_fake_call("tempest.api.%s" % set_name)
         mock_subprocess.side_effect = subprocess.CalledProcessError
 
-        self.verifier.verify(set_name, None, None, None, 0, False)
+        self.verifier.verify(set_name, None, None, None, None, 0, False)
         self.verifier.verification.start_verifying.assert_called_once_with(
             set_name)
 
@@ -416,7 +416,32 @@ class TempestVerifyTestCase(BaseTestCase):
         tests_file = "/path/to/tests/file"
         fake_call = self._get_fake_call("--load-list %s" % tests_file)
 
-        self.verifier.verify("", None, tests_file, None, 0, False)
+        self.verifier.verify("", None, tests_file, None, None, 0, False)
+        self.verifier.verification.start_verifying.assert_called_once_with("")
+
+        mock_subprocess.check_call.assert_called_once_with(
+            fake_call, env=mock_tempest_env, cwd=self.verifier.path(),
+            shell=True)
+        mock_tempest_parse_results.assert_called_once_with(None, None)
+
+    @mock.patch(TEMPEST_PATH + ".tempest.Tempest.parse_results",
+                return_value=None)
+    @mock.patch(TEMPEST_PATH + ".tempest.Tempest.env")
+    @mock.patch(TEMPEST_PATH + ".tempest.subprocess")
+    @mock.patch(TEMPEST_PATH + ".config.TempestResourcesContext")
+    @mock.patch("os.path.isfile", return_value=True)
+    @mock.patch("tempfile.NamedTemporaryFile", return_value=mock.MagicMock())
+    @mock.patch("six.moves.builtins.open", side_effect=mock.mock_open())
+    def test_verify_tests_file_to_skip_specified(
+            self, mock_open, mock_named_temporary_file, mock_isfile,
+            mock_tempest_resources_context, mock_subprocess, mock_tempest_env,
+            mock_tempest_parse_results):
+        mock_named_temporary_file.return_value.name = "some-file-name"
+        fake_call = self._get_fake_call("--load-list some-file-name")
+
+        tests_file_to_skip = "/path/to/tests/file"
+        self.verifier.verify(
+            "", None, None, tests_file_to_skip, None, 0, False)
         self.verifier.verification.start_verifying.assert_called_once_with("")
 
         mock_subprocess.check_call.assert_called_once_with(
@@ -437,7 +462,7 @@ class TempestVerifyTestCase(BaseTestCase):
         fake_call = self._get_fake_call(
             "tempest.api.%s" % set_name, "--concurrency 1")
 
-        self.verifier.verify("identity", None, None, None, 1, False)
+        self.verifier.verify("identity", None, None, None, None, 1, False)
         self.verifier.verification.start_verifying.assert_called_once_with(
             set_name)
 
@@ -458,7 +483,7 @@ class TempestVerifyTestCase(BaseTestCase):
         set_name = "identity"
         fake_call = self._get_fake_call("tempest.api.%s" % set_name)
 
-        self.verifier.verify("identity", None, None, None, 0, False)
+        self.verifier.verify("identity", None, None, None, None, 0, False)
         self.verifier.verification.start_verifying.assert_called_once_with(
             set_name)
 
@@ -478,7 +503,7 @@ class TempestVerifyTestCase(BaseTestCase):
                                       mock_subprocess, mock_tempest_env,
                                       mock_tempest_parse_results):
         fake_call = self._get_fake_call("--failing")
-        self.verifier.verify("", None, None, None, 0, True)
+        self.verifier.verify("", None, None, None, None, 0, True)
 
         self.verifier.verification.start_verifying.assert_called_once_with(
             "re-run-failed")