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

[Dylan Baker]

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.

> 
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20141002/b892641c/attachment.sig>