[Piglit] [RFC 08/30] profile.py: Lower all test keys before getting and setting.

Dylan Baker baker.dylan.c at gmail.com
Tue Jan 27 14:58:42 PST 2015


One of the problems for piglit is that it is a cross platform system, and that
it is useful to think about the file hierarchy and the group hierarchy as
equivalent. This allows auto-discovery of tests by doing a file system search,
which requires very little code and makes developers lives easy.

This is very easy on Linux and other systems with sane file
systems, because python has the same view of strings as these systems do of
paths. a == a, but a != A. To avert problems with case sensitivity files
generally are lower case, while in all.py they have mirrored the case of
the functionality they test, whether that be a function name or an
extension name.

This is problematic because it means that if there are tests for
ARB_ham_sandwich that are automatically discovered, and those that are
added by all.py they will end up in two separate groups
(ARB_ham_sandwhich and arb_ham_sandwich), and thus be spread across the
html summary on sane systems.

This patch adds another class, TestDict, which is, like Tree, derived
from dict, and provides a __getitem__, __setitem__ and __delitem__
method, which call str.lower() on the key input before actually getting,
setting, or deleting the key/value pair. They key is also forced to be a
string, while the value is forced to be a Test derived class.

Tree is, in turn, made a subclass of TestDict, providing the same
functionality for TestProfile.tests and TestProfile.test_list.

Finally, this change necessitates bumping the results version, and
adding all of the boilerplate that goes along with that.

Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
 framework/backends/json.py          |   2 +-
 framework/profile.py                |  43 ++++++++++-----
 framework/results.py                |  14 +++++
 framework/tests/profile_tests.py    |  19 ++++---
 framework/tests/results_v2_tests.py | 103 ++++++++++++++++++++++++++++++++++++
 5 files changed, 160 insertions(+), 21 deletions(-)
 create mode 100644 framework/tests/results_v2_tests.py

