avocado/core/test.py: rename TestName to TestID

The current TestName class records more than a test name (according
to the Avocado specification).  It records, and by default, presents
the Test ID (__str__() and __repr__()).

The Test Name, again according to the Avocado specifications, is
contained within a Test ID, so it's not necessary to have two
different classes IMO.

Finally, the str_filesystem() method was adapted to be coherent with
the fact that it returns a file system representation of the Test ID,
not the Test Name.  It also became a property, to match the same use
of str_uid and str_variant.
Signed-off-by: NCleber Rosa <crosa@redhat.com>

avocado/core/runner.py

@@ -580,9 +580,9 @@ class TestRunner(object):
                 index += 1
                 test_parameters = test_factory[1]
                 name = test_parameters.get("name")
-                test_parameters["name"] = test.TestName(index + 1, name,
-                                                        variant,
-                                                        no_digits)
+                test_parameters["name"] = test.TestID(index + 1, name,
+                                                      variant,
+                                                      no_digits)
                 if deadline is not None and time.time() > deadline:
                     summary.add('INTERRUPTED')
                     if 'methodName' in test_parameters:

avocado/core/test.py

@@ -52,19 +52,24 @@ from .output import LOG_JOB
 COMMON_TMPDIR_NAME = 'AVOCADO_TESTS_COMMON_TMPDIR'
 
 
-class TestName(object):
+class TestID(object):
 
     """
-    Test name representation
+    Test ID construction and representation according to specification
+
+    This class wraps the representation of both Avocado's Test ID
+    specification and Avocado's Test Name, which is part of a Test ID.
     """
 
     def __init__(self, uid, name, variant=None, no_digits=None):
         """
-        Test name according to avocado specification
+        Constructs a TestID instance
 
         :param uid: unique test id (within the job)
-        :param name: test name (identifies the executed test)
-        :param variant: variant id
+        :param name: test name, as returned by the Avocado test resolver
+                     (AKA as test loader)
+        :param variant: the variant applied to this Test ID
+        :type variant: dict
         :param no_digits: number of digits of the test uid
         """
         self.uid = uid
@@ -92,26 +97,39 @@ class TestName(object):
         else:
             return self.__dict__ == other.__dict__
 
+    @property
     def str_filesystem(self):
         """
-        File-system friendly representation of the test name
+        Test ID in a format suitable for use in file systems
+
+        The string returned should be safe to be used as a file or
+        directory name.  This file system version of the test ID may
+        have to shorten either the Test Name or the Variant ID.
+
+        The first component of a Test ID, the numeric unique test id,
+        AKA "uid", will be used as a an stable identifier between the
+        Test ID and the file or directory created based on the return
+        value of this method.  If the filesystem can not even
+        represent the "uid", than an exception will be raised.
+
+        For Test ID "001-mytest;foo", examples of shortened file
+        system versions include "001-mytest;f" or "001-myte;foo".
+
+        :raises: AssertionError
         """
-        name = str(self)
-        fsname = astring.string_to_safe_path(name)
-        if len(name) == len(fsname):    # everything fits in
-            return fsname
-        # 001-mytest;foo
-        # 001-mytest;f
-        # 001-myte;foo
-        idx_fit_variant = len(fsname) - len(self.str_variant)
+        test_id = str(self)
+        test_id_fs = astring.string_to_safe_path(test_id)
+        if len(test_id) == len(test_id_fs):    # everything fits in
+            return test_id_fs
+        idx_fit_variant = len(test_id_fs) - len(self.str_variant)
         if idx_fit_variant > len(self.str_uid):     # full uid+variant
