[v3,21/28] framework/profile: replace Testprofile.{dmesg, monitoring} with dict

Submitted by Dylan Baker on Oct. 31, 2016, 5:50 p.m.

Details

Message ID 8d6f16ce715af8c3ed891ac54683a5e7d6cbdfda.1477936071.git-series.dylan@pnwbakers.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Dylan Baker Oct. 31, 2016, 5:50 p.m.
This allows a significant amount of cleanup to happen. It allows
removing attributes from the global OPTIONS, removing tests that no
longer apply, and because of the split run method it allows more values
to simply be passed.

Signed-off-by: Dylan Baker <dylanx.c.baker@intel.com>
---
 framework/options.py                         |  4 +--
 framework/profile.py                         | 50 +++------------------
 framework/programs/run.py                    | 21 ++++-----
 framework/test/base.py                       | 31 ++++++-------
 tests/igt.py                                 |  7 +--
 unittests/framework/test/test_base.py        | 14 +++---
 unittests/framework/test/test_shader_test.py |  2 +-
 unittests/framework/test_profile.py          | 24 +----------
 8 files changed, 46 insertions(+), 107 deletions(-)

Patch hide | download patch | download mbox

diff --git a/framework/options.py b/framework/options.py
index db4bf76..211159a 100644
--- a/framework/options.py
+++ b/framework/options.py
@@ -48,8 +48,6 @@  class _Options(object):  # pylint: disable=too-many-instance-attributes
     Options are as follows:
     execute -- False for dry run
     valgrind -- True if valgrind is to be used
