Remove argument support for SIMPLE tests

To provide support for passing arguments for simple tests in command
line, we were creating some cases where the test reference could not be
properly handled. Also, for test references with white-spaces, we have
an inconsistent behavior. See reference.

This patch removes the support for arguments in test references,
restoring the proper behavior, which is expected to be shell-like.

As consequence, we now handle correctly white-spaces in tests references
as well as Unicode characters.

Reference: https://www.redhat.com/archives/avocado-devel/2016-November/msg00011.html
Reference: https://trello.com/c/bfE9NBblSigned-off-by: NAmador Pahim <apahim@redhat.com>

avocado/core/loader.py

@@ -23,7 +23,6 @@ import imp
 import inspect
 import os
 import re
-import pipes
 import shlex
 import sys
 
@@ -531,12 +530,6 @@ class FileLoader(TestLoader):
                 subtests_filter = re.compile(_subtests_filter)
 
         if not os.path.isdir(reference):  # Single file
-            if (not self._make_tests(reference, DEFAULT, subtests_filter) and
-                    not subtests_filter):
-                split_reference = shlex.split(reference)
-                if (os.access(split_reference[0], os.X_OK) and
-                        not os.path.isdir(split_reference[0])):
-                    return self._make_test(test.SimpleTest, reference)
             return self._make_tests(reference, which_tests, subtests_filter)
 
         tests = []
@@ -740,7 +733,7 @@ class FileLoader(TestLoader):
             else:
                 if os.access(test_path, os.X_OK):
                     return self._make_test(test.SimpleTest,
-                                           pipes.quote(test_path))
+                                           test_path)
                 else:
                     return make_broken(test.NotATest, test_path)
         else:
@@ -794,10 +787,7 @@ class ExternalLoader(TestLoader):
 
         if runner:
             external_runner_and_args = shlex.split(runner)
-            if len(external_runner_and_args) > 1:
-                executable = external_runner_and_args[0]
-            else:
-                executable = runner
+            executable = external_runner_and_args[0]
             if not os.path.exists(executable):
                 msg = ('Could not find the external runner executable "%s"'
                        % executable)

avocado/core/test.py

@@ -21,6 +21,7 @@ framework tests.
 import inspect
 import logging
 import os
+import pipes
 import re
 import shutil
 import sys
@@ -702,7 +703,9 @@ class SimpleTest(Test):
     def __init__(self, name, params=None, base_logdir=None, job=None):
         super(SimpleTest, self).__init__(name=name, params=params,
                                          base_logdir=base_logdir, job=job)
-        self._command = self.filename
+        self._command = None
+        if self.filename is not None:
+            self._command = pipes.quote(self.filename)
 
     @property
     def filename(self):

selftests/functional/test_basic.py

+# This Python file uses the following encoding: utf-8
 import aexpect
 import glob
 import json
@@ -88,6 +89,7 @@ class MyTest(Test):
          time.sleep(60)
 '''
 
+
 DIE_WITHOUT_REPORTING_STATUS = '''
 from avocado import Test
 import os
@@ -568,8 +570,8 @@ class RunnerHumanOutputTest(unittest.TestCase):
     @unittest.skipIf(not ECHO_BINARY, 'echo binary not available')
     def test_ugly_echo_cmd(self):
         os.chdir(basedir)