diff --git a/framework/backends/json.py b/framework/backends/json.py
index 7ba3452..23a5b28 100644
--- a/framework/backends/json.py
+++ b/framework/backends/json.py
@@ -40,7 +40,7 @@ __all__ = [
 
 
 # The current version of the JSON results
-CURRENT_JSON_VERSION = 2
+CURRENT_JSON_VERSION = 3
 
 
 def piglit_encoder(obj):
diff --git a/framework/profile.py b/framework/profile.py
index a1db8e0..a24b4fe 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -46,24 +46,24 @@ __all__ = [
 ]
 
 
-class Tree(dict):
-    """A tree-like object built with python dictionaries.
+class TestDict(dict):  # pylint: disable=too-few-public-methods
+    """A special kind of dict for tests.
 
-    When a node that doesn't exist is requested it is automatically created
-    with a new Tree node.
+    This dict lowers the names of keys by default
 
     """
-    def __missing__(self, key):
-        """Automatically create new Tree nodes."""
-        self[key] = Tree()
-        return self[key]
-
     def __setitem__(self, key, value):
         """Enforce types on set operations.
-        
+
         Keys should only be strings, and values should only be more Trees
         or Tests.
 
+        This method makes one additional requirement, it lowers the key before
+        adding it. This solves a couple of problems, namely that we want to be
+        able to use filesystem heirarchies as groups in some cases, and those
+        are assumed to be all lowercase to avoid problems on case insensitive
+        filesystems.
+
         """
         assert isinstance(key, basestring), \
                "Keys must be strings, but was {}".format(type(key))
@@ -73,7 +73,26 @@ class Tree(dict):
                "Values must be either a Tree or a Test, but was {}".format(
                    type(value))
 
-        super(Tree, self).__setitem__(key, value)
+        super(TestDict, self).__setitem__(key.lower(), value)
+
+    def __getitem__(self, key):
+        """Lower the value before returning."""
+        return super(TestDict, self).__getitem__(key.lower())
+
+
+class Tree(TestDict):  # pylint: disable=too-few-public-methods
+    """A tree-like object built with python dictionaries.
+
+    When a node that doesn't exist is requested it is automatically created
+    with a new Tree node.
+
+    This also enforces lowering of keys, both for getting and setting.
+
+    """
+    def __missing__(self, key):
+        """Automatically create new Tree nodes."""
+        self[key] = Tree()
+        return self[key]
 
 
 class TestProfile(object):
@@ -97,7 +116,7 @@ class TestProfile(object):
     def __init__(self):
         # Self.tests is deprecated, see above
         self.tests = Tree()
-        self.test_list = {}
+        self.test_list = TestDict()
         self.filters = []
         # Sets a default of a Dummy
         self._dmesg = None
diff --git a/framework/results.py b/framework/results.py
index 5819af5..643592f 100644
--- a/framework/results.py
+++ b/framework/results.py
@@ -241,6 +241,7 @@ def update_results(results, filepath):
         updates = {
             0: _update_zero_to_one,
             1: _update_one_to_two,
+            2: _update_two_to_three,
         }
 
         while results.results_version < CURRENT_JSON_VERSION:
@@ -405,3 +406,16 @@ def _update_one_to_two(results):
     results.results_version = 2
 
     return results
+
+
+def _update_two_to_three(results):
+    """Lower key names."""
+    for key, value in results.tests.items():
+        lowered = key.lower()
+        if not key == lowered:
+            results.tests[lowered] = value
+            del results.tests[key]
+
+    results.results_version = 3
+
+    return results
diff --git a/framework/tests/profile_tests.py b/framework/tests/profile_tests.py
index c5e71d1..183f526 100644
--- a/framework/tests/profile_tests.py
+++ b/framework/tests/profile_tests.py
@@ -100,6 +100,10 @@ def test_testprofile_flatten():
         'group1/group2/group3/test3': test3,
     })
 
+    # Do not flatten until after baseline, since we want to use the same
+    # isntances and tests should be emptied by flattening
+    profile_._flatten_group_hierarchy()
+
     # profile_.tests should have been emptied
     nt.assert_dict_equal(profile_.tests, {})
 
@@ -116,10 +120,9 @@ def test_testprofile_update_tests():
     profile1.tests['group1']['test1'] = utils.Test(['test1'])
 
     profile2 = profile.TestProfile()
-    baseline = profile.Tree()
+    baseline = profile2.tests
     baseline['group2']['test2'] = utils.Test(['test2'])
     baseline['group1']['test1'] = utils.Test(['test3'])
-    profile2.tests = baseline
 
     profile1.update(profile2)
 
@@ -129,16 +132,16 @@ def test_testprofile_update_tests():
 def test_testprofile_update_test_list():
     """ TestProfile.update() updates TestProfile.test_list """
     profile1 = profile.TestProfile()
-    profile1.test_list['group1/test1'] = 'test1'
+    profile1.test_list['group1/test1'] = utils.Test(['test1'])
 
-    baseline = {'group1/test1': 'test3', 'group2/test2': 'test2'}
 
     profile2 = profile.TestProfile()
-    profile2.test_list = baseline
+    profile2.test_list['group1/test1'] = utils.Test(['test3'])
+    profile2.test_list['group2/test2'] = utils.Test(['test2'])
 
     profile1.update(profile2)
 
-    nt.assert_dict_equal(profile1.test_list, baseline)
+    nt.assert_dict_equal(profile1.test_list, profile2.test_list)
 
 
 def generate_prepare_test_list_flatten():
@@ -182,10 +185,10 @@ def check_mixed_flatten(tests, testlist):
     """ flattening is correct when tests and test_list are defined """
     profile_ = profile.TestProfile()
     profile_.tests = tests
-    profile_.test_list['test8'] = 'other'
+    profile_.test_list['test8'] = utils.Test(['other'])
     profile_._flatten_group_hierarchy()
 
-    baseline = {'test8': 'other'}
+    baseline = {'test8': profile_.test_list['test8']}
     baseline.update(testlist)
 
     nt.assert_dict_equal(profile_.test_list, baseline)
diff --git a/framework/tests/results_v2_tests.py b/framework/tests/results_v2_tests.py
new file mode 100644
index 0000000..e533a2a
--- /dev/null
+++ b/framework/tests/results_v2_tests.py
@@ -0,0 +1,103 @@
+# Copyright (c) 2015 Intel Corporation
+
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+
+"""Tests for version 2 to version 3."""
+
+from __future__ import print_function, absolute_import, division
+
+import os
+try:
+    import simplejson as json
+except ImportError:
+    import json
+import nose.tools as nt
+
+import framework.results as results
+import framework.tests.utils as utils
+
+DATA = {
+    "results_version": 2,
+    "name": "test",
+    "options": {
+        "profile": ['quick'],
+        "dmesg": False,
+        "verbose": False,
+        "platform": "gbm",
+        "sync": False,
+        "valgrind": False,
+        "filter": [],
+        "concurrent": "all",
+        "test_count": 0,
+        "exclude_tests": [],
+        "exclude_filter": [],
+        "env": {
+            "lspci": "stuff",
+            "uname": "more stuff",
+            "glxinfo": "and stuff",
+            "wglinfo": "stuff"
+        }
+    },
+    "tests": {
+        "test/is/a/test": {
+            "returncode": 0,
+            "err": None,
+            "environment": None,
+            "command": "foo",
+            "result": "skip",
+            "time": 0.123,
+            "out": None,
+        },
+        "Test/Is/SomE/Other1/Test": {
+            "returncode": 0,
+            "err": None,
+            "environment": None,
+            "command": "foo",
+            "result": "skip",
+            "time": 0.123,
+            "out": None,
+        }
+    }
+}
+
+with utils.with_tempfile(json.dumps(DATA)) as t:
+    with open(t, 'r') as f:
+        # pylint: disable=protected-access
+        RESULT = results._update_two_to_three(results.TestrunResult.load(f))
+
+
+def test_unchanged():
+    """Version 2: results with no caps are not mangled."""
+    nt.ok_('test/is/a/test' in RESULT.tests)
+
+
+def test_lower():
+    """Version 2: results with aps are lowered."""
+    nt.ok_('test/is/some/other1/test' in RESULT.tests)
+
+
+def test_load_results():
+    """Version 2: load_results properly updates."""
+    with utils.tempdir() as d:
+        tempfile = os.path.join(d, 'results.json')
+        with open(tempfile, 'w') as f:
+            json.dump(DATA, f)
+        with open(tempfile, 'r') as f:
+            result = results.load_results(tempfile)
+            nt.assert_equal(result.results_version, 3)
-- 
2.2.2



More information about the Piglit mailing list