-    dmesg -- True if dmesg checking is desired. This forces concurrency off
-    monitored -- True if monitoring is desired. This forces concurrency off
     env -- environment variables set for each test before run
     deqp_mustpass -- True to enable the use of the deqp mustpass list feature.
     """
@@ -57,8 +55,6 @@  class _Options(object):  # pylint: disable=too-many-instance-attributes
     def __init__(self):
         self.execute = True
         self.valgrind = False
-        self.dmesg = False
-        self.monitored = False
         self.sync = False
         self.deqp_mustpass = False
         self.process_isolation = True
diff --git a/framework/profile.py b/framework/profile.py
index 1cd0c0d..e0c1f72 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -213,42 +213,10 @@  class TestProfile(object):
         self.test_list = TestDict()
         self.forced_test_list = []
         self.filters = []
-        # Sets a default of a Dummy
-        self._dmesg = None
-        self.dmesg = False
-        self.results_dir = None
-        self._monitoring = None
-        self.monitoring = False
-
-    @property
-    def dmesg(self):
-        """ Return dmesg """
-        return self._dmesg
-
-    @dmesg.setter
-    def dmesg(self, not_dummy):
-        """Use dmesg.
-
-        Arguments:
-        not_dummy -- Get a platform dependent Dmesg class if True, otherwise
-                     get a DummyDmesg.
-        """
-        self._dmesg = get_dmesg(not_dummy)
-
-    @property
-    def monitoring(self):
-        """ Return monitoring """
-        return self._monitoring
-
-    @monitoring.setter
-    def monitoring(self, monitored):
-        """Set monitoring.
-
-        Arguments:
-        monitored -- if Truthy Monitoring will enable monitoring according the
-                     defined rules
-        """
-        self._monitoring = Monitoring(monitored)
+        self.options = {
+            'dmesg': get_dmesg(False),
+            'monitor': Monitoring(False),
+        }
 
     @contextlib.contextmanager
     def group_manager(self, test_class, group, prefix=None, **default_args):
@@ -433,9 +401,9 @@  def run(profiles, logger, backend, concurrency):
     def test(name, test, profile, this_pool=None):
         """Function to call test.execute from map"""
         with backend.write_test(name) as w:
-            test.execute(name, log.get(), profile.dmesg, profile.monitoring)
+            test.execute(name, log.get(), profile.options)
             w(test.result)
-        if profile.monitoring.abort_needed:
+        if profile.options['monitor'].abort_needed:
             this_pool.terminate()
 
     def run_threads(pool, profile, test_list, filterby=None):
@@ -485,6 +453,6 @@  def run(profiles, logger, backend, concurrency):
     finally:
         log.get().summary()
 
-    for p in profiles:
-        if p.monitoring.abort_needed:
-            raise exceptions.PiglitAbort(p.monitoring.error_message)
+    for p, _ in profiles:
+        if p.options['monitor'].abort_needed:
+            raise exceptions.PiglitAbort(p.options['monitor'].error_message)
diff --git a/framework/programs/run.py b/framework/programs/run.py
index e1b3da3..1ee9e0d 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -34,6 +34,8 @@  import time
 import six
 
 from framework import core, backends, exceptions, options
+from framework import dmesg
+from framework import monitoring
 from framework import profile
 from framework.results import TimeAttribute
 from . import parsers
@@ -226,6 +228,8 @@  def _create_metadata(args, name):
     opts['concurrent'] = args.concurrent.name
     opts['include_filter'] = args.include_tests
     opts['exclude_filter'] = args.exclude_tests
+    opts['dmesg'] = args.dmesg
+    opts['monitoring'] = args.monitored
     if args.platform:
         opts['platform'] = args.platform
 
@@ -282,8 +286,6 @@  def run(input_):
     # Pass arguments into Options
     options.OPTIONS.execute = args.execute
     options.OPTIONS.valgrind = args.valgrind
-    options.OPTIONS.dmesg = args.dmesg
-    options.OPTIONS.monitored = args.monitored
     options.OPTIONS.sync = args.sync
     options.OPTIONS.deqp_mustpass = args.deqp_mustpass
     options.OPTIONS.process_isolation = args.process_isolation
@@ -330,11 +332,11 @@  def run(input_):
     # Set the dmesg type
     if args.dmesg:
         for p in profiles:
-            p.dmesg = args.dmesg
+            p.options['dmesg'] = dmesg.get_dmesg(args.dmesg)
 
     if args.monitored:
         for p in profiles:
-            p.monitoring = args.monitored
+            p.options['monitor'] = monitoring.Monitoring(args.monitored)
 
     for p in profiles:
         if args.exclude_tests:
@@ -376,8 +378,6 @@  def resume(input_):
     results = backends.load(args.results_path)
     options.OPTIONS.execute = results.options['execute']
     options.OPTIONS.valgrind = results.options['valgrind']
-    options.OPTIONS.dmesg = results.options['dmesg']
-    options.OPTIONS.monitored = results.options['monitored']
     options.OPTIONS.sync = results.options['sync']
     options.OPTIONS.deqp_mustpass = results.options['deqp_mustpass']
     options.OPTIONS.proces_isolation = results.options['process_isolation']
@@ -407,11 +407,12 @@  def resume(input_):
     for p in profiles:
         p.results_dir = args.results_path
 
-        if options.OPTIONS.dmesg:
-            p.dmesg = options.OPTIONS.dmesg
+        if results.options['dmesg']:
+            p.dmesg = dmesg.get_dmesg(results.options['dmesg'])
 
-        if options.OPTIONS.monitored:
-            p.monitoring = options.OPTIONS.monitored
+        if results.options['monitoring']:
+            p.options['monitor'] = monitoring.Monitoring(
+                results.options['monitoring'])
 
         if exclude_tests:
             p.filters.append(lambda n, _: n not in exclude_tests)
diff --git a/framework/test/base.py b/framework/test/base.py
index 5f0491d..6b7cab6 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -39,8 +39,9 @@  import warnings
 import six
 from six.moves import range
 
-from framework import exceptions, options
+from framework import exceptions
 from framework import status
+from framework.options import OPTIONS
 from framework.results import TestResult
 
 # We're doing some special crazy here to make timeouts work on python 2. pylint
@@ -186,30 +187,28 @@  class Test(object):
             assert isinstance(timeout, int)
             self.timeout = timeout
 
-    def execute(self, path, log, dmesg, monitoring):
+    def execute(self, path, log, options):
         """ Run a test
 
         Run a test, but with features. This times the test, uses dmesg checking
         (if requested), and runs the logger.
 
         Arguments:
