Re: [Piglit] [PATCH] Interpret dEQP QualityWarning as pass

2015-06-26 Thread Ilia Mirkin
On Fri, Jun 26, 2015 at 12:33 PM, Mark Janes mark.a.ja...@intel.com wrote:
 Ilia Mirkin imir...@alum.mit.edu writes:

 On Thu, Jun 25, 2015 at 8:52 PM, Mark Janes mark.a.ja...@intel.com wrote:
 Many tests in the functional.fbo.completeness.attachment_combinations
 report QualityWarning intermittently.  This particular warning is
 emitted because dEQP attempted to create an unsupported framebuffer.

 warning is interpreted as failure in junit, but this output from
 dEQP is clearly not a failure.

 So... fix junit. warn is not a failure output by piglit.


 I'm fine with changing piglit's junit back end to interpret warn as a
 junit pass.  Changing the dEQP wrapper has smaller scope.  Chad
 recommended changing deqp.py because:

  * dEQP warnings appear to be bogus

  * dEQP warnings are intermittent

That's fine, but then that's the justification, not junit interprets
warn as fail. Note that some (regular) piglit tests will also emit
warn, and they don't mean fail. I believe that junit needs to be
fixed to not interpret warn as fail irrespective of what happens in
deqp. But for the record, I don't use junit.

I'm not sure how I feel about the warn output in piglit in general,
it seems a bit weird. Bogus and intermittent warnings certainly sound
like a problem, and perhaps that's reason enough to just ignore them.


  * dEQP has fewer users at this point (is anyone running it?)

I know Rob has run it once or twice on freedreno.


 I could work around these warnings by not running the tests which
 produce warnings in Intel's automation, but then we would never know if
 the tests regress.

Of all the possible solutions, that sounds like by far the worst.


 -Mark

 ---
  framework/test/deqp.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/framework/test/deqp.py b/framework/test/deqp.py
 index 1462ca2..f4bf1cd 100644
 --- a/framework/test/deqp.py
 +++ b/framework/test/deqp.py
 @@ -105,7 +105,7 @@ class DEQPBaseTest(Test):
  __metaclass__ = abc.ABCMeta
  __RESULT_MAP = {Pass: pass,
  Fail: fail,
 -QualityWarning: warn,
 +QualityWarning: pass,
  InternalError: fail,
  Crash: crash,
  NotSupported: skip}
 --
 2.1.4

 ___
 Piglit mailing list
 Piglit@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] Interpret dEQP QualityWarning as pass

2015-06-26 Thread Dylan Baker
 That's fine, but then that's the justification, not junit interprets
 warn as fail. Note that some (regular) piglit tests will also emit
 warn, and they don't mean fail. I believe that junit needs to be
 fixed to not interpret warn as fail irrespective of what happens in
 deqp. But for the record, I don't use junit.
 
 I'm not sure how I feel about the warn output in piglit in general,
 it seems a bit weird. Bogus and intermittent warnings certainly sound
 like a problem, and perhaps that's reason enough to just ignore them.

I'm fine with either solution.

Warn is kind of an odd status. Basically if a test passes, but there is
anything unexpected in stderr, the test will get marked warn. (I think
some other suites use it differently, maybe igt?)

Dylan


signature.asc
Description: Digital signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] Interpret dEQP QualityWarning as pass

2015-06-26 Thread Mark Janes
Ilia Mirkin imir...@alum.mit.edu writes:

 On Thu, Jun 25, 2015 at 8:52 PM, Mark Janes mark.a.ja...@intel.com wrote:
 Many tests in the functional.fbo.completeness.attachment_combinations
 report QualityWarning intermittently.  This particular warning is
 emitted because dEQP attempted to create an unsupported framebuffer.

 warning is interpreted as failure in junit, but this output from
 dEQP is clearly not a failure.

 So... fix junit. warn is not a failure output by piglit.


I'm fine with changing piglit's junit back end to interpret warn as a
junit pass.  Changing the dEQP wrapper has smaller scope.  Chad
recommended changing deqp.py because:

 * dEQP warnings appear to be bogus

 * dEQP warnings are intermittent

 * dEQP has fewer users at this point (is anyone running it?)

I could work around these warnings by not running the tests which
produce warnings in Intel's automation, but then we would never know if
the tests regress.

