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

[Dylan Baker]

On Thursday, October 02, 2014 05:53:07 PM Ilia Mirkin wrote:
> 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.

yes, there is at least one in exectest where it is used to set the
default test result.

> 
> >
> >>
> >> 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/88a3e5db/attachment.sig>