-        path -- the name of the test
-        log -- a log.Log instance
-        dmesg -- a dmesg.BaseDmesg derived class
-        monitoring -- a monitoring.Monitoring instance
-
+        path    -- the name of the test
+        log     -- a log.Log instance
+        options -- a dictionary containing dmesg and monitoring objects
         """
         log.start(path)
         # Run the test
-        if options.OPTIONS.execute:
+        if OPTIONS.execute:
             try:
                 self.result.time.start = time.time()
-                dmesg.update_dmesg()
-                monitoring.update_monitoring()
+                options['dmesg'].update_dmesg()
+                options['monitor'].update_monitoring()
                 self.run()
                 self.result.time.end = time.time()
-                self.result = dmesg.update_result(self.result)
-                monitoring.check_monitoring()
+                self.result = options['dmesg'].update_result(self.result)
+                options['monitor'].check_monitoring()
             # This is a rare case where a bare exception is okay, since we're
             # using it to log exceptions
             except:
@@ -254,7 +253,7 @@  class Test(object):
         self.result.command = ' '.join(self.command)
         self.result.environment = " ".join(
             '{0}="{1}"'.format(k, v) for k, v in itertools.chain(
-                six.iteritems(options.OPTIONS.env), six.iteritems(self.env)))
+                six.iteritems(OPTIONS.env), six.iteritems(self.env)))
 
         try:
             self.is_skip()
@@ -319,7 +318,7 @@  class Test(object):
         else:
             f = six.text_type
         _base = itertools.chain(six.iteritems(os.environ),
-                                six.iteritems(options.OPTIONS.env),
+                                six.iteritems(OPTIONS.env),
                                 six.iteritems(self.env))
         fullenv = {f(k): f(v) for k, v in _base}
 
@@ -419,7 +418,7 @@  class ValgrindMixin(object):
     @Test.command.getter
     def command(self):
         command = super(ValgrindMixin, self).command
-        if options.OPTIONS.valgrind:
+        if OPTIONS.valgrind:
             return ['valgrind', '--quiet', '--error-exitcode=1',
                     '--tool=memcheck'] + command
         else:
@@ -436,7 +435,7 @@  class ValgrindMixin(object):
         """
         super(ValgrindMixin, self).interpret_result()
 
-        if options.OPTIONS.valgrind:
+        if OPTIONS.valgrind:
             # If the underlying test failed, simply report
             # 'skip' for this valgrind test.
             if self.result.result != 'pass' and (
diff --git a/tests/igt.py b/tests/igt.py
index 69c91d1..87b61dc 100644
--- a/tests/igt.py
+++ b/tests/igt.py
@@ -41,6 +41,7 @@  import re
 import subprocess
 
 from framework import grouptools, exceptions, core, options
+from framework import dmesg
 from framework.profile import TestProfile, Test
 
 __all__ = ['profile']
@@ -176,7 +177,5 @@  def populate_profile():
 
 
 populate_profile()
-profile.dmesg = True
-
-# the dmesg property of TestProfile returns a Dmesg object
-profile.dmesg.regex = re.compile(r"(\[drm:|drm_|intel_|i915_)")
+profile.options['dmesg'] = dmesg.get_dmesg(True)
+profile.options['dmesg'].regex = re.compile(r"(\[drm:|drm_|intel_|i915_)")
diff --git a/unittests/framework/test/test_base.py b/unittests/framework/test/test_base.py
index 6b0c299..656d839 100644
--- a/unittests/framework/test/test_base.py
+++ b/unittests/framework/test/test_base.py
@@ -209,8 +209,8 @@  class TestTest(object):
 
             test.execute(mocker.Mock(spec=six.text_type),
                          mocker.Mock(spec=log.BaseLog),
-                         mocker.Mock(spec=dmesg.BaseDmesg),
-                         mocker.Mock(spec=monitoring.Monitoring))
+                         {'dmesg': mocker.Mock(spec=dmesg.BaseDmesg),
+                          'monitor': mocker.Mock(spec=monitoring.Monitoring)})
             return test.result
 
         def test_result(self, shared_test):
@@ -321,7 +321,7 @@  class TestValgrindMixin(object):
 
     def test_command(self, mocker):
         """test.base.ValgrindMixin.command: overrides self.command."""