-        cmd_line = ('./scripts/avocado run "%s -ne '
-                    'foo\\\\\\n\\\'\\\\\\"\\\\\\nbar/baz" --job-results-dir %s'
+        cmd_line = ('./scripts/avocado run --external-runner "%s -ne" '
+                    '"foo\\\\\\n\\\'\\\\\\"\\\\\\nbar/baz" --job-results-dir %s'
                     ' --sysinfo=off  --show-job-log' %
                     (ECHO_BINARY, self.tmpdir))
         result = process.run(cmd_line, ignore_status=True)
@@ -580,8 +582,8 @@ class RunnerHumanOutputTest(unittest.TestCase):
         self.assertIn('[stdout] foo', result.stdout, result)
         self.assertIn('[stdout] \'"', result.stdout, result)
         self.assertIn('[stdout] bar/baz', result.stdout, result)
-        self.assertIn('PASS 1-%s -ne foo\\\\n\\\'\\"\\\\nbar/baz' %
-                      ECHO_BINARY, result.stdout, result)
+        self.assertIn('PASS 1-foo\\\\n\\\'\\"\\\\nbar/baz',
+                      result.stdout, result)
         # logdir name should escape special chars (/)
         test_dirs = glob.glob(os.path.join(self.tmpdir, 'latest',
                                            'test-results', '*'))
@@ -589,8 +591,7 @@ class RunnerHumanOutputTest(unittest.TestCase):
                          " test-results dir, but only one test was executed: "
                          "%s" % (test_dirs))
         self.assertEqual(os.path.basename(test_dirs[0]),
-                         '1-%s -ne foo\\\\n\\\'\\"\\\\nbar_baz' %
-                         ECHO_BINARY.replace('/', '_'))
+                         '1-foo\\\\n\\\'\\"\\\\nbar_baz')
 
     def test_replay_skip_skipped(self):
         cmd = ("./scripts/avocado run --job-results-dir %s --json - "
@@ -611,7 +612,7 @@ class RunnerSimpleTest(unittest.TestCase):
     def setUp(self):
         self.tmpdir = tempfile.mkdtemp(prefix='avocado_' + __name__)
         self.pass_script = script.TemporaryScript(
-            'avocado_pass.sh',
+            'ʊʋʉʈɑ ʅʛʌ',
             PASS_SCRIPT_CONTENTS,
             'avocado_simpletest_functional')
         self.pass_script.save()
@@ -624,7 +625,7 @@ class RunnerSimpleTest(unittest.TestCase):
     def test_simpletest_pass(self):
         os.chdir(basedir)
         cmd_line = ('./scripts/avocado run --job-results-dir %s --sysinfo=off'
-                    ' %s' % (self.tmpdir, self.pass_script.path))
+                    ' "%s"' % (self.tmpdir, self.pass_script.path))
         result = process.run(cmd_line, ignore_status=True)
         expected_rc = exit_codes.AVOCADO_ALL_OK
         self.assertEqual(result.exit_status, expected_rc,
@@ -704,7 +705,7 @@ class RunnerSimpleTest(unittest.TestCase):
         test_file_name = os.path.basename(self.pass_script.path)
         os.chdir(test_base_dir)
         cmd_line = ('%s run --job-results-dir %s --sysinfo=off'
-                    ' %s' % (avocado_path, self.tmpdir, test_file_name))
+                    ' "%s"' % (avocado_path, self.tmpdir, test_file_name))
         result = process.run(cmd_line, ignore_status=True)
         expected_rc = exit_codes.AVOCADO_ALL_OK
         self.assertEqual(result.exit_status, expected_rc,
@@ -713,10 +714,9 @@ class RunnerSimpleTest(unittest.TestCase):
 
     @unittest.skipIf(not SLEEP_BINARY, 'sleep binary not available')
     def test_kill_stopped_sleep(self):
-        sleep = "'%s 60'" % SLEEP_BINARY
-        proc = aexpect.Expect("./scripts/avocado run %s --job-results-dir %s "
-                              "--sysinfo=off --job-timeout 3"
-                              % (sleep, self.tmpdir))
+        proc = aexpect.Expect("./scripts/avocado run 60 --job-results-dir %s "
+                              "--external-runner %s --sysinfo=off --job-timeout 3"
+                              % (self.tmpdir, SLEEP_BINARY))
         proc.read_until_output_matches(["\(1/1\)"], timeout=3,
                                        internal_timeout=0.01)
         # We need pid of the avocado, not the shell executing it
@@ -739,7 +739,8 @@ class RunnerSimpleTest(unittest.TestCase):
                       "probably died unexpectadly")
         self.assertEqual(proc.get_status(), 8, "Avocado did not finish with "
                          "1.")
