Re: [Piglit] [PATCH 2/2] framework/test/base.py: try to use subprocess32

2015-10-15 Thread Thomas Wood 
On 14 October 2015 at 22:35, Dylan Baker  wrote:
> On Wed, Oct 14, 2015 at 04:27:56PM +0100, Thomas Wood wrote:
>> On 9 October 2015 at 20:53,   wrote:
>> > From: Dylan Baker 
>> >
>> > This adds subprocess32 as an optional dependency to get timeouts. This
>> > module is a backport of the feature from python 3.2 that adds the
>> > timeout argument to Popen.communicate (among others).
>> >
>> > There is a caveat to this feature. Unlike the python 3.x version, this
>> > doesn't work on windows, although that isn't a change from the current
>> > state of affairs, as the current timeouts don't work on windows
>> >
>> > What this does do though, is get us timeouts that work with concurrency
>> > on posix systems (like Linux and OSX).
>> >
>> > Signed-off-by: Dylan Baker 
>> > ---
>> >  README|   5 +-
>> >  framework/test/base.py| 123 
>> > +-
>> >  framework/tests/base_tests.py |   2 +
>> >  tox.ini   |   1 +
>> >  4 files changed, 43 insertions(+), 88 deletions(-)
>> >
>> > diff --git a/README b/README
>> > index be8f4df..5813af5 100644
>> > --- a/README
>> > +++ b/README
>> > @@ -47,9 +47,12 @@ Optionally, you can install the following:
>> >- lxml. An accelerated python xml library using libxml2 
>> > (http://lxml.de/)
>> >- simplejson. A fast C based implementation of the python json library.
>> >  (https://simplejson.readthedocs.org/en/latest/)
>> > -  - backports.lzma. A packport of python3's lzma module to python2,
>> > +  - backports.lzma. A backport of python3's lzma module to python2,
>> >  this enables fast native xz (de)compression in piglit for results 
>> > files
>> >  (https://github.com/peterjc/backports.lzma)
>> > +  - subprocess32. A backport of python 3.2's subprocess module to python 
>> > 2.
>> > +This provides the timeout mechanism.
>> > +(https://pypi.python.org/pypi/subprocess32/)
>> >
>> >  Now configure the build system:
>> >
>> > diff --git a/framework/test/base.py b/framework/test/base.py
>> > index bf00396..0f01ccc 100644
>> > --- a/framework/test/base.py
>> > +++ b/framework/test/base.py
>> > @@ -29,13 +29,33 @@ import subprocess
>> >  import time
>> >  import sys
>> >  import traceback
>> > -from datetime import datetime
>> > -import threading
>> > -import signal
>> >  import itertools
>> >  import abc
>> >  import copy
>> >
>> > +try:
>> > +from subprocess32 import Popen, TimeoutExpired
>> > +except ImportError:
>> > +from subprocess import Popen as _Popen
>> > +
>> > +class Popen(_Popen):
>> > +"""Wrapper to provide timout on communicate.
>> > +
>> > +This allows us to use the same code without a plethora of if 
>> > statements
>> > +to special case windows an non subprocess32 cases.
>> > +
>> > +"""
>> > +def communicate(self, *args, **kwargs):
>> > +try:
>> > +del kwargs['timeout']
>> > +except KeyError:
>> > +pass
>> > +
>> > +super(Popen, self).communicate(*args, **kwargs)
>> > +
>> > +class TimeoutExpired(Exception):
>> > +pass
>> > +
>> >  from framework import exceptions
>> >  from framework.core import Options
>> >  from framework.results import TestResult
>> > @@ -62,56 +82,6 @@ class TestRunError(exceptions.PiglitException):
>> >  self.status = status
>> >
>> >
>> > -class ProcessTimeout(threading.Thread):
>> > -""" Timeout class for test processes
>> > -
>> > -This class is for terminating tests that run for longer than a certain
>> > -amount of time. Create an instance by passing it a timeout in seconds
>> > -and the Popen object that is running your test. Then call the start
>> > -method (inherited from Thread) to start the timer. The Popen object is
>> > -killed if the timeout is reached and it has not completed. Wait for 
>> > the
>> > -outcome by calling the join() method from the parent.
>> > -
>> > -"""
>> > -
>> > -def __init__(self, timeout, proc):
>> > -threading.Thread.__init__(self)
>> > -self.proc = proc
>> > -self.timeout = timeout
>> > -self.status = 0
>> > -
>> > -def run(self):
>> > -start_time = datetime.now()
>> > -delta = 0
>> > -
>> > -# poll() returns the returncode attribute, which is either the 
>> > return
>> > -# code of the child process (which could be zero), or None if the
>> > -# process has not yet terminated.
>> > -
>> > -while delta < self.timeout and self.proc.poll() is None:
>> > -time.sleep(1)
>> > -delta = (datetime.now() - start_time).total_seconds()
>> > -
>> > -# if the test is not finished after timeout, first try to 
>> > terminate it
>> > -# and if that fails, send SIGKILL to all processes in the test's
>> > -

Re: [Piglit] [PATCH] ext_texture_format_bgra8888: Add test for GL_BGRA format in Tex(Sub)Image calls

2015-10-15 Thread Jason Ekstrand 
On Wed, Oct 14, 2015 at 6:50 PM, Eduardo Lima Mitev  wrote:
> This is a new test that checks valid and invalid combinations of GL_BGRA_EXT
> format and internal format in Tex(Sub)Image2D calls, as specified by the
> EXT_texture_format_BGRA extension
> .
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92265
> ---
>  tests/all.py   |   6 ++
>  tests/spec/CMakeLists.txt  |   1 +
>  .../CMakeLists.gles2.txt   |   7 ++
>  .../ext_texture_format_bgra/CMakeLists.txt |   1 +
>  tests/spec/ext_texture_format_bgra/teximage.c  | 103 
> +
>  5 files changed, 118 insertions(+)
>  create mode 100644 
> tests/spec/ext_texture_format_bgra/CMakeLists.gles2.txt
>  create mode 100644 tests/spec/ext_texture_format_bgra/CMakeLists.txt
>  create mode 100644 tests/spec/ext_texture_format_bgra/teximage.c
>
> diff --git a/tests/all.py b/tests/all.py
> index 610d89f..144b51e 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -2226,6 +2226,12 @@ with profile.group_manager(
>  g(['arb_occlusion_query2-api'], 'api')
>  g(['arb_occlusion_query2-render'], 'render')
>
> +# Group EXT_texture_format_BGRA tests
> +with profile.group_manager(
> +PiglitGLTest,
> +grouptools.join('spec', 'EXT_texture_format_BGRA')) as g:
> +g(['ext_texture_format_bgra-teximage'], 'teximage')
> +
>  with profile.group_manager(
>  PiglitGLTest,
>  grouptools.join('spec', 'ARB_pixel_buffer_object')) as g:
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index 5dc37a1..bf22680 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -133,3 +133,4 @@ add_subdirectory (arb_texture_stencil8)
>  add_subdirectory (arb_vertex_attrib_64bit)
>  add_subdirectory (ext_framebuffer_blit)
>  add_subdirectory (mesa_pack_invert)
> +add_subdirectory (ext_texture_format_bgra)
> diff --git a/tests/spec/ext_texture_format_bgra/CMakeLists.gles2.txt 
> b/tests/spec/ext_texture_format_bgra/CMakeLists.gles2.txt
> new file mode 100644
> index 000..38edd70
> --- /dev/null
> +++ b/tests/spec/ext_texture_format_bgra/CMakeLists.gles2.txt
> @@ -0,0 +1,7 @@
> +link_libraries (
> +   piglitutil_${piglit_target_api}
> +)
> +
> +piglit_add_executable (ext_texture_format_bgra-teximage teximage.c)
> +
> +# vim: ft=cmake:
> diff --git a/tests/spec/ext_texture_format_bgra/CMakeLists.txt 
> b/tests/spec/ext_texture_format_bgra/CMakeLists.txt
> new file mode 100644
> index 000..144a306
> --- /dev/null
> +++ b/tests/spec/ext_texture_format_bgra/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git a/tests/spec/ext_texture_format_bgra/teximage.c 
> b/tests/spec/ext_texture_format_bgra/teximage.c
> new file mode 100644
> index 000..cb40db2
> --- /dev/null
> +++ b/tests/spec/ext_texture_format_bgra/teximage.c

Let's call this api-errors.c or something like that.  It really isn't
testing TexImage.  We should probably eventually have a test that
tests that the texture actually gets on the screen.  However, I'm not
too terribly worried about it since, as long as we don't error out, we
*probably* do the right thing there.

With it re-named to api-errors,

Reviewed-by: Jason Ekstrand 

> @@ -0,0 +1,103 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +/**
> + * @file
> + * Tests glTex(Sub)Image functions for valid and invalid combinations of
> + * GL_BGRA_EXT format and internal format, as defined by the extension
> + * EXT_texture_format_BGRA888.
> + */
> +
> +#include "piglit-util-gl.h"
> +
>

[Piglit] [PATCH] Add new arb_draw_buffers_blend-state_set_get test

2015-10-15 Thread Brian Paul 
Test glBlendFuncSeparate, glBlendEquationSeparate, and the indexed
versions to make sure all state is set properly.
---
 tests/all.py   |   1 +
 tests/spec/CMakeLists.txt  |   1 +
 .../spec/arb_draw_buffers_blend/CMakeLists.gl.txt  |  13 ++
 tests/spec/arb_draw_buffers_blend/CMakeLists.txt   |   1 +
 tests/spec/arb_draw_buffers_blend/state_set_get.c  | 228 +
 5 files changed, 244 insertions(+)
 create mode 100644 tests/spec/arb_draw_buffers_blend/CMakeLists.gl.txt
 create mode 100644 tests/spec/arb_draw_buffers_blend/CMakeLists.txt
 create mode 100644 tests/spec/arb_draw_buffers_blend/state_set_get.c

diff --git a/tests/all.py b/tests/all.py
index 4531b01..fff4403 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -3954,6 +3954,7 @@ with profile.group_manager(
 with profile.group_manager(
 PiglitGLTest,
 grouptools.join('spec', 'arb_draw_buffers_blend')) as g:
+g(['arb_draw_buffers_blend-state_set_get'])
 g(['fbo-draw-buffers-blend'], run_concurrent=False)
 
 with profile.group_manager(
diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
index 5dc37a1..34e5260 100644
--- a/tests/spec/CMakeLists.txt
+++ b/tests/spec/CMakeLists.txt
@@ -118,6 +118,7 @@ add_subdirectory (arb_vertex_type_2_10_10_10_rev)
 add_subdirectory (ext_texture_array)
 add_subdirectory (ext_texture_integer)
 add_subdirectory (arb_draw_buffers)
+add_subdirectory (arb_draw_buffers_blend)
 add_subdirectory (oes_draw_texture)
 add_subdirectory (oes_fixed_point)
 add_subdirectory (ext_image_dma_buf_import)
diff --git a/tests/spec/arb_draw_buffers_blend/CMakeLists.gl.txt 
b/tests/spec/arb_draw_buffers_blend/CMakeLists.gl.txt
new file mode 100644
index 000..784886f
--- /dev/null
+++ b/tests/spec/arb_draw_buffers_blend/CMakeLists.gl.txt
@@ -0,0 +1,13 @@
+include_directories(
+   ${GLEXT_INCLUDE_DIR}
+   ${OPENGL_INCLUDE_PATH}
+)
+
+link_libraries (
+   piglitutil_${piglit_target_api}
+   ${OPENGL_gl_LIBRARY}
+)
+
+piglit_add_executable (arb_draw_buffers_blend-state_set_get state_set_get.c)
+
+# vim: ft=cmake:
diff --git a/tests/spec/arb_draw_buffers_blend/CMakeLists.txt 
b/tests/spec/arb_draw_buffers_blend/CMakeLists.txt
new file mode 100644
index 000..144a306
--- /dev/null
+++ b/tests/spec/arb_draw_buffers_blend/CMakeLists.txt
@@ -0,0 +1 @@
+piglit_include_target_api()
diff --git a/tests/spec/arb_draw_buffers_blend/state_set_get.c 
b/tests/spec/arb_draw_buffers_blend/state_set_get.c
new file mode 100644
index 000..a1bb35e
--- /dev/null
+++ b/tests/spec/arb_draw_buffers_blend/state_set_get.c
@@ -0,0 +1,228 @@
+/*
+ * Copyright 2015 VMware, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+/**
+ * Test setting/getting state related to GL_ARB_draw_buffers_blend.
+ * In particular, make sure glBlendFunc and glBlendEquation updates
+ * all buffer state.
+ *
+ * Brian Paul
+ * October 2015
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+   config.supports_gl_compat_version = 20;
+   config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGB;
+PIGLIT_GL_TEST_CONFIG_END
+
+static GLint num_buffers;
+
+struct blend_state
+{
+   GLenum srcRGB, srcA, dstRGB, dstA;
+   GLenum eqRGB, eqA;
+};
+
+#define MAX_BUFFERS 16
+
+static struct blend_state state[MAX_BUFFERS];
+
+
+static void
+set_state(int buffer,
+ GLenum srcRGB, GLenum srcA,
+ GLenum dstRGB, GLenum dstA,
+ GLenum eqRGB, GLenum eqA)
+{
+   state[buffer].srcRGB = srcRGB;
+   state[buffer].srcA = srcA;
+   state[buffer].dstRGB = dstRGB;
+   state[buffer].dstA = dstA;
+   state[buffer].eqRGB = eqRGB;
+   state[buffer].eqA = eqA;
+
+   glBlendFuncSeparateiARB(buffer, srcRGB, dstRGB, srcA, dstA);
+   glBlendEquationSeparateiARB(buffer, eqRGB, eqA);
+
+

[Piglit] [PATCH] Add new arb_draw_buffers_blend-state_set_get test

2015-10-15 Thread Brian Paul 
Test glBlendFuncSeparate, glBlendEquationSeparate, and the indexed
versions to make sure all state is set properly.

v2: also test display list compile/execute which exposed a mistake in
Mesa's save_BlendFunci().
---
 tests/all.py   |   1 +
 tests/spec/CMakeLists.txt  |   1 +
 .../spec/arb_draw_buffers_blend/CMakeLists.gl.txt  |  13 +
 tests/spec/arb_draw_buffers_blend/CMakeLists.txt   |   1 +
 tests/spec/arb_draw_buffers_blend/state_set_get.c  | 290 +
 5 files changed, 306 insertions(+)
 create mode 100644 tests/spec/arb_draw_buffers_blend/CMakeLists.gl.txt
 create mode 100644 tests/spec/arb_draw_buffers_blend/CMakeLists.txt
 create mode 100644 tests/spec/arb_draw_buffers_blend/state_set_get.c

diff --git a/tests/all.py b/tests/all.py
index 4531b01..fff4403 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -3954,6 +3954,7 @@ with profile.group_manager(
 with profile.group_manager(
 PiglitGLTest,
 grouptools.join('spec', 'arb_draw_buffers_blend')) as g:
+g(['arb_draw_buffers_blend-state_set_get'])
 g(['fbo-draw-buffers-blend'], run_concurrent=False)
 
 with profile.group_manager(
diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
index 5dc37a1..34e5260 100644
--- a/tests/spec/CMakeLists.txt
+++ b/tests/spec/CMakeLists.txt
@@ -118,6 +118,7 @@ add_subdirectory (arb_vertex_type_2_10_10_10_rev)
 add_subdirectory (ext_texture_array)
 add_subdirectory (ext_texture_integer)
 add_subdirectory (arb_draw_buffers)
+add_subdirectory (arb_draw_buffers_blend)
 add_subdirectory (oes_draw_texture)
 add_subdirectory (oes_fixed_point)
 add_subdirectory (ext_image_dma_buf_import)
diff --git a/tests/spec/arb_draw_buffers_blend/CMakeLists.gl.txt 
b/tests/spec/arb_draw_buffers_blend/CMakeLists.gl.txt
new file mode 100644
index 000..784886f
--- /dev/null
+++ b/tests/spec/arb_draw_buffers_blend/CMakeLists.gl.txt
@@ -0,0 +1,13 @@
+include_directories(
+   ${GLEXT_INCLUDE_DIR}
+   ${OPENGL_INCLUDE_PATH}
+)
+
+link_libraries (
+   piglitutil_${piglit_target_api}
+   ${OPENGL_gl_LIBRARY}
+)
+
+piglit_add_executable (arb_draw_buffers_blend-state_set_get state_set_get.c)
+
+# vim: ft=cmake:
diff --git a/tests/spec/arb_draw_buffers_blend/CMakeLists.txt 
b/tests/spec/arb_draw_buffers_blend/CMakeLists.txt
new file mode 100644
index 000..144a306
--- /dev/null
+++ b/tests/spec/arb_draw_buffers_blend/CMakeLists.txt
@@ -0,0 +1 @@
+piglit_include_target_api()
diff --git a/tests/spec/arb_draw_buffers_blend/state_set_get.c 
b/tests/spec/arb_draw_buffers_blend/state_set_get.c
new file mode 100644
index 000..4c95734
--- /dev/null
+++ b/tests/spec/arb_draw_buffers_blend/state_set_get.c
@@ -0,0 +1,290 @@
+/*
+ * Copyright 2015 VMware, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+/**
+ * Test setting/getting state related to GL_ARB_draw_buffers_blend.
+ * In particular, make sure glBlendFunc and glBlendEquation updates
+ * all buffer state.
+ *
+ * Brian Paul
+ * October 2015
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+   config.supports_gl_compat_version = 20;
+   config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGB;
+PIGLIT_GL_TEST_CONFIG_END
+
+static GLint num_buffers;
+
+struct blend_state
+{
+   GLenum srcRGB, srcA, dstRGB, dstA;
+   GLenum eqRGB, eqA;
+};
+
+#define MAX_BUFFERS 16
+
+static struct blend_state state[MAX_BUFFERS];
+
+static bool test_dlist = false;
+
+
+static void
+set_state(int buffer,
+ GLenum srcRGB, GLenum srcA,
+ GLenum dstRGB, GLenum dstA,
+ GLenum eqRGB, GLenum eqA)
+{
+   GLuint list = 0;
+
+   state[buffer].srcRGB = srcRGB;
+   state[buffer].srcA = srcA;
+   state[buffer].dstRGB = dstRGB;
+   state[buffer].dstA = dstA;
+   state[buffer].eqRGB = eqRGB;
+   state[buffer].eqA = eqA;
+
+

Re: [Piglit] Allow junit to be used for summary generation

2015-10-15 Thread Dylan Baker 
On Fri, Oct 09, 2015 at 10:46:08AM -0700, Dylan Baker wrote:
> On Thu, Oct 08, 2015 at 06:09:16PM -0700, Mark Janes wrote:
> > baker.dyla...@gmail.com writes:
> > 
> > > This series updates the junit backend to allow it to properly load junit
> > > and convert it back into piglit's internal representation, thus allowing
> > > it to be summarized using the piglit summary tools
> > >
> > > There is still some work that needs to be done beyond this, most of the
> > > platform metadata isn't stored yet and restored, but I have a plan for
> > > that. I have some other refactoring work that I think will make that
> > > easier, and I'd like to get there before landing that.
> > >
> > > This is enough to be able to compare junit and json results using the
> > > console and html summaries.
> > 
> > I don't have a use case for comparing junit and json.  If anyone tried
> > to compare json to our junit, they would see lots of differences because
> > json does not respect the "expected-failures" filter.  That filter will
> > also be unusable for json, because of the same test name character
> > conversion.
> > 
> > > There is a caveat here, and that's patch 3. To compare json and junit we
> > > need to be able to restore the names of the junit tests to *exactly*
> > > what they were before, and currently we don't have a way to reverse the
> > > '.' -> '_' conversion. My proposal is to change '.' into '___', which is
> > > very unlikely in a real test name (though we could change it to almost
> > > anything that would be unique). This may break some existing setup
> > > (Mark, I think this will probably break some of our expected fail/crash
> > > data).
> > 
> > It seems to me that it will be simpler for everyone to disallow
> > junit/json comparisons.  I just need a way for users to visualize a
> > dozen junit test files for disparate platforms and test suites.
> 
> 
> In the mean time, how do you feel about patches 1 and 2, or at least 2?
> 
> Getting 2 landed would allow junit to be summarized correctly right now,
> and one would give more accurate timestamps (if people care about
> those).
> 
> Dylan

ping?


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

[Piglit] [PATCH v2] arb_texture_view: Add test for 2d layer as 2d

2015-10-15 Thread Glenn Kennard 
Test that layers other than 0 from a 2d layer texture
can be used in a 2d texture view.

Signed-off-by: Glenn Kennard 
---
 tests/all.py   |   2 +
 tests/spec/arb_texture_view/CMakeLists.gl.txt  |   1 +
 .../sampling-2d-array-as-2d-layer.c| 195 +
 3 files changed, 198 insertions(+)
 create mode 100644 tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c

diff --git a/tests/all.py b/tests/all.py
index 610d89f..738cec2 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -2451,6 +2451,8 @@ with profile.group_manager(
   'sampling-2d-array-as-cubemap')
 g(['arb_texture_view-sampling-2d-array-as-cubemap-array'],
   'sampling-2d-array-as-cubemap-array')
+g(['arb_texture_view-sampling-2d-array-as-2d-layer'],
+  'sampling-2d-array-as-2d-layer')
 
 with profile.group_manager(
 PiglitGLTest,
diff --git a/tests/spec/arb_texture_view/CMakeLists.gl.txt 
b/tests/spec/arb_texture_view/CMakeLists.gl.txt
index 197731a..772f8b4 100644
--- a/tests/spec/arb_texture_view/CMakeLists.gl.txt
+++ b/tests/spec/arb_texture_view/CMakeLists.gl.txt
@@ -23,6 +23,7 @@ piglit_add_executable(arb_texture_view-rendering-formats 
rendering-formats.c)
 piglit_add_executable(arb_texture_view-rendering-layers rendering_layers.c 
common.c)
 piglit_add_executable(arb_texture_view-rendering-levels rendering_levels.c 
common.c)
 piglit_add_executable(arb_texture_view-rendering-target rendering_target.c 
common.c)
+piglit_add_executable(arb_texture_view-sampling-2d-array-as-2d-layer 
sampling-2d-array-as-2d-layer.c)
 piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap-array 
sampling-2d-array-as-cubemap-array.c)
 piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap 
sampling-2d-array-as-cubemap.c)
 piglit_add_executable(arb_texture_view-targets targets.c common.c)
diff --git a/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c 
b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
new file mode 100644
index 000..65074d2
--- /dev/null
+++ b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
@@ -0,0 +1,195 @@
+/*
+ * Copyright © 2015 Glenn Kennard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Author: Glenn Kennard 
+ */
+
+/**
+ * \file sampling-2d-array-as-2d-layer.c
+ * This tests that you can cast from a 2D Array texture to a regular 2D texture
+ * with layer>0 and sample from the latter.
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+   config.supports_gl_compat_version = 30;
+// config.supports_gl_core_version = 32;
+   config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DOUBLE;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+static const float green[] = {0, 1, 0, 1};
+static const float red[] = {1, 0, 0, 1};
+
+typedef struct Params {
+   int num_layers;
+   int width;
+   int height;
+   const char * const desc;
+} Params;
+
+/* test a few size combinations that tend to require particular alignment
+   requirements by the hardware */
+static const Params testparams[] = {
+   { 8, 1, 1, "1x1" },
+   { 3, 2, 1, "2x1" },
+   { 3, 8, 1, "8x1" },
+   { 1, 16, 1, "16x1" },
+   { 5, 1, 16, "1x16" },
+   { 9, 32, 32, "32x32" },
+   { 2, 64, 64, "64x64" },
+   { 4, 128, 64, "128x64" },
+   { 3, 35, 67, "35x67" }
+};
+
+static float *makesolidimage(int w, int h, const float color[4])
+{
+   float *p = malloc(w * h * 4 * sizeof(GLfloat));
+   size_t n;
+   assert(p);
+   for (n = 0; n < w * h; n++) {
+   p[n*4 + 0] = color[0];
+   p[n*4 + 1] = color[1];
+   p[n*4 + 2] = color[2];
+   p[n*4 + 3] = color[3];
+   }
+   return p;
+}
+
+static bool
+test_single_layer(const Params* p, int layer)
+{
+   int l;
+   GLuint

Re: [Piglit] [PATCH 1/3] framework/backends/junit.py: restore time for stderr

2015-10-15 Thread Mark Janes 
baker.dyla...@gmail.com writes:

> From: Dylan Baker 
>
> This allows time to be fully restored.
> ---
>  framework/backends/junit.py | 12 
>  framework/tests/junit_backends_tests.py | 22 +-
>  2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
> index 11fb09e..ce6a27a 100644
> --- a/framework/backends/junit.py
> +++ b/framework/backends/junit.py
> @@ -255,6 +255,9 @@ def _load(results_file):
>  name = name[:-1]
>  
>  result.result = test.attrib['status']
> +
> +# This is the fallback path, we'll try to overwrite this with the 
> value
> +# in stderr
>  result.time = results.TimeAttribute(end=float(test.attrib['time']))
>  result.err = test.find('system-err').text
>  

I noticed that a few lines above this hunk, there is this:

# Take the class name minus the 'piglit.' element, replace junit's '.'
# separator with piglit's separator, and join the group and test names

The transformation is not reversible, eg:

test/foo.bar/baz  -> test.foo.bar.baz -> test/foo/bar/baz

This doesn't affect my use case, though.

Reviewed-by: Mark Janes 

> @@ -264,6 +267,15 @@ def _load(results_file):
>  result.command = out[0]
>  result.out = '\n'.join(out[1:])
>  
> +# Try to get the values in stderr for time
> +if 'time start' in result.err:
> +for line in result.err.split('\n'):
> +if line.startswith('time start:'):
> +result.time.start = float(line[len('time start: '):])
> +elif line.startswith('time end:'):
> +result.time.end = float(line[len('time end: '):])
> +break
> +
>  run_result.tests[name] = result
>  
>  return run_result
> diff --git a/framework/tests/junit_backends_tests.py 
> b/framework/tests/junit_backends_tests.py
> index 96335f3..1a4be0e 100644
> --- a/framework/tests/junit_backends_tests.py
> +++ b/framework/tests/junit_backends_tests.py
> @@ -47,7 +47,11 @@ _XML = """\
>  
> time="1.12345">
>  this/is/a/command\nThis is stdout
> -this is stderr
> +this is stderr
> +
> +time start: 1.0
> +time end: 4.5
> +
>
>  
>
> @@ -234,11 +238,17 @@ class TestJUnitLoad(utils.StaticDirectory):
>  nt.assert_is_instance(self.xml().tests[self.testname].result,
>status.Status)
>  
> -def test_time(self):
> -"""backends.junit._load: Time is loaded correctly."""
> +def test_time_start(self):
> +"""backends.junit._load: Time.start is loaded correctly."""
>  time = self.xml().tests[self.testname].time
>  nt.assert_is_instance(time, results.TimeAttribute)
> -nt.assert_equal(time.total, 1.12345)
> +nt.eq_(time.start, 1.0)
> +
> +def test_time_end(self):
> +"""backends.junit._load: Time.end is loaded correctly."""
> +time = self.xml().tests[self.testname].time
> +nt.assert_is_instance(time, results.TimeAttribute)
> +nt.eq_(time.end, 4.5)
>  
>  def test_command(self):
>  """backends.junit._load: command is loaded correctly."""
> @@ -253,7 +263,9 @@ class TestJUnitLoad(utils.StaticDirectory):
>  def test_err(self):
>  """backends.junit._load: stderr is loaded correctly."""
>  test = self.xml().tests[self.testname].err
> -nt.assert_equal(test, 'this is stderr')
> +nt.eq_(
> +test, 'this is stderr\n\ntime start: 1.0\ntime end: 4.5\n
> ')
> +
>  
>  @utils.no_error
>  def test_load_file(self):
> -- 
> 2.6.1
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 2/3] framework/backends/junit.py: add calculate_group_totals() to load

2015-10-15 Thread Mark Janes 
Reviewed-by: Mark Janes 

baker.dyla...@gmail.com writes:

> From: Dylan Baker 
>
> This calculates the group totals on load (this is stored in the json),
> which is required to get proper totals information.
> ---
>  framework/backends/junit.py | 2 ++
>  framework/tests/junit_backends_tests.py | 4 
>  2 files changed, 6 insertions(+)
>
> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
> index ce6a27a..15c6c0b 100644
> --- a/framework/backends/junit.py
> +++ b/framework/backends/junit.py
> @@ -278,6 +278,8 @@ def _load(results_file):
>  
>  run_result.tests[name] = result
>  
> +run_result.calculate_group_totals()
> +
>  return run_result
>  
>  
> diff --git a/framework/tests/junit_backends_tests.py 
> b/framework/tests/junit_backends_tests.py
> index 1a4be0e..7d5a3fc 100644
> --- a/framework/tests/junit_backends_tests.py
> +++ b/framework/tests/junit_backends_tests.py
> @@ -266,6 +266,10 @@ class TestJUnitLoad(utils.StaticDirectory):
>  nt.eq_(
>  test, 'this is stderr\n\ntime start: 1.0\ntime end: 4.5\n
> ')
>  
> +def test_totals(self):
> +"""backends.junit._load: Totals are calculated."""
> +nt.ok_(bool(self.xml()))
> +
>  
>  @utils.no_error
>  def test_load_file(self):
> -- 
> 2.6.1
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH v2] arb_texture_view: Add test for 2d layer as 2d

2015-10-15 Thread Ilia Mirkin 
Reviewed-by: Ilia Mirkin 

On Thu, Oct 15, 2015 at 7:10 PM, Glenn Kennard  wrote:
> Test that layers other than 0 from a 2d layer texture
> can be used in a 2d texture view.
>
> Signed-off-by: Glenn Kennard 
> ---
>  tests/all.py   |   2 +
>  tests/spec/arb_texture_view/CMakeLists.gl.txt  |   1 +
>  .../sampling-2d-array-as-2d-layer.c| 195 
> +
>  3 files changed, 198 insertions(+)
>  create mode 100644 
> tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
>
> diff --git a/tests/all.py b/tests/all.py
> index 610d89f..738cec2 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -2451,6 +2451,8 @@ with profile.group_manager(
>'sampling-2d-array-as-cubemap')
>  g(['arb_texture_view-sampling-2d-array-as-cubemap-array'],
>'sampling-2d-array-as-cubemap-array')
> +g(['arb_texture_view-sampling-2d-array-as-2d-layer'],
> +  'sampling-2d-array-as-2d-layer')
>
>  with profile.group_manager(
>  PiglitGLTest,
> diff --git a/tests/spec/arb_texture_view/CMakeLists.gl.txt 
> b/tests/spec/arb_texture_view/CMakeLists.gl.txt
> index 197731a..772f8b4 100644
> --- a/tests/spec/arb_texture_view/CMakeLists.gl.txt
> +++ b/tests/spec/arb_texture_view/CMakeLists.gl.txt
> @@ -23,6 +23,7 @@ piglit_add_executable(arb_texture_view-rendering-formats 
> rendering-formats.c)
>  piglit_add_executable(arb_texture_view-rendering-layers rendering_layers.c 
> common.c)
>  piglit_add_executable(arb_texture_view-rendering-levels rendering_levels.c 
> common.c)
>  piglit_add_executable(arb_texture_view-rendering-target rendering_target.c 
> common.c)
> +piglit_add_executable(arb_texture_view-sampling-2d-array-as-2d-layer 
> sampling-2d-array-as-2d-layer.c)
>  piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap-array 
> sampling-2d-array-as-cubemap-array.c)
>  piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap 
> sampling-2d-array-as-cubemap.c)
>  piglit_add_executable(arb_texture_view-targets targets.c common.c)
> diff --git a/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c 
> b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
> new file mode 100644
> index 000..65074d2
> --- /dev/null
> +++ b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
> @@ -0,0 +1,195 @@
> +/*
> + * Copyright © 2015 Glenn Kennard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Author: Glenn Kennard 
> + */
> +
> +/**
> + * \file sampling-2d-array-as-2d-layer.c
> + * This tests that you can cast from a 2D Array texture to a regular 2D 
> texture
> + * with layer>0 and sample from the latter.
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +   config.supports_gl_compat_version = 30;
> +// config.supports_gl_core_version = 32;
> +   config.window_visual = PIGLIT_GL_VISUAL_RGBA | 
> PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const float green[] = {0, 1, 0, 1};
> +static const float red[] = {1, 0, 0, 1};
> +
> +typedef struct Params {
> +   int num_layers;
> +   int width;
> +   int height;
> +   const char * const desc;
> +} Params;
> +
> +/* test a few size combinations that tend to require particular alignment
> +   requirements by the hardware */
> +static const Params testparams[] = {
> +   { 8, 1, 1, "1x1" },
> +   { 3, 2, 1, "2x1" },
> +   { 3, 8, 1, "8x1" },
> +   { 1, 16, 1, "16x1" },
> +   { 5, 1, 16, "1x16" },
> +   { 9, 32, 32, "32x32" },
> +   { 2, 64, 64, "64x64" },
> +   { 4, 128, 64, "128x64" },
> +   { 3, 35, 67, "35x67" }
> +};
> +
> +static float *makesolidimage(int w, int h, const float color[4])
> +{
> +   float *p = malloc(w * h * 4

[Piglit] [PATCH 2/2] generated_tests/test_generators.py: Add support for expected failures

2015-10-15 Thread baker . dylan . c 
From: Dylan Baker 

There are a number of tests with expected failures that there are
patches for, but have not been reviewed or landed. This in the meantime
makes tox turn green, which is what we expect.

Signed-off-by: Dylan Baker 
---
 generated_tests/test_generators.py | 33 +++--
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/generated_tests/test_generators.py 
b/generated_tests/test_generators.py
index 23da4cb..5f38b10 100644
--- a/generated_tests/test_generators.py
+++ b/generated_tests/test_generators.py
@@ -30,6 +30,7 @@ from __future__ import absolute_import, division, 
print_function
 import os
 import subprocess
 import functools
+import sys
 
 import nose.tools as nt
 
@@ -44,6 +45,16 @@ BLACKLIST = 
set([os.path.abspath(os.path.join(os.path.dirname(__file__), _p))
  for _p in BLACKLIST])
 
 
+# This dict holds the name of a generator expected to fail, and then a value of
+# True or False (True if it is expected to fail. This allows us to have
+# condtional failures, like when we expect python3 to fail.
+EXPECTED_FAILS = {
+'gen_tes_input_tests.py': sys.version.startswith('3'),
+'gen_tcs_input_tests.py': sys.version.startswith('3'),
+}
+
+
+
 def discover_generators():
 """Discover all of the generators and return that as a set.
 
@@ -121,22 +132,32 @@ def nose_generator(func):
 def test_generators():
 """Generate tests for the various generators."""
 
-def test(name):
-"""Tester function."""
+def test(name, fail=False):
+"""Tester function.
+
+If fail is true the the test is expected to Fail
+
+"""
 msg = ''
+rcode = 0
 
 try:
 with open(os.devnull, 'w') as d:
 rcode = subprocess.check_call(['python', name], stderr=d,
   stdout=d)
 except subprocess.CalledProcessError as e:
-msg = "While calling {}:\n{}".format(name, str(e))
-rcode = e.returncode
+if not fail:
+msg = "While calling {}:\n{}".format(name, str(e))
+rcode = e.returncode
+else:
+if fail:
+raise AssertionError("Expected failure for {} not encountered")
 
 nt.eq_(rcode, 0, msg)
 
 description = 'generator: {} runs successfully'
 
 for generator in discover_generators():
-test.description = description.format(os.path.basename(generator))
-yield test, generator
+name = os.path.basename(generator)
+test.description = description.format(name)
+yield test, generator, EXPECTED_FAILS.get(name, False)
-- 
2.6.1

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

[Piglit] [PATCH 1/2] tox.ini: run generator tests with python 2.7 as well

2015-10-15 Thread baker . dylan . c 
From: Dylan Baker 

Signed-off-by: Dylan Baker 
---
 tox.ini | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tox.ini b/tox.ini
index ca97a51..7aa741a 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,5 +1,5 @@
 [tox]
-envlist = py27-{noaccel,accel},py{33,34,35}
+envlist = py27-{noaccel,accel},py{27,33,34,35}
 skipsdist=True
 
 [testenv]
-- 
2.6.1

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

Re: [Piglit] [PATCH v2] arb_texture_view: Add test for 2d layer as 2d

2015-10-15 Thread Emil Velikov 
Hi Glenn,

On 16 October 2015 at 00:10, Glenn Kennard  wrote:
> Test that layers other than 0 from a 2d layer texture
> can be used in a 2d texture view.
>
> Signed-off-by: Glenn Kennard 
> ---
>  tests/all.py   |   2 +
>  tests/spec/arb_texture_view/CMakeLists.gl.txt  |   1 +
>  .../sampling-2d-array-as-2d-layer.c| 195 
> +
>  3 files changed, 198 insertions(+)
>  create mode 100644 
> tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
>
> diff --git a/tests/all.py b/tests/all.py
> index 610d89f..738cec2 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -2451,6 +2451,8 @@ with profile.group_manager(
>'sampling-2d-array-as-cubemap')
>  g(['arb_texture_view-sampling-2d-array-as-cubemap-array'],
>'sampling-2d-array-as-cubemap-array')
> +g(['arb_texture_view-sampling-2d-array-as-2d-layer'],
> +  'sampling-2d-array-as-2d-layer')
>
>  with profile.group_manager(
>  PiglitGLTest,
> diff --git a/tests/spec/arb_texture_view/CMakeLists.gl.txt 
> b/tests/spec/arb_texture_view/CMakeLists.gl.txt
> index 197731a..772f8b4 100644
> --- a/tests/spec/arb_texture_view/CMakeLists.gl.txt
> +++ b/tests/spec/arb_texture_view/CMakeLists.gl.txt
> @@ -23,6 +23,7 @@ piglit_add_executable(arb_texture_view-rendering-formats 
> rendering-formats.c)
>  piglit_add_executable(arb_texture_view-rendering-layers rendering_layers.c 
> common.c)
>  piglit_add_executable(arb_texture_view-rendering-levels rendering_levels.c 
> common.c)
>  piglit_add_executable(arb_texture_view-rendering-target rendering_target.c 
> common.c)
> +piglit_add_executable(arb_texture_view-sampling-2d-array-as-2d-layer 
> sampling-2d-array-as-2d-layer.c)
>  piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap-array 
> sampling-2d-array-as-cubemap-array.c)
>  piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap 
> sampling-2d-array-as-cubemap.c)
>  piglit_add_executable(arb_texture_view-targets targets.c common.c)
> diff --git a/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c 
> b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
> new file mode 100644
> index 000..65074d2
> --- /dev/null
> +++ b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
> @@ -0,0 +1,195 @@
> +/*
> + * Copyright © 2015 Glenn Kennard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Author: Glenn Kennard 
> + */
> +
> +/**
> + * \file sampling-2d-array-as-2d-layer.c
> + * This tests that you can cast from a 2D Array texture to a regular 2D 
> texture
> + * with layer>0 and sample from the latter.
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +   config.supports_gl_compat_version = 30;
> +// config.supports_gl_core_version = 32;
> +   config.window_visual = PIGLIT_GL_VISUAL_RGBA | 
> PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const float green[] = {0, 1, 0, 1};
> +static const float red[] = {1, 0, 0, 1};
> +
> +typedef struct Params {
> +   int num_layers;
> +   int width;
> +   int height;
> +   const char * const desc;
> +} Params;
> +
Bikeshed: Drop the typedef ?

> +/* test a few size combinations that tend to require particular alignment
> +   requirements by the hardware */
> +static const Params testparams[] = {
> +   { 8, 1, 1, "1x1" },
> +   { 3, 2, 1, "2x1" },
> +   { 3, 8, 1, "8x1" },
> +   { 1, 16, 1, "16x1" },
> +   { 5, 1, 16, "1x16" },
> +   { 9, 32, 32, "32x32" },
> +   { 2, 64, 64, "64x64" },
> +   { 4, 128, 64, "128x64" },
> +   { 3, 35, 67, "35x67" }
> +};
> +
> +static float *makesolidimage(int w, int h, const float color[4])
> +{
> +   float *p = malloc(w * h * 4 *

Re: [Piglit] [PATCH v2] arb_texture_view: Add test for 2d layer as 2d

2015-10-15 Thread Emil Velikov 
On 16 October 2015 at 01:14, Ilia Mirkin  wrote:
> On Thu, Oct 15, 2015 at 8:07 PM, Emil Velikov  
> wrote:
>> Hi Glenn,
>>
>> On 16 October 2015 at 00:10, Glenn Kennard  wrote:
[snip]
>>> +   subtest_pass &= test_single_layer([n], 
>>> layer);
>> subtest_pass = test_foo() && subtest_pass;
>>
>>> +   }
>>> +   piglit_report_subtest_result(subtest_pass ? PIGLIT_PASS : 
>>> PIGLIT_FAIL,
>>> +   testparams[n].desc);
>>> +   pass &= subtest_pass;
>> pass = subtest_pass && pass;
>>
>> Without these we might bail on the remaining tests, if one fails.
>
> Nope. && short-circuits, & doesn't.
Indeed it doesn't. Not sure what I was smoking earlier.

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

Re: [Piglit] [PATCH v2] arb_texture_view: Add test for 2d layer as 2d

2015-10-15 Thread Ilia Mirkin 
On Thu, Oct 15, 2015 at 8:07 PM, Emil Velikov  wrote:
> Hi Glenn,
>
> On 16 October 2015 at 00:10, Glenn Kennard  wrote:
>> Test that layers other than 0 from a 2d layer texture
>> can be used in a 2d texture view.
>>
>> Signed-off-by: Glenn Kennard 
>> ---
>>  tests/all.py   |   2 +
>>  tests/spec/arb_texture_view/CMakeLists.gl.txt  |   1 +
>>  .../sampling-2d-array-as-2d-layer.c| 195 
>> +
>>  3 files changed, 198 insertions(+)
>>  create mode 100644 
>> tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
>>
>> diff --git a/tests/all.py b/tests/all.py
>> index 610d89f..738cec2 100644
>> --- a/tests/all.py
>> +++ b/tests/all.py
>> @@ -2451,6 +2451,8 @@ with profile.group_manager(
>>'sampling-2d-array-as-cubemap')
>>  g(['arb_texture_view-sampling-2d-array-as-cubemap-array'],
>>'sampling-2d-array-as-cubemap-array')
>> +g(['arb_texture_view-sampling-2d-array-as-2d-layer'],
>> +  'sampling-2d-array-as-2d-layer')
>>
>>  with profile.group_manager(
>>  PiglitGLTest,
>> diff --git a/tests/spec/arb_texture_view/CMakeLists.gl.txt 
>> b/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> index 197731a..772f8b4 100644
>> --- a/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> +++ b/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> @@ -23,6 +23,7 @@ piglit_add_executable(arb_texture_view-rendering-formats 
>> rendering-formats.c)
>>  piglit_add_executable(arb_texture_view-rendering-layers rendering_layers.c 
>> common.c)
>>  piglit_add_executable(arb_texture_view-rendering-levels rendering_levels.c 
>> common.c)
>>  piglit_add_executable(arb_texture_view-rendering-target rendering_target.c 
>> common.c)
>> +piglit_add_executable(arb_texture_view-sampling-2d-array-as-2d-layer 
>> sampling-2d-array-as-2d-layer.c)
>>  piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap-array 
>> sampling-2d-array-as-cubemap-array.c)
>>  piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap 
>> sampling-2d-array-as-cubemap.c)
>>  piglit_add_executable(arb_texture_view-targets targets.c common.c)
>> diff --git a/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c 
>> b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
>> new file mode 100644
>> index 000..65074d2
>> --- /dev/null
>> +++ b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
>> @@ -0,0 +1,195 @@
>> +/*
>> + * Copyright © 2015 Glenn Kennard
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>> DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Author: Glenn Kennard 
>> + */
>> +
>> +/**
>> + * \file sampling-2d-array-as-2d-layer.c
>> + * This tests that you can cast from a 2D Array texture to a regular 2D 
>> texture
>> + * with layer>0 and sample from the latter.
>> + */
>> +
>> +#include "piglit-util-gl.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +   config.supports_gl_compat_version = 30;
>> +// config.supports_gl_core_version = 32;
>> +   config.window_visual = PIGLIT_GL_VISUAL_RGBA | 
>> PIGLIT_GL_VISUAL_DOUBLE;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +static const float green[] = {0, 1, 0, 1};
>> +static const float red[] = {1, 0, 0, 1};
>> +
>> +typedef struct Params {
>> +   int num_layers;
>> +   int width;
>> +   int height;
>> +   const char * const desc;
>> +} Params;
>> +
> Bikeshed: Drop the typedef ?
>
>> +/* test a few size combinations that tend to require particular alignment
>> +   requirements by the hardware */
>> +static const Params testparams[] = {
>> +   { 8, 1, 1, "1x1" },
>> +   { 3, 2, 1, "2x1" },
>> +   { 3, 8, 1, "8x1" },
>> +   { 1, 16, 1, "16x1" },
>> +   { 5, 1, 16, "1x16" },
>> +   { 9, 32, 32, "32x32" },
>> +   { 2, 64,

[Piglit] [PATCH] arb_texture_view: Add test for 2d layer as 2d

2015-10-15 Thread Glenn Kennard 
Test that layers other than 0 from a 2d layer texture
can be used in a 2d texture view.

Signed-off-by: Glenn Kennard 
---
Test passes on i965/hsw, nvc0, r600 (see patch on mesa-dev) but fails on
llvmpipe/softpipe

 tests/all.py   |   4 +-
 tests/spec/arb_texture_view/CMakeLists.gl.txt  |   1 +
 .../sampling-2d-array-as-2d-layer.c| 202 +
 3 files changed, 206 insertions(+), 1 deletion(-)
 create mode 100644 tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c

diff --git a/tests/all.py b/tests/all.py
index 610d89f..a07c841 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -2450,7 +2450,9 @@ with profile.group_manager(
 g(['arb_texture_view-sampling-2d-array-as-cubemap'],
   'sampling-2d-array-as-cubemap')
 g(['arb_texture_view-sampling-2d-array-as-cubemap-array'],
-  'sampling-2d-array-as-cubemap-array')
+  'sampling-2d-array-as-cubemap-array'),
+g(['arb_texture_view-sampling-2d-array-as-2d-layer'],
+  'sampling-2d-array-as-2d-layer')
 
 with profile.group_manager(
 PiglitGLTest,
diff --git a/tests/spec/arb_texture_view/CMakeLists.gl.txt 
b/tests/spec/arb_texture_view/CMakeLists.gl.txt
index 197731a..772f8b4 100644
--- a/tests/spec/arb_texture_view/CMakeLists.gl.txt
+++ b/tests/spec/arb_texture_view/CMakeLists.gl.txt
@@ -23,6 +23,7 @@ piglit_add_executable(arb_texture_view-rendering-formats 
rendering-formats.c)
 piglit_add_executable(arb_texture_view-rendering-layers rendering_layers.c 
common.c)
 piglit_add_executable(arb_texture_view-rendering-levels rendering_levels.c 
common.c)
 piglit_add_executable(arb_texture_view-rendering-target rendering_target.c 
common.c)
+piglit_add_executable(arb_texture_view-sampling-2d-array-as-2d-layer 
sampling-2d-array-as-2d-layer.c)
 piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap-array 
sampling-2d-array-as-cubemap-array.c)
 piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap 
sampling-2d-array-as-cubemap.c)
 piglit_add_executable(arb_texture_view-targets targets.c common.c)
diff --git a/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c 
b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
new file mode 100644
index 000..ee8df13
--- /dev/null
+++ b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
@@ -0,0 +1,202 @@
+/*
+ * Copyright © 2015 Glenn Kennard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Author: Glenn Kennard 
+ */
+
+/**
+ * \file sampling-2d-array-as-2d-layer.c
+ * This tests that you can cast from a 2D Array texture to a regular 2D texture
+ * with layer>0 and sample from the latter.
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+   config.supports_gl_compat_version = 30;
+// config.supports_gl_core_version = 32;
+   config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DOUBLE;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+static const float green[] = {0, 1, 0, 1};
+static const float red[] = {1, 0, 0, 1};
+
+typedef struct Params {
+   int num_layers;
+   int width;
+   int height;
+   const char * const desc;
+} Params;
+
+/* test a few size combinations that tend to require particular alignment
+   requirements by the hardware */
+static const Params testparams[] = {
+   { 8, 1, 1, "1x1" },
+   { 3, 2, 1, "2x1" },
+   { 3, 8, 1, "8x1" },
+   { 1, 16, 1, "16x1" },
+   { 5, 1, 16, "1x16" },
+   { 9, 32, 32, "32x32" },
+   { 2, 64, 64, "64x64" },
+   { 4, 64, 64, "128x64" },
+   { 3, 35, 67, "35x67" }
+};
+
+static float *makesolidimage(int w, int h, const float color[4])
+{
+   float *p = malloc(w * h * 4 * sizeof(GLfloat));
+   size_t n;
+   assert(p);
+   for (n = 0; n < w * h; n++) {
+   p[n*4 + 0] = color[0];
+   p[n*4 + 1] =

Re: [Piglit] [PATCH] arb_texture_view: Add test for 2d layer as 2d

2015-10-15 Thread Ilia Mirkin 
On Thu, Oct 15, 2015 at 6:30 PM, Glenn Kennard  wrote:
> Test that layers other than 0 from a 2d layer texture
> can be used in a 2d texture view.
>
> Signed-off-by: Glenn Kennard 
> ---
> Test passes on i965/hsw, nvc0, r600 (see patch on mesa-dev) but fails on
> llvmpipe/softpipe
>
>  tests/all.py   |   4 +-
>  tests/spec/arb_texture_view/CMakeLists.gl.txt  |   1 +
>  .../sampling-2d-array-as-2d-layer.c| 202 
> +
>  3 files changed, 206 insertions(+), 1 deletion(-)
>  create mode 100644 
> tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
>
> diff --git a/tests/all.py b/tests/all.py
> index 610d89f..a07c841 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -2450,7 +2450,9 @@ with profile.group_manager(
>  g(['arb_texture_view-sampling-2d-array-as-cubemap'],
>'sampling-2d-array-as-cubemap')
>  g(['arb_texture_view-sampling-2d-array-as-cubemap-array'],
> -  'sampling-2d-array-as-cubemap-array')
> +  'sampling-2d-array-as-cubemap-array'),

Stray ,

> +g(['arb_texture_view-sampling-2d-array-as-2d-layer'],
> +  'sampling-2d-array-as-2d-layer')
>
>  with profile.group_manager(
>  PiglitGLTest,
> diff --git a/tests/spec/arb_texture_view/CMakeLists.gl.txt 
> b/tests/spec/arb_texture_view/CMakeLists.gl.txt
> index 197731a..772f8b4 100644
> --- a/tests/spec/arb_texture_view/CMakeLists.gl.txt
> +++ b/tests/spec/arb_texture_view/CMakeLists.gl.txt
> @@ -23,6 +23,7 @@ piglit_add_executable(arb_texture_view-rendering-formats 
> rendering-formats.c)
>  piglit_add_executable(arb_texture_view-rendering-layers rendering_layers.c 
> common.c)
>  piglit_add_executable(arb_texture_view-rendering-levels rendering_levels.c 
> common.c)
>  piglit_add_executable(arb_texture_view-rendering-target rendering_target.c 
> common.c)
> +piglit_add_executable(arb_texture_view-sampling-2d-array-as-2d-layer 
> sampling-2d-array-as-2d-layer.c)
>  piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap-array 
> sampling-2d-array-as-cubemap-array.c)
>  piglit_add_executable(arb_texture_view-sampling-2d-array-as-cubemap 
> sampling-2d-array-as-cubemap.c)
>  piglit_add_executable(arb_texture_view-targets targets.c common.c)
> diff --git a/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c 
> b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
> new file mode 100644
> index 000..ee8df13
> --- /dev/null
> +++ b/tests/spec/arb_texture_view/sampling-2d-array-as-2d-layer.c
> @@ -0,0 +1,202 @@
> +/*
> + * Copyright © 2015 Glenn Kennard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Author: Glenn Kennard 
> + */
> +
> +/**
> + * \file sampling-2d-array-as-2d-layer.c
> + * This tests that you can cast from a 2D Array texture to a regular 2D 
> texture
> + * with layer>0 and sample from the latter.
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +   config.supports_gl_compat_version = 30;
> +// config.supports_gl_core_version = 32;
> +   config.window_visual = PIGLIT_GL_VISUAL_RGBA | 
> PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const float green[] = {0, 1, 0, 1};
> +static const float red[] = {1, 0, 0, 1};
> +
> +typedef struct Params {
> +   int num_layers;
> +   int width;
> +   int height;
> +   const char * const desc;
> +} Params;
> +
> +/* test a few size combinations that tend to require particular alignment
> +   requirements by the hardware */
> +static const Params testparams[] = {
> +   { 8, 1, 1, "1x1" },
> +   { 3, 2, 1, "2x1" },
> +   { 3, 8, 1, "8x1" },
> +   { 1, 16, 1, "16x1" },
> +   { 5, 1, 16, "1x16" },
> +   { 9, 32, 32, "32x32" },
> +   { 2, 64, 64, "64x64" },
> +

Re: [Piglit] [PATCH] ext_texture_format_bgra8888: Add test for GL_BGRA format in Tex(Sub)Image calls

2015-10-15 Thread Emil Velikov 
Hi Eduardo,

A few humble suggestions.

On 15 October 2015 at 02:50, Eduardo Lima Mitev  wrote:
> This is a new test that checks valid and invalid combinations of GL_BGRA_EXT
> format and internal format in Tex(Sub)Image2D calls, as specified by the
> EXT_texture_format_BGRA extension
> .
There is no need to provide a link to the spec in commit messages.

[snip]
> +static GLboolean
Please use bool.

> +run_test(void)
> +{
> +   GLuint tex;
> +
> +   glGenTextures(1, );
> +   glBindTexture(GL_TEXTURE_2D, tex);
> +   if (!piglit_check_gl_error(GL_NO_ERROR))
> +  return false;
> +
> +   /* glTexImage2D */
> +   glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT, 2, 2, 0, GL_BGRA_EXT, 
> GL_UNSIGNED_BYTE, NULL);
Wrap the last three arguments to the next line ? Here and below.

> +   if (!piglit_check_gl_error(GL_NO_ERROR))
> +  return false;
Most places in piglit continue running all (sub)tests, even if there
is a failure midway. One common pattern is

bool pass = true;

...

glFoo()
if (!piglit_check_gl_error...)
   pass = false;

...
return pass;


Cheers,
Emil

P.S. piglit tries to use tabs for indentation, albeit some tests use spaces.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH] ext_texture_format_bgra8888: Add test for GL_BGRA format in Tex(Sub)Image calls

2015-10-15 Thread Dylan Baker 
On Thu, Oct 15, 2015 at 06:25:26PM +0100, Emil Velikov wrote:
> Hi Eduardo,
> 
> A few humble suggestions.
> 
> On 15 October 2015 at 02:50, Eduardo Lima Mitev  wrote:
> > This is a new test that checks valid and invalid combinations of GL_BGRA_EXT
> > format and internal format in Tex(Sub)Image2D calls, as specified by the
> > EXT_texture_format_BGRA extension
> > .
> There is no need to provide a link to the spec in commit messages.
> 
> [snip]
> > +static GLboolean
> Please use bool.
> 
> > +run_test(void)
> > +{
> > +   GLuint tex;
> > +
> > +   glGenTextures(1, );
> > +   glBindTexture(GL_TEXTURE_2D, tex);
> > +   if (!piglit_check_gl_error(GL_NO_ERROR))
> > +  return false;
> > +
> > +   /* glTexImage2D */
> > +   glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT, 2, 2, 0, GL_BGRA_EXT, 
> > GL_UNSIGNED_BYTE, NULL);
> Wrap the last three arguments to the next line ? Here and below.
> 
> > +   if (!piglit_check_gl_error(GL_NO_ERROR))
> > +  return false;
> Most places in piglit continue running all (sub)tests, even if there
> is a failure midway. One common pattern is
> 
> bool pass = true;

Yes please. Not running all of the subtests (or at least returning
values for them) breaks assumptions made by the framework.

Dylan

> 
> ...
> 
> glFoo()
> if (!piglit_check_gl_error...)
>pass = false;
> 
> ...
> return pass;
> 
> 
> Cheers,
> Emil
> 
> P.S. piglit tries to use tabs for indentation, albeit some tests use spaces.
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit


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

Re: [Piglit] [PATCH 2/2] framework/test/base.py: try to use subprocess32

2015-10-15 Thread Dylan Baker 
On Thu, Oct 15, 2015 at 11:45:57AM +0100, Thomas Wood wrote:
> On 14 October 2015 at 22:35, Dylan Baker  wrote:
> > On Wed, Oct 14, 2015 at 04:27:56PM +0100, Thomas Wood wrote:
> >> On 9 October 2015 at 20:53,   wrote:
> >> > From: Dylan Baker 
> >>
> >> The process is not killed when the timeout occurs, so the previous
> >> code to implement that needs to be added here. This involves sending
> >> SIGTERM to the child process and then sending SIGKILL to the process
> >> group if SIGTERM failed to remove the process. The child processes
> >> were placed in their own process group by the preexec_fn parameter
> >> being set to __set_process_group.
> >>
> >
> > If you have subprocess32 it will be killed when the timeout expires.
> > This is a backport of the python3 behavior, from the python3 docs:
> >
> > """
> > The timeout argument is passed to Popen.communicate(). If the timeout
> > expires, the child process will be killed and waited for. The
> > TimeoutExpired exception will be re-raised after the child process has
> > terminated.
> > """
> >
> > Am I missing something here?
> 
> I think that is the documentation for subprocess.run, which has a
> slightly different behaviour to Popen.communicate:
> 
> https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate
> 
> "The child process is not killed if the timeout expires, so in order
> to cleanup properly a well-behaved application should kill the child
> process and finish communication:"
> 

The docs recommend using proc.kill() for this purpose.

I'm really hesitant to use preexec_fn, because there is a big warning
that preexec_fn is not safe to use with threads, and for the Open{G,C}L
tests having concurrency is a requirement, so if we require preexec_fn
we make timeouts and concurency mutually exclusive.

Looking at the docs there may be a way to handle process groups without
the use of a preexec_fn. I'll look into it further.

> 
> The additional step of sending SIGKILL to the process group also helps
> to clean up misbehaving tests that have forked.
> 

Do IGT tests regularly fork? I don't think any of the native piglit
tests fork.

[snip]


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