[Piglit] [PATCH v2 03/16] framework/test/base: Use an exception for _run_command failure

[Dylan Baker]

This removes the need for passing and allows us to set the result in one
place instead of multiple places, it also means that Mixins like the
WindowResizeMixin don't need to handle errors from the base version, if
there is an exception, it will go until it's caught in Test.run()

Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
 framework/test/base.py        | 53 ++++++++++++++++++-------------------------
 framework/tests/base_tests.py | 16 +++++++++++--
 2 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/framework/test/base.py b/framework/test/base.py
index 9a3a149..b01d378 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -52,6 +52,13 @@ class TestIsSkip(exceptions.PiglitException):
     pass
 
 
+class TestRunError(exceptions.PiglitException):
+    """Exception raised if the test fails to run."""
+    def __init__(self, message, status):
+        super(TestRunError, self).__init__(message)
+        self.status = status
+
+
 class ProcessTimeout(threading.Thread):
     """ Timeout class for test processes
 
@@ -208,15 +215,18 @@ class Test(object):
             self.is_skip()
         except TestIsSkip as e:
             self.result['result'] = 'skip'
-            self.result['out'] = e.message
-            self.result['err'] = ""
+            self.result['out'] = unicode(e.message)
+            self.result['err'] = u""
             self.result['returncode'] = None
             return
 
-        run_error = self._run_command()
-        if run_error:
-            # If there was an error self.result should already have been set,
-            # return early
+        try:
+            self._run_command()
+        except TestRunError as e:
+            self.result['result'] = unicode(e.status)
+            self.result['out'] = unicode(e.message)
+            self.result['err'] = u""
+            self.result['returncode'] = None
             return
 
         self.interpret_result()
@@ -262,11 +272,6 @@ class Test(object):
         executable isn't found it sets the result to skip.
 
         """
-        # if there is an error in run command this will be set to True, if this
-        # is True then the run test method will return early. If this is set to
-        # True then self.result should be properly filled out
-        error = False
-
         # Setup the environment for the test. Environment variables are taken
         # from the following sources, listed in order of increasing precedence:
         #
@@ -317,11 +322,7 @@ class Test(object):
             # Piglit should not report that test as having
             # failed.
             if e.errno == errno.ENOENT:
-                self.result['result'] = 'skip'
-                out = "Test executable not found.\n"
-                err = ""
-                returncode = None
-                error = True
+                raise TestRunError("Test executable not found.\n", 'skip')
             else:
                 raise e
 
@@ -343,8 +344,6 @@ class Test(object):
         self.result['err'] = err.decode('utf-8', 'replace')
         self.result['returncode'] = returncode
 
-        return error
-
     def __eq__(self, other):
         return self.command == other.command
 
@@ -373,18 +372,10 @@ class WindowResizeMixin(object):
 
         """
         for _ in xrange(5):
-            err = super(WindowResizeMixin, self)._run_command()
-            if err:
-                return err
-            elif "Got spurious window resize" not in self.result['out']:
-                return False
+            super(WindowResizeMixin, self)._run_command()
+            if "Got spurious window resize" not in self.result['out']:
+                return
 
         # If we reach this point then there has been no error, but spurious
-        # resize was detected more than 5 times. Set the result to fail, and
-        # add a message about why, and return True so that the test will exit
-        # early
-        self.result['result'] = 'fail'
-        self.result['err'] = unicode()
-        self.result['out'] = unicode('Got spurious resize more than 5 times')
-        self.result['returncode'] = None
-        return True
+        # resize was detected more than 5 times. Set the result to fail
+        raise TestRunError('Got spurious resize more than 5 times', 'fail')
diff --git a/framework/tests/base_tests.py b/framework/tests/base_tests.py
index 9bbd88e..a9e0e88 100644
--- a/framework/tests/base_tests.py
+++ b/framework/tests/base_tests.py
@@ -25,7 +25,7 @@ from __future__ import print_function, absolute_import
 import nose.tools as nt
 
 import framework.tests.utils as utils
-from framework.test.base import Test, WindowResizeMixin
+from framework.test.base import Test, WindowResizeMixin, TestRunError
 
 
 # Helpers
@@ -43,6 +43,18 @@ class TestTest(Test):
 
 
 # Tests
+def test_run_return_early():
+    """ Test.run() exits early when Test._run_command() has exception """
+    def helper():
+        raise utils.TestFailure("The test didn't return early")
+
+    # Of course, this won't work if you actually have a foobarcommand in your
+    # path...
+    test = TestTest(['foobaroinkboink_zing'])
+    test.test_interpret_result = helper
+    test.run()
+
+
 def test_timeout():
     """test.base.Test.run(): kills tests that exceed timeout when set"""
     utils.binary_check('sleep')
@@ -111,7 +123,7 @@ def test_run_command_early():
             raise utils.TestFailure("The test didn't return early")
 
         def _run_command(self):
-            return True
+            raise TestRunError('an error', 'skip')
 
     # Of course, if there is an executable 'foobarboinkoink' in your path this
     # test will fail. It seems pretty unlikely that you would
-- 
2.4.0