-            return (fsname[:idx_fit_variant] +
+            return (test_id_fs[:idx_fit_variant] +
                     astring.string_to_safe_path(self.str_variant))
-        elif len(self.str_uid) <= len(fsname):   # full uid
+        elif len(self.str_uid) <= len(test_id_fs):   # full uid
             return astring.string_to_safe_path(self.str_uid + self.str_variant)
         else:       # not even uid could be stored in fs
-            raise AssertionError("Test uid is too long to be stored on the "
-                                 "filesystem: %s\nFull test name is %s"
+            raise AssertionError('Test uid is too long to be stored on the '
+                                 'filesystem: "%s"\nFull Test ID: "%s"'
                                  % (self.str_uid, str(self)))
 
 
@@ -150,7 +168,7 @@ class Test(unittest.TestCase):
                      written with the avocado API, this should not be
                      set.  This is reserved for internal Avocado use,
                      such as when running random executables as tests.
-        :type name: :class:`avocado.core.test.TestName`
+        :type name: :class:`avocado.core.test.TestID`
         :param base_logdir: Directory where test logs should go. If None
                             provided, it'll use
                             :func:`avocado.data_dir.create_job_logs_dir`.
@@ -165,7 +183,7 @@ class Test(unittest.TestCase):
         if name is not None:
             self.__name = name
         else:
-            self.__name = TestName(0, self.__class__.__name__)
+            self.__name = TestID(0, self.__class__.__name__)
 
         self.__job = job
 
@@ -181,7 +199,7 @@ class Test(unittest.TestCase):
         if base_logdir is None:
             base_logdir = data_dir.create_job_logs_dir()
         base_logdir = os.path.join(base_logdir, 'test-results')
-        logdir = os.path.join(base_logdir, self.name.str_filesystem())
+        logdir = os.path.join(base_logdir, self.name.str_filesystem)
         if os.path.exists(logdir):
             raise exceptions.TestSetupFail("Log dir already exists, this "
                                            "should never happen: %s"
@@ -248,7 +266,9 @@ class Test(unittest.TestCase):
     @property
     def name(self):
         """
-        The test name (TestName instance)
+        Returns the Test ID, which includes the test name
+
+        :rtype: TestID
         """
         return self.__name
 

docs/source/TestParameters.rst

@@ -102,7 +102,7 @@ AvocadoParams
 Is a "database" of params present in every (instrumented) avocado test.
 It's produced during :class:`avocado.core.test.Test`'s ``__init__``
 from a `variant`_. It accepts a list of `TreeNode`_ objects; test name
-:class:`avocado.core.test.TestName` (for logging purposes); list of
+:class:`avocado.core.test.TestID` (for logging purposes); list of
 default paths (`Mux path`_) and the `Test's default params`_.
 
 In test it allows querying for data by using::

optional_plugins/runner_remote/avocado_runner_remote/__init__.py

@@ -37,7 +37,7 @@ from avocado.core.output import LOG_JOB, LOG_UI
 from avocado.core.plugin_interfaces import CLI
 from avocado.core.runner import TestRunner
 from avocado.core.settings import settings
-from avocado.core.test import TestName
+from avocado.core.test import TestID
 from avocado.utils import archive
 from avocado.utils import astring
 from avocado.utils import process
@@ -507,7 +507,7 @@ class RemoteTestRunner(TestRunner):
                 name = [name[0]] + name[1].split(';')
                 if len(name) == 3:
                     name[2] = {"variant_id": name[2]}
-                name = TestName(*name, no_digits=-1)
+                name = TestID(*name, no_digits=-1)
                 state = dict(name=name,
                              time_elapsed=tst['time'],
                              time_start=tst['start'],

selftests/unit/test_loader.py

@@ -253,7 +253,7 @@ class LoaderTest(unittest.TestCase):
         test_class, test_parameters = (
             self.loader.discover(simple_test.path, loader.ALL)[0])
         self.assertTrue(test_class == test.SimpleTest, test_class)
-        test_parameters['name'] = test.TestName(0, test_parameters['name'])
+        test_parameters['name'] = test.TestID(0, test_parameters['name'])
         test_parameters['base_logdir'] = self.tmpdir
         tc = test_class(**test_parameters)
         tc.run_avocado()
@@ -299,7 +299,7 @@ class LoaderTest(unittest.TestCase):
         test_class, test_parameters = (
             self.loader.discover(avocado_not_a_test.path, loader.ALL)[0])
         self.assertTrue(test_class == test.SimpleTest, test_class)
-        test_parameters['name'] = test.TestName(0, test_parameters['name'])
+        test_parameters['name'] = test.TestID(0, test_parameters['name'])
         test_parameters['base_logdir'] = self.tmpdir
         tc = test_class(**test_parameters)
         # The test can't be executed (no shebang), raising an OSError
@@ -315,7 +315,7 @@ class LoaderTest(unittest.TestCase):
         test_class, test_parameters = (
             self.loader.discover(avocado_simple_test.path, loader.ALL)[0])
         self.assertTrue(test_class == test.SimpleTest)
-        test_parameters['name'] = test.TestName(0, test_parameters['name'])
+        test_parameters['name'] = test.TestID(0, test_parameters['name'])
         test_parameters['base_logdir'] = self.tmpdir
         tc = test_class(**test_parameters)
         tc.run_avocado()

selftests/unit/test_test.py

@@ -33,7 +33,7 @@ class TestClassTestUnit(unittest.TestCase):
     def test_ugly_name(self):
         def run(name, path_name):
             """ Initialize test and check the dirs were created """
-            tst = self.DummyTest("test", test.TestName(1, name),
+            tst = self.DummyTest("test", test.TestID(1, name),
                                  base_logdir=self.tmpdir)
             self.assertEqual(os.path.basename(tst.logdir), path_name)
             self.assertTrue(os.path.exists(tst.logdir))
@@ -58,7 +58,7 @@ class TestClassTestUnit(unittest.TestCase):
 
     def test_long_name(self):
         def check(uid, name, variant, exp_logdir):
-            tst = self.DummyTest("test", test.TestName(uid, name, variant),
+            tst = self.DummyTest("test", test.TestID(uid, name, variant),
                                  base_logdir=self.tmpdir)
             self.assertEqual(os.path.basename(tst.logdir), exp_logdir)
             return tst
@@ -97,7 +97,7 @@ class TestClassTestUnit(unittest.TestCase):
         flexmock(os.path)
         os.path.should_receive('exists').and_return(True)
         self.assertRaises(exceptions.TestSetupFail, self.DummyTest, "test",
-                          test.TestName(1, "name"), base_logdir=self.tmpdir)
+                          test.TestID(1, "name"), base_logdir=self.tmpdir)
 
     def test_try_override_test_variable(self):
         test = self.DummyTest(base_logdir=self.tmpdir)
@@ -166,12 +166,12 @@ class SimpleTestClassTest(unittest.TestCase):
         self.fail_script.save()
 
         self.tst_instance_pass = test.SimpleTest(
-            name=test.TestName(1, self.pass_script.path),
+            name=test.TestID(1, self.pass_script.path),
             base_logdir=self.tmpdir)
         self.tst_instance_pass.run_avocado()
 
         self.tst_instance_fail = test.SimpleTest(
-            name=test.TestName(1, self.fail_script.path),
+            name=test.TestID(1, self.fail_script.path),
             base_logdir=self.tmpdir)
         self.tst_instance_fail.run_avocado()
 
@@ -196,14 +196,14 @@ class MockingTest(unittest.TestCase):
         # No params
         self.tests.append(test.MockingTest())
         # Positional
-        self.tests.append(test.MockingTest("test", test.TestName(1, "my_name"),
+        self.tests.append(test.MockingTest("test", test.TestID(1, "my_name"),
                                            {}, None, "1",
                                            None, None, "extra_param1",
                                            "extra_param2"))
         self.assertEqual(self.tests[-1].name, "1-my_name")
         # Kwargs
         self.tests.append(test.MockingTest(methodName="test",
-                                           name=test.TestName(1, "my_name2"),
+                                           name=test.TestID(1, "my_name2"),
                                            params={}, base_logdir=None,
                                            tag="a", job=None, runner_queue=None,
                                            extra1="extra_param1",
@@ -215,14 +215,14 @@ class MockingTest(unittest.TestCase):
                                            None, None, "extra_param1",
                                            "extra_param2",
                                            methodName="test",
-                                           name=test.TestName(1, "my_name3"),
+                                           name=test.TestID(1, "my_name3"),
                                            params={}, base_logdir=None,
                                            tag="3", job=None, runner_queue=None,
                                            extra1="extra_param3",
                                            extra2="extra_param4"))
         self.assertEqual(self.tests[-1].name, "1-my_name3")
         # combination
-        self.tests.append(test.MockingTest("test", test.TestName(1, "my_name4"),
+        self.tests.append(test.MockingTest("test", test.TestID(1, "my_name4"),
                                            tag="321",
                                            other_param="Whatever"))
         self.assertEqual(self.tests[-1].name, "1-my_name4")
@@ -232,7 +232,7 @@ class MockingTest(unittest.TestCase):
         # ones.
         name = "positional_method_name_becomes_test_name"
         tag = "positional_base_logdir_becomes_tag"
-        self.tests.append(test.MockingTest(test.TestName(1, name), None, None, tag,
+        self.tests.append(test.MockingTest(test.TestID(1, name), None, None, tag,
                                            methodName="test",
                                            other_param="Whatever"))
         self.assertEqual(self.tests[-1].name, "1-" + name)
@@ -245,5 +245,81 @@ class MockingTest(unittest.TestCase):
                 pass
 
 
+class TestID(unittest.TestCase):
+
+    def test_uid_name(self):
+        uid = 1
+        name = 'file.py:klass.test_method'
+        test_id = test.TestID(uid, name)
+        self.assertEqual(test_id.uid, 1)
+        self.assertEqual(test_id.str_uid, '1')
+        self.assertEqual(test_id.str_filesystem, '%s-%s' % (uid, name))
+        self.assertIs(test_id.variant, None)
+        self.assertIs(test_id.str_variant, '')
+
+    def test_uid_name_no_digits(self):
+        uid = 1
+        name = 'file.py:klass.test_method'
+        test_id = test.TestID(uid, name, no_digits=2)
+        self.assertEqual(test_id.uid, 1)
+        self.assertEqual(test_id.str_uid, '01')
+        self.assertEqual(test_id.str_filesystem, '%s-%s' % ('01', name))
+        self.assertIs(test_id.variant, None)
+        self.assertIs(test_id.str_variant, '')
+
+    def test_uid_name_large_digits(self):
+        """
+        Tests that when the filesystem can only cope with the size of
+        the Test ID, that's the only thing that will be kept.
+        """
+        uid = 1
+        name = 'test'
+        test_id = test.TestID(uid, name, no_digits=255)
+        self.assertEqual(test_id.uid, 1)
+        self.assertEqual(test_id.str_uid, '%0255i' % uid)
+        self.assertEqual(test_id.str_filesystem, '%0255i' % uid)
+        self.assertIs(test_id.variant, None)
+        self.assertIs(test_id.str_variant, '')
+
+    def test_uid_name_uid_too_large_digitis(self):
+        """
+        Tests that when the filesystem can not cope with the size of
+        the Test ID, not even the test uid, an exception will be
+        raised.
+        """
+        test_id = test.TestID(1, 'test', no_digits=256)
+        self.assertRaises(AssertionError, lambda: test_id.str_filesystem)
+
+    def test_uid_large_name(self):
+        """
+        Tests that when the filesystem can not cope with the size of
+        the Test ID, the name will be shortened.
+        """
+        uid = 1
+        name = 'test_' * 51     # 255 characters
+        test_id = test.TestID(uid, name)
+        self.assertEqual(test_id.uid, 1)
+        # only 253 can fit for the test name
+        self.assertEqual(test_id.str_filesystem, '%s-%s' % (uid, name[:253]))
+        self.assertIs(test_id.variant, None)
+        self.assertIs(test_id.str_variant, "")
+
+    def test_uid_name_large_variant(self):
+        """
+        Tests that when the filesystem can not cope with the size of
+        the Test ID, and a variant name is present, the name will be
+        removed.
+        """
+        uid = 1
+        name = 'test'
+        variant_id = 'fast_' * 51    # 255 characters
+        variant = {'variant_id': variant_id}
+        test_id = test.TestID(uid, name, variant=variant)
+        self.assertEqual(test_id.uid, 1)
+        self.assertEqual(test_id.str_filesystem, '%s;%s' % (uid, variant_id[:253]))
+        self.assertIs(test_id.variant, variant_id)
+        self.assertEqual(test_id.str_variant, ";%s" % variant_id)
+
+
 if __name__ == '__main__':
     unittest.main()