-        opts = mocker.patch('framework.test.base.options.OPTIONS',
+        opts = mocker.patch('framework.test.base.OPTIONS',
                             new_callable=Options)
 
         class Test(base.ValgrindMixin, _Test):
@@ -358,7 +358,7 @@  class TestValgrindMixin(object):
             binary itself, so any status other than pass is irrelavent, and
             should be marked as skip.
             """
-            mock_opts = mocker.patch('framework.test.base.options.OPTIONS',
+            mock_opts = mocker.patch('framework.test.base.OPTIONS',
                                      new_callable=Options)
             mock_opts.valgrind = True
 
@@ -372,7 +372,7 @@  class TestValgrindMixin(object):
         def test_problems_with_valgrind_disabled(self, starting, mocker):
             """When valgrind is disabled nothign shoud change
             """
-            mock_opts = mocker.patch('framework.test.base.options.OPTIONS',
+            mock_opts = mocker.patch('framework.test.base.OPTIONS',
                                      new_callable=Options)
             mock_opts.valgrind = False
 
@@ -386,7 +386,7 @@  class TestValgrindMixin(object):
             """test.base.ValgrindMixin.run: when test is 'pass' and returncode
             is '0' result is pass.
             """
-            mock_opts = mocker.patch('framework.test.base.options.OPTIONS',
+            mock_opts = mocker.patch('framework.test.base.OPTIONS',
                                      new_callable=Options)
             test = self.test(['foo'])
             mock_opts.valgrind = True
@@ -399,7 +399,7 @@  class TestValgrindMixin(object):
             """test.base.ValgrindMixin.run: when a test is 'pass' but
             returncode is not 0 it's 'fail'.
             """
-            mock_opts = mocker.patch('framework.test.base.options.OPTIONS',
+            mock_opts = mocker.patch('framework.test.base.OPTIONS',
                                      new_callable=Options)
             mock_opts.valgrind = True
             test = self.test(['foo'])
diff --git a/unittests/framework/test/test_shader_test.py b/unittests/framework/test/test_shader_test.py
index c62aee3..5484dca 100644
--- a/unittests/framework/test/test_shader_test.py
+++ b/unittests/framework/test/test_shader_test.py
@@ -42,7 +42,7 @@  class _Setup(object):
     def __init__(self):
         self.__patchers = []
         self.__patchers.append(mock.patch.dict(
-            'framework.test.base.options.OPTIONS.env',
+            'framework.test.base.OPTIONS.env',
             {'PIGLIT_PLATFORM': 'foo'}))
 
     def setup(self, _):
diff --git a/unittests/framework/test_profile.py b/unittests/framework/test_profile.py
index 9ac0d21..9067362 100644
--- a/unittests/framework/test_profile.py
+++ b/unittests/framework/test_profile.py
@@ -27,13 +27,11 @@  from __future__ import (
 import pytest
 import six
 
-from framework import dmesg
 from framework import exceptions
 from framework import grouptools
 from framework import profile
 from framework.test.gleantest import GleanTest
 from . import utils
-from . import skip
 
 # pylint: disable=invalid-name,no-self-use,protected-access
 
@@ -73,28 +71,6 @@  class TestLoadTestProfile(object):
 class TestTestProfile(object):
     """Tests for profile.TestProfile."""
 
-    def test_default_dmesg(self):
-        """profile.TestProfile: Dmesg is dummy by default."""
-        profile_ = profile.TestProfile()
-        assert isinstance(profile_.dmesg, dmesg.DummyDmesg)
-
-    @skip.linux
-    def test_set_dmesg_true(self):
-        """profile.TestProfile: Dmesg returns an appropriate dmesg is set to
-        True.
-        """
-        profile_ = profile.TestProfile()
-        profile_.dmesg = True
-        assert isinstance(profile_.dmesg, dmesg.LinuxDmesg)
-
-    @skip.linux
-    def test_set_dmesg_false(self):
-        """profile.TestProfile: Dmesg returns a DummyDmesg if set to False."""
-        profile_ = profile.TestProfile()
-        profile_.dmesg = True
-        profile_.dmesg = False
-        assert isinstance(profile_.dmesg, dmesg.DummyDmesg)
-
     class TestGroupManager(object):
         """Tests for TestProfile.group_manager."""