-        sleep_dir = astring.string_to_safe_path("1-" + sleep[1:-1])
+
+        sleep_dir = astring.string_to_safe_path("1-60")
         debug_log = os.path.join(self.tmpdir, "latest", "test-results",
                                  sleep_dir, "debug.log")
         debug_log = open(debug_log).read()
@@ -1090,10 +1091,12 @@ class PluginsJSONTest(AbsPluginsTest, unittest.TestCase):
         super(PluginsJSONTest, self).setUp()
 
     def run_and_check(self, testname, e_rc, e_ntests, e_nerrors,
-                      e_nfailures, e_nskip):
+                      e_nfailures, e_nskip, external_runner=None):
         os.chdir(basedir)
         cmd_line = ('./scripts/avocado run --job-results-dir %s --sysinfo=off --json - --archive %s' %
                     (self.tmpdir, testname))
+        if external_runner is not None:
+            cmd_line += " --external-runner '%s'" % external_runner
         result = process.run(cmd_line, ignore_status=True)
         json_output = result.stdout
         self.assertEqual(result.exit_status, e_rc,
@@ -1139,15 +1142,15 @@ class PluginsJSONTest(AbsPluginsTest, unittest.TestCase):
 
     @unittest.skipIf(not ECHO_BINARY, 'echo binary not available')
     def test_ugly_echo_cmd(self):
-        data = self.run_and_check('"/bin/echo -ne foo\\\\\\n\\\'\\\\\\"\\\\\\'
+        data = self.run_and_check('"-ne foo\\\\\\n\\\'\\\\\\"\\\\\\'
                                   'nbar/baz"', exit_codes.AVOCADO_ALL_OK, 1, 0,
-                                  0, 0)
+                                  0, 0, ECHO_BINARY)
         # The executed test should be this
         self.assertEqual(data['tests'][0]['url'],
-                         '1-/bin/echo -ne foo\\\\n\\\'\\"\\\\nbar/baz')
+                         '1--ne foo\\\\n\\\'\\"\\\\nbar/baz')
         # logdir name should escape special chars (/)
         self.assertEqual(os.path.basename(data['tests'][0]['logdir']),
-                         '1-_bin_echo -ne foo\\\\n\\\'\\"\\\\nbar_baz')
+                         '1--ne foo\\\\n\\\'\\"\\\\nbar_baz')
 
     def tearDown(self):
         shutil.rmtree(self.tmpdir)

selftests/functional/test_output.py

@@ -261,8 +261,8 @@ class OutputPluginTest(unittest.TestCase):
                 pass
 
     def test_nonprintable_chars(self):
-        cmd_line = ("./scripts/avocado run '/bin/ls "
-                    "NON_EXISTING_FILE_WITH_NONPRINTABLE_CHARS_IN_HERE\x1b' "
+        cmd_line = ("./scripts/avocado run --external-runner /bin/ls "
+                    "'NON_EXISTING_FILE_WITH_NONPRINTABLE_CHARS_IN_HERE\x1b' "
                     "--job-results-dir %s --sysinfo=off" % self.tmpdir)
         result = process.run(cmd_line, ignore_status=True)
         output = result.stdout + result.stderr

selftests/unit/test_loader.py

@@ -204,11 +204,9 @@ class LoaderTest(unittest.TestCase):
         test_parameters['base_logdir'] = self.tmpdir
         tc = test_class(**test_parameters)
         tc.run_avocado()
-        # Load with params
-        simple_with_params = simple_test.path + " 'foo bar' --baz"
-        suite = self.loader.discover(simple_with_params, loader.ALL)
+        suite = self.loader.discover(simple_test.path, loader.ALL)
         self.assertEqual(len(suite), 1)
-        self.assertEqual(suite[0][1]["name"], simple_with_params)
+        self.assertEqual(suite[0][1]["name"], simple_test.path)
         simple_test.remove()
 
     def test_load_simple_not_exec(self):