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

[Ilia Mirkin]

On Thu, Oct 2, 2014 at 5:46 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> On Thursday, October 02, 2014 05:40:11 PM Ilia Mirkin wrote:
>> 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?
>
> It gives effectively two constructors, one that we call in cases where
> we're working with existing data, and one that we call when working with
> new data.

Ah, so there are places that call TestResult() directly (other than
.load)? I probably should have checked that first.

>
>>
>> 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