-Mark

 ---
  framework/test/deqp.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/framework/test/deqp.py b/framework/test/deqp.py
 index 1462ca2..f4bf1cd 100644
 --- a/framework/test/deqp.py
 +++ b/framework/test/deqp.py
 @@ -105,7 +105,7 @@ class DEQPBaseTest(Test):
  __metaclass__ = abc.ABCMeta
  __RESULT_MAP = {Pass: pass,
  Fail: fail,
 -QualityWarning: warn,
 +QualityWarning: pass,
  InternalError: fail,
  Crash: crash,
  NotSupported: skip}
 --
 2.1.4

 ___
 Piglit mailing list
 Piglit@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] Interpret dEQP QualityWarning as pass

2015-06-26 Thread Ilia Mirkin
On Fri, Jun 26, 2015 at 4:02 PM, Mark Janes mark.a.ja...@intel.com wrote:
 Ilia Mirkin imir...@alum.mit.edu writes:

 On Fri, Jun 26, 2015 at 2:44 PM, Dylan Baker baker.dyla...@gmail.com wrote:
 That's fine, but then that's the justification, not junit interprets
 warn as fail. Note that some (regular) piglit tests will also emit
 warn, and they don't mean fail. I believe that junit needs to be
 fixed to not interpret warn as fail irrespective of what happens in
 deqp. But for the record, I don't use junit.

 I'm not sure how I feel about the warn output in piglit in general,
 it seems a bit weird. Bogus and intermittent warnings certainly sound
 like a problem, and perhaps that's reason enough to just ignore them.

 I'm fine with either solution.

 Warn is kind of an odd status. Basically if a test passes, but there is
 anything unexpected in stderr, the test will get marked warn. (I think
 some other suites use it differently, maybe igt?)

 Well, with the GL suite, a test can on its own decide to return a
 warn status. The framework doesn't automatically add that in (or at
 least didn't before).

   -ilia

 And this is why warn status is problematic in a test suite.  Consider
 the case where a commit introduces a new warning.  Should a bug be
 written up?  It depends: as with compiler warnings, some projects may
 want to be more strict, and disallow warnings.  Other projects may not
 mind warnings, with the result that test writers emit warnings that are
 not actionable.

Defaults are important. The default is warn is ok. That's how I see
it from piglit. I'd do the split between warn and dmesg-warn. The
latter indicates a real issue, whereas warn indicates a highly
theoretical one. If you add a flag to treat warn as error optionally
(-Werror style), I certainly wouldn't object.


 At least with piglit the warnings are stable, so any CI displaying JUnit
 trends will not report spurious regressions.  Additionally, the
 developers can easily remedy warnings which are intermittent.  For this
 reason, warnings-as-errors provides a sharper edge with with to catch
 regressions in the core piglit test suite (caveat: I'm not sure how
 dmesg-warn is handled for tests executed in parallel).

 dEQP has 2 issues that work against using warnings as errors:

  * warnings are intermittent, at least on intel hardware.  Minimal
investigation by Chad indicated that the warnings are bogus.

  * developers don't have easy access to upstream, to improve the tests.

 There may be no optimal, consistent warning policy for both test suites.
 I'm inclined to turn down the warning noise emitted by dEQP, because it
 is external to piglit and can't be easily fixed.  Even at Google, dEQP
 runners blacklist thousands of tests because they are noisy.

 -Mark

If warns from deqp are useless, then those should also be disabled.
But that has no bearing on what junit does with warns...

  -ilia
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] Interpret dEQP QualityWarning as pass

2015-06-26 Thread Dylan Baker
[snip]
 regressions in the core piglit test suite (caveat: I'm not sure how
 dmesg-warn is handled for tests executed in parallel).

piglit forces serialization when dmesg checking is enabled.


signature.asc
Description: Digital signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] Interpret dEQP QualityWarning as pass

2015-06-26 Thread Ilia Mirkin
On Fri, Jun 26, 2015 at 2:44 PM, Dylan Baker baker.dyla...@gmail.com wrote:
 That's fine, but then that's the justification, not junit interprets
 warn as fail. Note that some (regular) piglit tests will also emit
 warn, and they don't mean fail. I believe that junit needs to be
 fixed to not interpret warn as fail irrespective of what happens in
 deqp. But for the record, I don't use junit.

 I'm not sure how I feel about the warn output in piglit in general,
 it seems a bit weird. Bogus and intermittent warnings certainly sound
 like a problem, and perhaps that's reason enough to just ignore them.

 I'm fine with either solution.

 Warn is kind of an odd status. Basically if a test passes, but there is
 anything unexpected in stderr, the test will get marked warn. (I think
 some other suites use it differently, maybe igt?)

Well, with the GL suite, a test can on its own decide to return a
warn status. The framework doesn't automatically add that in (or at
least didn't before).

  -ilia
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] Interpret dEQP QualityWarning as pass

2015-06-26 Thread Mark Janes
Ilia Mirkin imir...@alum.mit.edu writes:

 On Fri, Jun 26, 2015 at 2:44 PM, Dylan Baker baker.dyla...@gmail.com wrote:
 That's fine, but then that's the justification, not junit interprets
 warn as fail. Note that some (regular) piglit tests will also emit
 warn, and they don't mean fail. I believe that junit needs to be
 fixed to not interpret warn as fail irrespective of what happens in
 deqp. But for the record, I don't use junit.

 I'm not sure how I feel about the warn output in piglit in general,
 it seems a bit weird. Bogus and intermittent warnings certainly sound
 like a problem, and perhaps that's reason enough to just ignore them.

 I'm fine with either solution.

 Warn is kind of an odd status. Basically if a test passes, but there is
 anything unexpected in stderr, the test will get marked warn. (I think
 some other suites use it differently, maybe igt?)

 Well, with the GL suite, a test can on its own decide to return a
 warn status. The framework doesn't automatically add that in (or at
 least didn't before).

   -ilia

And this is why warn status is problematic in a test suite.  Consider
the case where a commit introduces a new warning.  Should a bug be
written up?  It depends: as with compiler warnings, some projects may
want to be more strict, and disallow warnings.  Other projects may not
mind warnings, with the result that test writers emit warnings that are
not actionable.

At least with piglit the warnings are stable, so any CI displaying JUnit
trends will not report spurious regressions.  Additionally, the
developers can easily remedy warnings which are intermittent.  For this
reason, warnings-as-errors provides a sharper edge with with to catch
regressions in the core piglit test suite (caveat: I'm not sure how
dmesg-warn is handled for tests executed in parallel).

dEQP has 2 issues that work against using warnings as errors:

 * warnings are intermittent, at least on intel hardware.  Minimal
   investigation by Chad indicated that the warnings are bogus.

 * developers don't have easy access to upstream, to improve the tests.

There may be no optimal, consistent warning policy for both test suites.
I'm inclined to turn down the warning noise emitted by dEQP, because it
is external to piglit and can't be easily fixed.  Even at Google, dEQP
runners blacklist thousands of tests because they are noisy.

-Mark
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] Interpret dEQP QualityWarning as pass

2015-06-25 Thread Mark Janes
Many tests in the functional.fbo.completeness.attachment_combinations
report QualityWarning intermittently.  This particular warning is
emitted because dEQP attempted to create an unsupported framebuffer.

warning is interpreted as failure in junit, but this output from
dEQP is clearly not a failure.
---
 framework/test/deqp.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/test/deqp.py b/framework/test/deqp.py
index 1462ca2..f4bf1cd 100644
--- a/framework/test/deqp.py
+++ b/framework/test/deqp.py
@@ -105,7 +105,7 @@ class DEQPBaseTest(Test):
 __metaclass__ = abc.ABCMeta
 __RESULT_MAP = {Pass: pass,
 Fail: fail,
-QualityWarning: warn,
+QualityWarning: pass,
 InternalError: fail,
 Crash: crash,
 NotSupported: skip}
-- 
2.1.4

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] Interpret dEQP QualityWarning as pass

2015-06-25 Thread Ilia Mirkin
On Thu, Jun 25, 2015 at 8:52 PM, Mark Janes mark.a.ja...@intel.com wrote:
 Many tests in the functional.fbo.completeness.attachment_combinations
 report QualityWarning intermittently.  This particular warning is
 emitted because dEQP attempted to create an unsupported framebuffer.

 warning is interpreted as failure in junit, but this output from
 dEQP is clearly not a failure.

So... fix junit. warn is not a failure output by piglit.

 ---
  framework/test/deqp.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/framework/test/deqp.py b/framework/test/deqp.py
 index 1462ca2..f4bf1cd 100644
 --- a/framework/test/deqp.py
 +++ b/framework/test/deqp.py
 @@ -105,7 +105,7 @@ class DEQPBaseTest(Test):
  __metaclass__ = abc.ABCMeta
  __RESULT_MAP = {Pass: pass,
  Fail: fail,
 -QualityWarning: warn,
 +QualityWarning: pass,
  InternalError: fail,
  Crash: crash,
  NotSupported: skip}
 --
 2.1.4

 ___
 Piglit mailing list
 Piglit@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit