[Piglit] [PATCH 2/3] results.py: Do add load class method to TestResult

[Ilia Mirkin]

I'm probably just missing something... but what's the difference
between doing work in a constructor or doing work in a function that
everything calls instead of using the constructor directly?

On Thu, Oct 2, 2014 at 5:31 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> This creates an alternate constructor for loading an existing test
> result, designed for use with loading an existing TestrunResult. This
> fixes the bug in demonstrated by the test in the last patch by not
> converting the result key into a status object in all conditions.
>
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> ---
>  framework/results.py             | 33 ++++++++++++++++++++-------------
>  framework/tests/results_tests.py |  4 ++--
>  2 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/framework/results.py b/framework/results.py
> index 03aa494..cf7b833 100644
> --- a/framework/results.py
> +++ b/framework/results.py
> @@ -43,17 +43,6 @@ __all__ = [
>
>
>  class TestResult(dict):
> -    def __init__(self, *args):
> -        super(TestResult, self).__init__(*args)
> -
> -        # Replace the result with a status object
> -        try:
> -            self['result'] = status.status_lookup(self['result'])
> -        except KeyError:
> -            # If there isn't a result (like when used by piglit-run), go on
> -            # normally
> -            pass
> -
>      def recursive_update(self, dictionary):
>          """ Recursively update the TestResult
>
> @@ -88,6 +77,24 @@ class TestResult(dict):
>
>          update(self, dictionary, False)
>
> +    @classmethod
> +    def load(cls, res):
> +        """Load an already generated result.
> +
> +        This is used as an alternate constructor which converts an existing
> +        dictionary into a TestResult object. It converts a key 'result' into a
> +        status.Status object
> +
> +        """
> +        result = cls(res)
> +
> +        # Replace the result with a status object. 'result' is a required key
> +        # for results, so don't do any checking. This should fail if it doesn't
> +        # exist.
> +        result['result'] = status.status_lookup(result['result'])
> +
> +        return result
> +
>
>  class TestrunResult(object):
>      def __init__(self, resultfile=None):
> @@ -130,7 +137,7 @@ class TestrunResult(object):
>
>              # Replace each raw dict in self.tests with a TestResult.
>              for (path, result) in self.tests.items():
> -                self.tests[path] = TestResult(result)
> +                self.tests[path] = TestResult.load(result)
>
>      def __repair_file(self, file_):
>          '''
> @@ -239,7 +246,7 @@ class TestrunResult(object):
>              # XXX: There has to be a better way to get a single key: value out
>              # of a dict even when the key name isn't known
>              for key, value in test.iteritems():
> -                testrun.tests[key] = TestResult(value)
> +                testrun.tests[key] = TestResult.load(value)
>
>          return testrun
>
> diff --git a/framework/tests/results_tests.py b/framework/tests/results_tests.py
> index 0b4cd23..990e373 100644
> --- a/framework/tests/results_tests.py
> +++ b/framework/tests/results_tests.py
> @@ -81,10 +81,10 @@ def test_load_results_file():
>          results.load_results(tfile.name)
>
>
> -def test_testresult_to_status():
> +def test_testresult_load_to_status():
>      """ TestResult initialized with result key converts the value to a Status
>      """
> -    result = results.TestResult({'result': 'pass'})
> +    result = results.TestResult.load({'result': 'pass'})
>      assert isinstance(result['result'], status.Status), \
>          "Result key not converted to a status object"
>
> --
> 2.1.1
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit