Re: [Piglit] [PATCH 2/2] framework/test/base.py: Handle fail cases for tests.
On Mon, Dec 14, 2015 at 05:14:18PM -0800, Mark Janes wrote: > Reviewed-by: Mark Janes > > baker.dyla...@gmail.com writes: > > > From: Dylan Baker > > > > I'm going to admit I'm a bit puzzled how this could have slipped through > > without being caught (I'm guessing an unrelated change uncovered this). > > But basically if a test doesn't raise an exception, and the returncode > > is > 0, it should mark the test with a status of "fail", but it doesn't. > > Instead the default status "notrun" is passed to the logger, which it > > doesn't support, and an exception is raised. > > > > This patch corrects that problem. > > > > bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93340 > > cc: mark.a.ja...@intel.com > > Signed-off-by: Dylan Baker This seems to break igt badly, which uses special return codes (decoded in the igt runner) to mark tests as skipped. CI is pretty red because of this now here :( Not sure what exactly needs to be fixed here ... -Daniel > > --- > > > > I'm not really sure this is exactly the right solution. I wonder if we'd > > be better just to get rid of the "warn" and just say if the returncode > > is 0 leave it alone, if it's crash/timeout (as defined by the function), > > do that, otherwise mark it as "fail". > > > > framework/test/base.py| 7 +-- > > framework/tests/base_tests.py | 15 +++ > > 2 files changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/framework/test/base.py b/framework/test/base.py > > index 378f185..4232e6c 100644 > > --- a/framework/test/base.py > > +++ b/framework/test/base.py > > @@ -210,8 +210,11 @@ class Test(object): > > self.result.result = 'timeout' > > else: > > self.result.result = 'crash' > > -elif self.result.returncode != 0 and self.result.result == 'pass': > > -self.result.result = 'warn' > > +elif self.result.returncode != 0: > > +if self.result.result == 'pass': > > +self.result.result = 'warn' > > +else: > > +self.result.result = 'fail' > > > > def run(self): > > """ > > diff --git a/framework/tests/base_tests.py b/framework/tests/base_tests.py > > index c005273..20bea87 100644 > > --- a/framework/tests/base_tests.py > > +++ b/framework/tests/base_tests.py > > @@ -260,3 +260,18 @@ class TestValgrindMixinRun(object): > > test.result.returncode = 1 > > test.run() > > nt.eq_(test.result.result, 'fail') > > + > > + > > +def test_interpret_result_greater_zero(): > > +"""test.base.Test.interpret_result: A test with status > 0 is fail""" > > +class _Test(Test): > > +def interpret_result(self): > > +super(_Test, self).interpret_result() > > + > > +test = _Test(['foobar']) > > +test.result.returncode = 1 > > +test.result.out = 'this is some\nstdout' > > +test.result.err = 'this is some\nerrors' > > +test.interpret_result() > > + > > +nt.eq_(test.result.result, 'fail') > > -- > > 2.6.4 > ___ > Piglit mailing list > Piglit@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/piglit -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
Re: [Piglit] [PATCH 2/2] framework/test/base.py: Handle fail cases for tests.
Reviewed-by: Mark Janes baker.dyla...@gmail.com writes: > From: Dylan Baker > > I'm going to admit I'm a bit puzzled how this could have slipped through > without being caught (I'm guessing an unrelated change uncovered this). > But basically if a test doesn't raise an exception, and the returncode > is > 0, it should mark the test with a status of "fail", but it doesn't. > Instead the default status "notrun" is passed to the logger, which it > doesn't support, and an exception is raised. > > This patch corrects that problem. > > bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93340 > cc: mark.a.ja...@intel.com > Signed-off-by: Dylan Baker > --- > > I'm not really sure this is exactly the right solution. I wonder if we'd > be better just to get rid of the "warn" and just say if the returncode > is 0 leave it alone, if it's crash/timeout (as defined by the function), > do that, otherwise mark it as "fail". > > framework/test/base.py| 7 +-- > framework/tests/base_tests.py | 15 +++ > 2 files changed, 20 insertions(+), 2 deletions(-) > > diff --git a/framework/test/base.py b/framework/test/base.py > index 378f185..4232e6c 100644 > --- a/framework/test/base.py > +++ b/framework/test/base.py > @@ -210,8 +210,11 @@ class Test(object): > self.result.result = 'timeout' > else: > self.result.result = 'crash' > -elif self.result.returncode != 0 and self.result.result == 'pass': > -self.result.result = 'warn' > +elif self.result.returncode != 0: > +if self.result.result == 'pass': > +self.result.result = 'warn' > +else: > +self.result.result = 'fail' > > def run(self): > """ > diff --git a/framework/tests/base_tests.py b/framework/tests/base_tests.py > index c005273..20bea87 100644 > --- a/framework/tests/base_tests.py > +++ b/framework/tests/base_tests.py > @@ -260,3 +260,18 @@ class TestValgrindMixinRun(object): > test.result.returncode = 1 > test.run() > nt.eq_(test.result.result, 'fail') > + > + > +def test_interpret_result_greater_zero(): > +"""test.base.Test.interpret_result: A test with status > 0 is fail""" > +class _Test(Test): > +def interpret_result(self): > +super(_Test, self).interpret_result() > + > +test = _Test(['foobar']) > +test.result.returncode = 1 > +test.result.out = 'this is some\nstdout' > +test.result.err = 'this is some\nerrors' > +test.interpret_result() > + > +nt.eq_(test.result.result, 'fail') > -- > 2.6.4 ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
[Piglit] [PATCH 2/2] framework/test/base.py: Handle fail cases for tests.
From: Dylan Baker I'm going to admit I'm a bit puzzled how this could have slipped through without being caught (I'm guessing an unrelated change uncovered this). But basically if a test doesn't raise an exception, and the returncode is > 0, it should mark the test with a status of "fail", but it doesn't. Instead the default status "notrun" is passed to the logger, which it doesn't support, and an exception is raised. This patch corrects that problem. bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93340 cc: mark.a.ja...@intel.com Signed-off-by: Dylan Baker --- I'm not really sure this is exactly the right solution. I wonder if we'd be better just to get rid of the "warn" and just say if the returncode is 0 leave it alone, if it's crash/timeout (as defined by the function), do that, otherwise mark it as "fail". framework/test/base.py| 7 +-- framework/tests/base_tests.py | 15 +++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/framework/test/base.py b/framework/test/base.py index 378f185..4232e6c 100644 --- a/framework/test/base.py +++ b/framework/test/base.py @@ -210,8 +210,11 @@ class Test(object): self.result.result = 'timeout' else: self.result.result = 'crash' -elif self.result.returncode != 0 and self.result.result == 'pass': -self.result.result = 'warn' +elif self.result.returncode != 0: +if self.result.result == 'pass': +self.result.result = 'warn' +else: +self.result.result = 'fail' def run(self): """ diff --git a/framework/tests/base_tests.py b/framework/tests/base_tests.py index c005273..20bea87 100644 --- a/framework/tests/base_tests.py +++ b/framework/tests/base_tests.py @@ -260,3 +260,18 @@ class TestValgrindMixinRun(object): test.result.returncode = 1 test.run() nt.eq_(test.result.result, 'fail') + + +def test_interpret_result_greater_zero(): +"""test.base.Test.interpret_result: A test with status > 0 is fail""" +class _Test(Test): +def interpret_result(self): +super(_Test, self).interpret_result() + +test = _Test(['foobar']) +test.result.returncode = 1 +test.result.out = 'this is some\nstdout' +test.result.err = 'this is some\nerrors' +test.interpret_result() + +nt.eq_(test.result.result, 'fail') -- 2.6.4 ___ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit
