Re: [Piglit] [PATCH] Add test for external sampler with 2D tex

2019-02-02 Thread Mark Janes
FWIW, the skqp test is run constantly in mesa ci:

https://mesa-ci.01.org/mesa_master/builds/15138/results/174306816


Aditya Swarup  writes:

> Commit 649c149dbb96ac367 removed the test for intel external sampler
> with dma only because of failure due to commit a5c39ed974402c -
> ("i965: Lift restriction in external textures for EGLImage support").
> Since, there is no test coverage for external sampler with 2D tex,
> adding the test with modifications to pass for external sampler using
> EGLImage with 2D tex.
>
> Signed-off-by: Aditya Swarup 
> Cc: Tapani Pälli 
> Cc: Kenneth Graunke 
> ---
> This test case covers the scenario of binding a texture ID of type 
> GL_TEXTURE_EXTERNAL with EGLImage created using 2D texture. 
>
> This test is not covered in deqp but in CTS SKQP test - CtsSkQPTestCases 
> "#unitTest_EGLImageTest". Explanation in 
> https://bugs.freedesktop.org/show_bug.cgi?id=105301.
>
> It would be nice to have test coverage in piglit as well, so that we come to 
> know if this scenario is broken in near future. 
>
>  tests/opengl.py   |   2 +
>  .../CMakeLists.gles2.txt  |   1 +
>  .../intel_external_sampler_with_2D_tex.c  | 103 ++
>  3 files changed, 106 insertions(+)
>  create mode 100644 
> tests/spec/ext_image_dma_buf_import/intel_external_sampler_with_2D_tex.c
>
> diff --git a/tests/opengl.py b/tests/opengl.py
> index af68560bf..926af2036 100644
> --- a/tests/opengl.py
> +++ b/tests/opengl.py
> @@ -3017,6 +3017,8 @@ with profile.test_list.group_manager(
>  g(['ext_image_dma_buf_import-unsupported_format'], run_concurrent=False)
>  g(['ext_image_dma_buf_import-intel_external_sampler_only'],
>run_concurrent=False)
> +g(['ext_image_dma_buf_import-intel_external_sampler_with_2D_tex'],
> +  run_concurrent=False)
>  g(['ext_image_dma_buf_import-refcount'])
>  g(['ext_image_dma_buf_import-sample_rgb', '-fmt=AR24'],
>'ext_image_dma_buf_import-sample_argb', run_concurrent=False)
> diff --git a/tests/spec/ext_image_dma_buf_import/CMakeLists.gles2.txt 
> b/tests/spec/ext_image_dma_buf_import/CMakeLists.gles2.txt
> index f99a5d800..c22aeed73 100644
> --- a/tests/spec/ext_image_dma_buf_import/CMakeLists.gles2.txt
> +++ b/tests/spec/ext_image_dma_buf_import/CMakeLists.gles2.txt
> @@ -19,6 +19,7 @@ if(PIGLIT_BUILD_DMA_BUF_TESTS)
>   piglit_add_executable(ext_image_dma_buf_import-refcount refcount.c 
> sample_common.c image_common.c)
>   piglit_add_executable(ext_image_dma_buf_import-sample_yuv sample_yuv.c 
> sample_common.c image_common.c)
>   piglit_add_executable(ext_image_dma_buf_import-sample_rgb sample_rgb.c 
> sample_common.c image_common.c)
> + 
> piglit_add_executable(ext_image_dma_buf_import-intel_external_sampler_with_2D_tex
>  intel_external_sampler_with_2D_tex.c image_common.c)
>   
> piglit_add_executable(ext_image_dma_buf_import-transcode-nv12-as-r8-gr88 
> transcode-nv12-as-r8-gr88.c image_common.c)
>  endif()
>  
> diff --git 
> a/tests/spec/ext_image_dma_buf_import/intel_external_sampler_with_2D_tex.c 
> b/tests/spec/ext_image_dma_buf_import/intel_external_sampler_with_2D_tex.c
> new file mode 100644
> index 0..82e6a201e
> --- /dev/null
> +++ b/tests/spec/ext_image_dma_buf_import/intel_external_sampler_with_2D_tex.c
> @@ -0,0 +1,103 @@
> +/*
> + * Copyright © 2013 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.
> + */
> +
> +#include "image_common.h"
> +
> +/**
> + * @file intel_external_sampler_with_2D_tex.c
> + *
> + * Intel driver allows image external sampler to be used with regular 
> 2D-texutre
> + * and imported dma buffers. This test creates an egl image based on a 
> 2D-texture,
> + * attempts to use the image as target for an external texture, and should 
> pass.
> + *
> + */
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + 

Re: [Piglit] [PATCH v2] miptree: test ability to use upside down miptree

2018-12-03 Thread Mark Janes
It's preferable to push piglit tests first, and produce the CI test
failure.  CI staff will track the failure, and attribute it to the
piglit patch.  When the subsequent mesa patch is pushed, we track the
resolution.  The process gives us artifacts in git that help to analyze
driver differences between stable releases.

-Mark

andrey simiklit  writes:

> Hello,
>
> If this patch is acceptable by everybody then
> I guess that we need to push the following mesa patch before this one
> to avoid errors from Intel CI because the fix for an issue which is tested
> by this test is still there.
>
> https://patchwork.freedesktop.org/patch/254397/
> One of the last comment from Kenneth Graunke :
>
>>".
>> So, back to Reviewed-by.  I think once we get a Piglit test, I'm happy
>> to land this patch.
>>
>>--Ken"
>>
>
> Thanks,
> Andrii.
>
> On Mon, Dec 3, 2018 at 5:07 PM  wrote:
>
>> From: Andrii Simiklit 
>>
>> Test that usage of upside down miptree doesn't cause assertion
>>
>> The miptree:
>>
>> level 0 = 1x1
>> level 1 = 2x2
>> level 2 = 4x4
>> ...
>> level n = NxN
>>
>> should be acceptable for case when we don't use a min filter.
>>
>> v2: - Unnecessary function calls were removed
>> - The 'glClearColor' call was moved to 'piglit_display' function
>> - The program creation was moved to 'piglit_init' function
>> - The requirements check was moved to 'piglit_init' function
>>( Erik Faye-Lund  )
>>
>> - Fixed a leak of texture which is created in 'setup_texture'
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107987
>> Signed-off-by: Andrii Simiklit 
>> Reviewed-by: Erik Faye-Lund 
>> ---
>>  tests/opengl.py   |   1 +
>>  tests/texturing/CMakeLists.gl.txt |   1 +
>>  tests/texturing/tex-upside-down-miptree.c | 171 ++
>>  3 files changed, 173 insertions(+)
>>  create mode 100644 tests/texturing/tex-upside-down-miptree.c
>>
>> diff --git a/tests/opengl.py b/tests/opengl.py
>> index f7e408cd5..f6a38e40e 100644
>> --- a/tests/opengl.py
>> +++ b/tests/opengl.py
>> @@ -704,6 +704,7 @@ with profile.test_list.group_manager(
>>  g(['getteximage-targets', '1D'])
>>  g(['getteximage-targets', '2D'])
>>  g(['teximage-scale-bias'])
>> +g(['tex-upside-down-miptree'])
>>  add_msaa_visual_plain_tests(g, ['draw-pixels'])
>>  add_msaa_visual_plain_tests(g, ['read-front'], run_concurrent=False)
>>  add_msaa_visual_plain_tests(g, ['read-front', 'clear-front-first'],
>> diff --git a/tests/texturing/CMakeLists.gl.txt
>> b/tests/texturing/CMakeLists.gl.txt
>> index e5d41e432..02b572c79 100644
>> --- a/tests/texturing/CMakeLists.gl.txt
>> +++ b/tests/texturing/CMakeLists.gl.txt
>> @@ -98,5 +98,6 @@ piglit_add_executable (texture-al texture-al.c)
>>  piglit_add_executable (texture-rg texture-rg.c)
>>  piglit_add_executable (teximage-colors teximage-colors.c)
>>  piglit_add_executable (zero-tex-coord zero-tex-coord.c)
>> +piglit_add_executable (tex-upside-down-miptree tex-upside-down-miptree.c)
>>
>>  # vim: ft=cmake:
>> diff --git a/tests/texturing/tex-upside-down-miptree.c
>> b/tests/texturing/tex-upside-down-miptree.c
>> new file mode 100644
>> index 0..8e8f7154b
>> --- /dev/null
>> +++ b/tests/texturing/tex-upside-down-miptree.c
>> @@ -0,0 +1,171 @@
>> +/*
>> + * Copyright (c) 2018 Andrii Simiklit
>> + *
>> + * 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.
>> + *
>> + * Authors:
>> + *Andrii Simiklit 
>> + *
>> + */
>> +
>> +/**
>> + * Test what there no an assertion when we use upside down miptree and
>> + * GL_TEXTURE_MIN_FILTER is GL_LINEAR, base level is not 0
>> + * Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107987
>> + */
>> +
>> +#include "piglit-util-gl.h"
>> +#define TW 64
>> +#define TH 64
>> +
>> +PIGLIT_GL_TEST_CON

Re: [Piglit] [PATCH] arb_texture_view: Test interaction with ARB_shader_image_load_store

2018-11-16 Thread Mark Janes
Thanks for writing this test.  I checked it in CI and pushed it.

-Mark

Danylo Piliaiev  writes:

> Hello,
>
> Since the patch which fixes the issue got pushed in
> f9fd0cf4790cb2a530e75d1a2206dbb9d8af7cb2
> could this test be reviewed/pushed?
>
> Thanks!
>
> On 10/24/18 2:17 PM, Danylo Piliaiev wrote:
>> Tests that binding texture view to image unit results in a correct
>> calculation of layers:
>> - Correct layer should be read
>> - Image should have correct layers count
>>
>> Exercises the bug:
>> https://bugs.freedesktop.org/show_bug.cgi?id=107856
>>
>> Signed-off-by: Danylo Piliaiev 
>> ---
>>   tests/opengl.py   |   1 +
>>   tests/spec/arb_texture_view/CMakeLists.gl.txt |   1 +
>>   .../arb_texture_view/rendering-layers-image.c | 211 ++
>>   3 files changed, 213 insertions(+)
>>   create mode 100644 tests/spec/arb_texture_view/rendering-layers-image.c
>>
>> diff --git a/tests/opengl.py b/tests/opengl.py
>> index f7e408cd5..caa0d2813 100644
>> --- a/tests/opengl.py
>> +++ b/tests/opengl.py
>> @@ -2461,6 +2461,7 @@ with profile.test_list.group_manager(
>>   g(['arb_texture_view-rendering-target'], 'rendering-target')
>>   g(['arb_texture_view-rendering-levels'], 'rendering-levels')
>>   g(['arb_texture_view-rendering-layers'], 'rendering-layers')
>> +g(['arb_texture_view-rendering-layers-image'], 'rendering-layers-image')
>>   g(['arb_texture_view-rendering-formats'], 'rendering-formats')
>>   g(['arb_texture_view-rendering-r32ui'], 'rendering-r32ui')
>>   g(['arb_texture_view-lifetime-format'], 'lifetime-format')
>> diff --git a/tests/spec/arb_texture_view/CMakeLists.gl.txt 
>> b/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> index 39330dad7..eca0c18a5 100644
>> --- a/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> +++ b/tests/spec/arb_texture_view/CMakeLists.gl.txt
>> @@ -21,6 +21,7 @@ piglit_add_executable(arb_texture_view-mipgen mipgen.c)
>>   piglit_add_executable(arb_texture_view-params params.c)
>>   piglit_add_executable(arb_texture_view-queries queries.c)
>>   piglit_add_executable(arb_texture_view-rendering-formats 
>> rendering-formats.c)
>> +piglit_add_executable(arb_texture_view-rendering-layers-image 
>> rendering-layers-image.c common.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-r32ui rendering-r32ui.c)
>> diff --git a/tests/spec/arb_texture_view/rendering-layers-image.c 
>> b/tests/spec/arb_texture_view/rendering-layers-image.c
>> new file mode 100644
>> index 0..415b01657
>> --- /dev/null
>> +++ b/tests/spec/arb_texture_view/rendering-layers-image.c
>> @@ -0,0 +1,211 @@
>> +/* Copyright © 2018 Danylo Piliaiev
>> + *
>> + * 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.
>> + */
>> +
>> +/**
>> + * Tests GL_ARB_texture_view interaction with ARB_shader_image_load_store.
>> + * Creates texture maps with different solid colors for each layer,
>> + * reads the framebuffer to ensure the rendered color is correct
>> + * and verifies that image has expected layers count.
>> + */
>> +
>> +#include "piglit-util-gl.h"
>> +#include "common.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> +config.supports_gl_core_version = 32;
>> +
>> +config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DOUBLE;
>> +config.khr_no_error_support = PIGLIT_NO_ERRORS;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +struct test_info
>> +{
>> +GLenum target;
>> +const char* uniform_type;
>> +const char* img_layers_dimension;
>> +const char* img_access;
>> +int program;
>> +};
>> +
>> +struct test_info tests[] = {
>> +{GL_TEXTURE_1D_ARRAY, "image1DArray", "y", "ivec2(0, tex_layer)", -1

Re: [Piglit] [PATCH] vulkan: Prefix group names for VkRunner tests with “vulkan”

2018-11-13 Thread Mark Janes
Let me know when this is pushed, and I'll enable the tests in i965 CI.

Dylan Baker  writes:

> Quoting Neil Roberts (2018-11-13 12:50:16)
>> This will make it easier to distinguish tests written for Vulkan. It
>> makes a bit of an inconsistency because the GL and CL tests don\u2019t have
>> any prefix. Ideally maybe we would add a prefix for those too, but
>> changing the test names at this late stage would probably cause a lot
>> of hassle.
>> 
>> Adding the prefix will avoid problems if we eventually decide to merge
>> the Vulkan profile into the \u201cquick\u201d profile because otherwise we 
>> risk
>> having name collisions when the same thing is tested on both Vulkan
>> and GL.
>> ---
>>  tests/vulkan.py | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/vulkan.py b/tests/vulkan.py
>> index eed199c55..ae01677a9 100644
>> --- a/tests/vulkan.py
>> +++ b/tests/vulkan.py
>> @@ -21,7 +21,8 @@ profile = TestProfile()
>>  for basedir in [TESTS_DIR, GENERATED_TESTS_DIR]:
>>  _basedir = os.path.join(basedir, 'vulkan')
>>  for dirpath, _, filenames in os.walk(_basedir):
>> -groupname = grouptools.from_path(os.path.relpath(dirpath, _basedir))
>> +groupname = ('vulkan' + grouptools.SEPARATOR +
>> + grouptools.from_path(os.path.relpath(dirpath, 
>> _basedir)))
>
> There's a grouptools.join function for this:
>
> groupname = grouptools.join('vulkan', grouptools.from_path(...))
>
> With that change:
> Reviewed-by: Dylan Baker 
>
> Thanks for following up on this.
>
>>  dirname = os.path.relpath(dirpath, os.path.join(basedir, '..'))
>>  for filename in filenames:
>>  testname, ext = os.path.splitext(filename)
>> -- 
>> 2.17.1
>> 
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] vulkan: Prefix group names for VkRunner tests with “vulkan”

2018-11-13 Thread Mark Janes
Reviewed-by: Mark Janes 
Tested-by: Mark Janes 

Neil Roberts  writes:

> This will make it easier to distinguish tests written for Vulkan. It
> makes a bit of an inconsistency because the GL and CL tests don’t have
> any prefix. Ideally maybe we would add a prefix for those too, but
> changing the test names at this late stage would probably cause a lot
> of hassle.
>
> Adding the prefix will avoid problems if we eventually decide to merge
> the Vulkan profile into the “quick” profile because otherwise we risk
> having name collisions when the same thing is tested on both Vulkan
> and GL.
> ---
>  tests/vulkan.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/vulkan.py b/tests/vulkan.py
> index eed199c55..ae01677a9 100644
> --- a/tests/vulkan.py
> +++ b/tests/vulkan.py
> @@ -21,7 +21,8 @@ profile = TestProfile()
>  for basedir in [TESTS_DIR, GENERATED_TESTS_DIR]:
>  _basedir = os.path.join(basedir, 'vulkan')
>  for dirpath, _, filenames in os.walk(_basedir):
> -groupname = grouptools.from_path(os.path.relpath(dirpath, _basedir))
> +groupname = ('vulkan' + grouptools.SEPARATOR +
> + grouptools.from_path(os.path.relpath(dirpath, 
> _basedir)))
>  dirname = os.path.relpath(dirpath, os.path.join(basedir, '..'))
>  for filename in filenames:
>  testname, ext = os.path.splitext(filename)
> -- 
> 2.17.1
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH v1] opengl/intel/ppgtt: memory alignment test.

2018-10-24 Thread Mark Janes
Tested-by: Mark Janes 

Sergii Romantsov  writes:

> Intel ppgtt-model requires memory addresses to be aligned by page size.
> Test explores cases with cached memory-buckets and sizes above cached.
>
> CC: Kenneth Graunke 
> CC: Lionel G Landwerlin 
> CC: Chris Wilson 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106997
> Tests: 24839663a402 (intel/ppgtt: memory address alignment)
> Tests: a363bb2cd0e2 (i965: Allocate VMA in userspace for full-PPGTT systems.)
> Signed-off-by: Sergii Romantsov 
> ---
>  tests/general/CMakeLists.gl.txt|   1 +
>  tests/general/ppgtt_memory_alignment.c | 124 
> +
>  tests/opengl.py|   1 +
>  3 files changed, 126 insertions(+)
>  create mode 100644 tests/general/ppgtt_memory_alignment.c
>
> diff --git a/tests/general/CMakeLists.gl.txt b/tests/general/CMakeLists.gl.txt
> index 83189fc..7b43f84 100644
> --- a/tests/general/CMakeLists.gl.txt
> +++ b/tests/general/CMakeLists.gl.txt
> @@ -91,6 +91,7 @@ piglit_add_executable (polygon-mode-offset 
> polygon-mode-offset.c)
>  piglit_add_executable (polygon-mode-facing polygon-mode-facing.c)
>  piglit_add_executable (polygon-mode polygon-mode.c)
>  piglit_add_executable (polygon-offset polygon-offset.c)
> +piglit_add_executable (ppgtt_memory_alignment ppgtt_memory_alignment.c)
>  piglit_add_executable (primitive-restart primitive-restart.c)
>  piglit_add_executable (primitive-restart-draw-mode 
> primitive-restart-draw-mode.c)
>  piglit_add_executable (provoking-vertex provoking-vertex.c)
> diff --git a/tests/general/ppgtt_memory_alignment.c 
> b/tests/general/ppgtt_memory_alignment.c
> new file mode 100644
> index 000..b5db532
> --- /dev/null
> +++ b/tests/general/ppgtt_memory_alignment.c
> @@ -0,0 +1,124 @@
> +/*
> + * Copyright © 2018 Sergii Romantsov ()
> + *
> + * 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.
> + *
> + *  Created on: Oct 11, 2018
> + *  Author: Sergii Romantsov
> + *
> + *  Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106997
> + *  Tests: 24839663a402 (intel/ppgtt: memory address alignment)
> + *  Tests: a363bb2cd0e2 (i965: Allocate VMA in userspace for full-PPGTT 
> systems.)
> + */
> +
> +
> +#include "piglit-util-gl.h"
> +#include "unistd.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
> +
> + config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
> + config.requires_displayed_window = true;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static enum piglit_result g_result = PIGLIT_FAIL;
> +static GLuint g_cur_size = 0;
> +static GLubyte *g_buffer = NULL;
> +
> +static void
> +free_buffer(void)
> +{
> + if (g_buffer)
> + {
> + free(g_buffer);
> + g_buffer = NULL;
> + }
> +}
> +
> +static void
> +test_fail_check(void)
> +{
> + /* Intel: Function is needed to check fail-status of execution:
> +  * currently if batch-buffer wasn't submitted than Mesa will exit 
> application.
> +  * Otherwise piglit_display has logic to detect fail across call 
> glGetError.
> +  */
> + if (g_result != PIGLIT_PASS)
> + {
> + fprintf(stderr, "Test failed for buffer size: %x \n", 
> g_cur_size);
> + free_buffer();
> + piglit_report_result(g_result);
> + }
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + /* Maximal value of cache-size supported by d

Re: [Piglit] [PATCH] Cmake: Install shader_source files

2018-08-28 Thread Mark Janes
Reviewed-by: Mark Janes 
Tested-by: Mark Janes 

Dylan Baker  writes:

> Otherwise these tests fail in odd ways when running from an installed
> instance.
>
> Cc: Alejandro Piñeiro 
> Cc: Mark Janes 
> Cc: Clayton Craft 
> ---
>  CMakeLists.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 8f19457fc..596870b9b 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -537,7 +537,7 @@ install (
>  install (
>   DIRECTORY tests
>   DESTINATION ${PIGLIT_INSTALL_LIBDIR}
> - FILES_MATCHING REGEX 
> ".*\\.(xml|xml.gz|py|program_test|shader_test|frag|vert|geom|tesc|tese|comp|ktx|cl|txt|inc)$"
> + FILES_MATCHING REGEX 
> ".*\\.(xml|xml.gz|py|program_test|shader_test|shader_source|frag|vert|geom|tesc|tese|comp|ktx|cl|txt|inc)$"
>   REGEX 
> "CMakeFiles|CMakeLists|serializer.py|opengl.py|cl.py|quick_gl.py|glslparser.py|shader.py|quick_shader.py|no_error.py|llvmpipe_gl.py|sanity.py"
>  EXCLUDE
>  )
>  
> -- 
> 2.18.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/2] multisample-accuracy: Add an error margin for lit/unlit pixels

2018-07-20 Thread Mark Janes
Hi Neil,

We still get intermittent failures with this threshold, and have
historically disabled the multisample accuracy tests in CI.

Can we relax the threshold a bit more?

-Mark

Neil Roberts  writes:

> The unlit and totally_lit stats are supposed to count the pixels where
> either a primitive completely covers the pixel or no primitive touches
> it at all. However this is effectively only determined by checking
> whether any primitive has intersected one of the supersample positions
> of the reference image. It's completely possible for a primitive to
> intersect the pixel boundary but completely miss any of the
> supersample positions. The GL implementation is free to pick whatever
> multisample positions it wants within the pixel boundary so it's also
> possible for a primitive to intersect a multisample position but miss
> all of the supersample positions of the reference image. To cope with
> this this patch now allows a small margin of error for the pixels that
> are either fully lit or fully unlit.
>
> This was causing the test to give false negatives for i965 with the
> 16x MSAA positions because in that case it chooses positions that are
> exactly on the pixel boundary and these would be outside of the grid
> of positions chosen by the supersampling in the reference image. The
> positions used are actually those described by the D3D API so it is
> possible that other hardware would have the same problem.
> ---
>  tests/spec/ext_framebuffer_multisample/common.cpp | 43 
> +--
>  1 file changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/tests/spec/ext_framebuffer_multisample/common.cpp 
> b/tests/spec/ext_framebuffer_multisample/common.cpp
> index d7be84f..fb42cc5 100644
> --- a/tests/spec/ext_framebuffer_multisample/common.cpp
> +++ b/tests/spec/ext_framebuffer_multisample/common.cpp
> @@ -530,15 +530,6 @@ Test::measure_accuracy()
>   }
>   }
>  
> - printf("Pixels that should be unlit\n");
> - unlit_stats.summarize();
> - pass = unlit_stats.is_perfect() && pass;
> - printf("Pixels that should be totally lit\n");
> - totally_lit_stats.summarize();
> - pass = totally_lit_stats.is_perfect() && pass;
> - printf("Pixels that should be partially lit\n");
> - partially_lit_stats.summarize();
> -
>   double error_threshold;
>   if (test_resolve) {
>   /* For depth and stencil resolves, the implementation
> @@ -558,7 +549,39 @@ Test::measure_accuracy()
>   error_threshold = 0.333 *
>   pow(0.6, log((double)effective_num_samples) / log(2.0));
>   }
> - printf("The error threshold for this test is %f\n", error_threshold);
> +
> + /* The unlit and totally_lit stats are supposed to count the
> +  * pixels where either a primitive completely covers the pixel
> +  * or no primitive touches it at all. However this is
> +  * effectively only determined by checking whether any
> +  * primitive has intersected one of the supersample positions
> +  * of the reference image. It's completely possible for a
> +  * primitive to intersect the pixel boundary but completely
> +  * miss any of the supersample positions. The GL
> +  * implementation is free to pick whatever multisample
> +  * positions it wants within the pixel boundary so it's also
> +  * possible for a primitive to intersect a multisample
> +  * position but miss all of the supersample positions of the
> +  * reference image. To cope with this we allow a small margin
> +  * of error for the pixels that are either fully lit or fully
> +  * unlit.
> +  */
> + double full_pixel_threshold = error_threshold * 0.05f;
> +
> + printf("Pixels that should be unlit\n");
> + unlit_stats.summarize();
> + pass = unlit_stats.is_better_than(full_pixel_threshold) && pass;
> + printf("Pixels that should be totally lit\n");
> + totally_lit_stats.summarize();
> + pass = totally_lit_stats.is_better_than(full_pixel_threshold) && pass;
> + printf("The error threshold for unlit and totally lit "
> +   "pixels test is %f\n",
> +   full_pixel_threshold);
> + printf("Pixels that should be partially lit\n");
> + partially_lit_stats.summarize();
> +
> + printf("The error threshold for partially lit pixels is %f\n",
> +   error_threshold);
>   pass = partially_lit_stats.is_better_than(error_threshold) && pass;
>   // TODO: deal with sRGB.
>   return pass;
> -- 
> 1.9.3
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 00/35] Serialize profiles into XML at build time

2018-04-10 Thread Mark Janes
Dylan Baker  writes:

> Quoting Marek Olšák (2018-04-10 14:22:10)
>> On Tue, Apr 10, 2018 at 2:15 PM, Dylan Baker  wrote:
>> 
>> Quoting Eric Anholt (2018-04-09 17:10:35)
>> > Marek Olšák  writes:
>> >
>> > > Is this use case affected?
>> > >
>> > > piglit run --deqp-mustpass-list --process-isolation 0 -p gbm -c quick
>> > > cts_gl45 deqp_gles2 deqp_gles3 deqp_gles31
>> > >
>> > > Yes, that is just 1 command to run all those test suites at the same
>> time.
>> > >
>> > > I use my personal "deqp" piglit branch that also disables process
>> isolation
>> > > for glcts and deqp, and parses the deqp mustpass lists which are in 
>> txt
>> > > files.
>> >
>> > Parsing the mustpass lists sounds really useful.  Trying to construct 
>> an
>> > appropriate command line otherwise has been quite a challenge.
>> 
>> That option is already in core piglit, you just need to configure your
>> piglit.conf appropriately. I guess we really should have that in the
>> piglit.conf.example file...
>> 
>> 
>> I doubt it. Why would I have these then:
>> 
>> https://cgit.freedesktop.org/~mareko/piglit/commit/?h=deqp&id=
>> 0b11344e0c18b9bf07ad12381b94f308f362eb88
>> https://cgit.freedesktop.org/~mareko/piglit/commit/?h=deqp&id=
>> b086d8f82d41338055ab48bdda78c4a0c1ee02d0
>> https://cgit.freedesktop.org/~mareko/piglit/commit/?h=deqp&id=
>> 90beefa825cda792eaa72bff2cefac463af6d08a
>> 
>> Marek
>
> Because in late 2016 (there is some internal stuff that happened so we stopped
> using piglit to run deqp I'm not entirely happy about) they changed the 
> mustpass
> list from xml to txt and no one ever updated it in master.

I'm not sure what you are referring to here.  We stopped using piglit
for dEQP because the run-time was 10X faster with a custom runner.  We
don't have process isolation anymore, but we recover from
crashes/assertions without paying the penalty of iterating the dEQP test
list for each test run.

Google dEQP authors recommended this path.

> Dylan
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] tests: Added a new GLES test profile for the Khronos CTS runner

2018-02-14 Thread Mark Janes
Mesa i965 CI doesn't use piglit anymore to run CTS, so we won't be
affected by this patch.

Andres Gomez  writes:

> OpenGL GLES*-CTS case lists were renamed to KHR-GLES* in the upstream
> repository.
>
> We want to keep the existing profiles so we are able to keep running
> the caselists from previous CTS releases and for branches in the open
> sourced repository created for API-specific release branches, as
> explained at:
> https://github.com/KhronosGroup/VK-GL-CTS/wiki/Contributing#branches
>
> Therefore, we add this new test profile to be able to run the renamed
> tests in the master branch of the opensourced Khronos CTS tests at:
> https://github.com/KhronosGroup/VK-GL-CTS
>
> Cc: Mark Janes 
> Cc: Dylan Baker 
> Cc: Juan A. Suarez Romero 
> Signed-off-by: Andres Gomez 
> ---
>  tests/khr_gles.py | 88 
> +++
>  1 file changed, 88 insertions(+)
>  create mode 100644 tests/khr_gles.py
>
> diff --git a/tests/khr_gles.py b/tests/khr_gles.py
> new file mode 100644
> index 0..59a0fe089
> --- /dev/null
> +++ b/tests/khr_gles.py
> @@ -0,0 +1,88 @@
> +# Copyright (c) 2017 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 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.
> +
> +"""Piglit integration for the now open sourced Khronos CTS tests being
> +developed at https://github.com/KhronosGroup/VK-GL-CTS
> +
> +By default this will run GLES2, GLES3, GLES31, GLES32, and GLESEXT
> +test cases. Those desiring to run only a subset of them should
> +consider using the -t or -x options to include or exclude tests.
> +
> +For example:
> +./piglit run khr_gles -c foo -t ES3- would run only ES3 tests (note
> +the dash to exclude ES31 tests)
> +
> +This integration requires some configuration in piglit.conf, or the
> +use of environment variables.
> +
> +In piglit.conf one should set the following:
> +[khr_gles]:bin -- Path to the glcts binary
> +[khr_gles]:extra_args -- any extra arguments to be passed to cts
> +(optional)
> +
> +Alternatively (or in addition, since environment variables have
> +precedence), one could set:
> +PIGLIT_KHR_GLES_BIN -- environment equivalent of [khr_gles]:bin
> +PIGLIT_KHR_GLES_EXTRA_ARGS -- environment equivalent of
> +[khr_gles]:extra_args
> +
> +"""
> +
> +from __future__ import (
> +absolute_import, division, print_function, unicode_literals
> +)
> +import itertools
> +
> +from framework.test import deqp
> +
> +__all__ = ['profile']
> +
> +_KHR_BIN = deqp.get_option('PIGLIT_KHR_GLES_BIN', ('khr_gles', 'bin'),
> +   required=True)
> +
> +_EXTRA_ARGS = deqp.get_option('PIGLIT_KHR_GLES_EXTRA_ARGS', ('khr_gles', 
> 'extra_args'),
> +  default='').split()
> +
> +
> +class DEQPKHRTest(deqp.DEQPBaseTest):
> +deqp_bin = _KHR_BIN
> +
> +@property
> +def extra_args(self):
> +return super(DEQPKHRTest, self).extra_args + \
> +[x for x in _EXTRA_ARGS if not x.startswith('--deqp-case')]
> +
> +
> +# Add all of the suites by default, users can use filters to remove them.
> +profile = deqp.make_profile(  # pylint: disable=invalid-name
> +itertools.chain(
> +deqp.iter_deqp_test_cases(
> +deqp.gen_caselist_txt(_KHR_BIN, 'KHR-GLES2-cases.txt', 
> _EXTRA_ARGS)),
> +deqp.iter_deqp_test_cases(
> +deqp.gen_caselist_txt(_KHR_BIN, 'KHR-GLES3-cases.txt', 
> _EXTRA_ARGS)),
> +deq

Re: [Piglit] [PATCH v2] tests: Added a couple of new test profiles for the Khronos CTS runner

2017-06-13 Thread Mark Janes
Tested-by: Mark Janes 

Andres Gomez  writes:

> After:
> https://github.com/KhronosGroup/VK-GL-CTS/commit/af8c22a343ee2c230488f6de71b36dc3070b2024
>
> OpenGL GL*-CTS case lists were renamed to KHR-GL*.
>
> We want to keep the existing profiles so we are able to keep running
> the caselists from previous CTS releases and for branches in the open
> sourced repository created for API-specific release branches, as
> explained at:
> https://github.com/KhronosGroup/VK-GL-CTS/wiki/Contributing#branches
>
> Therefore, we add these 2 new test profiles to be able to run the
> newly named tests in the master branch of the opensourced Khronos CTS
> tests at:
> https://github.com/KhronosGroup/VK-GL-CTS
>
> Signed-off-by: Andres Gomez 
> Cc: Mark Janes 
> Cc: Dylan Baker 
> Cc: Marek Olšák 
> Acked-by: Marek Olšák 
> Acked-by: Dylan Baker 
> ---
>  piglit.conf.example | 10 ++
>  tests/khr_gl.py | 93 
> +
>  tests/khr_gl45.py   | 69 +++
>  3 files changed, 172 insertions(+)
>  create mode 100644 tests/khr_gl.py
>  create mode 100644 tests/khr_gl45.py
>
> diff --git a/piglit.conf.example b/piglit.conf.example
> index 17f2329a0..ced807b80 100644
> --- a/piglit.conf.example
> +++ b/piglit.conf.example
> @@ -124,6 +124,16 @@ testB
>  ; overrides the value set here.
>  ;extra_args=--deqp-visibility hidden
>  
> +[khr_gl]
> +; path to the Khronos CTS OpenGL executable
> +; can be overwritten by PIGLIT_KHR_GL_BIN environment variable
> +;bin=/home/knuth/vk-gl-cts/build/external/openglcts/modules/glcts
> +
> +; Space-separated list of extra command line arguments for cts. The
> +; option is not required. The environment variable PIGLIT_KHR_GL_EXTRA_ARGS
> +; overrides the value set here.
> +;extra_args=--deqp-visibility hidden
> +
>  ; Section for specific oclconform test.  One of these sections is required 
> for
>  ; each test list in the oclconform section and must be called:
>  ; oclconform-$testname
> diff --git a/tests/khr_gl.py b/tests/khr_gl.py
> new file mode 100644
> index 0..0cc86afff
> --- /dev/null
> +++ b/tests/khr_gl.py
> @@ -0,0 +1,93 @@
> +# Copyright (c) 2017 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 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.
> +
> +"""Piglit integration for the now open sourced Khronos CTS tests being
> +developed at https://github.com/KhronosGroup/VK-GL-CTS
> +
> +By default this will run GL30, GL31, GL32, GL33, GL40, GL41, GL42, GL43, GL44
> +and GL45 test cases. Those desiring to run only a subset of them should 
> consider
> +using the -t or -x options to include or exclude tests.
> +
> +For example:
> +./piglit run khr_gl -c foo -t GL30- would run only GL30 tests
> +
> +This integration requires some configuration in piglit.conf, or the use of
> +environment variables.
> +
> +In piglit.conf one should set the following:
> +[khr_gl]:bin -- Path to the glcts binary
> +[khr_gl]:extra_args -- any extra arguments to be passed to cts (optional)
> +
> +Alternatively (or in addition, since environment variables have precedence),
> +one could set:
> +PIGLIT_KHR_GL_BIN -- environment equivalent of [khr_gl]:bin
> +PIGLIT_KHR_GL_EXTRA_ARGS -- environment equivalent of [khr_gl]:extra_args
> +
> +"""
> +
> +from __future__ import (
> +absolute_import, division, print_function, unicode_literals
> +)
> +import itertools
> +
> +from framework.test import deqp
> +
> +__all__ = ['profile']
> +
> +_KHR_BIN = deqp.get_option('PIGLIT_KHR_GL_BIN&#

Re: [Piglit] [PATCH 0/2] Rewind the changes to the Khronos GL CTS profiles

2017-06-06 Thread Mark Janes
Tested-by: Mark Janes 

Andres Gomez  writes:

> Recently I added some changes in the cts_gl* profiles to adapt to
> renames in the opensourced Khronos CTS at:
> https://github.com/KhronosGroup/VK-GL-CTS
>
> Unfortunately, I didn't foresee that this change would break backward
> compatibility with previous CTS releases and with branches created for
> API-specific release, as explained at:
> https://github.com/KhronosGroup/VK-GL-CTS/wiki/Contributing#branches
>
> Now, we revert the changes and provide 2 new profiles to use with the
> renamed tests.
>
> Andres Gomez (2):
>   Revert "tests: Update integration for khronos CTS runner."
>   tests: Added a couple of new test profies for the Khronos CTS runner
>
>  tests/cts_gl.py   | 20 ++--
>  tests/cts_gl45.py |  2 +-
>  tests/khr_gl.py   | 93 
> +++
>  tests/khr_gl45.py | 69 +
>  4 files changed, 173 insertions(+), 11 deletions(-)
>  create mode 100644 tests/khr_gl.py
>  create mode 100644 tests/khr_gl45.py
>
> -- 
> 2.11.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 6/8] fbo: Use renderbuffers instead of textures

2017-06-02 Thread Mark Janes
Hi Ian,

This test fails on g33.  Was that expected?

-Mark

Ian Romanick  writes:

> From: Ian Romanick 
>
> This allows fbo-blit-stretch to run on platforms that support
> ARB_framebuffer_object but not ARB_texture_non_power_of_two.
>
> Coneptually similar to 7b3f6d5.
>
> Signed-off-by: Ian Romanick 
> ---
>  tests/fbo/fbo-blit-stretch.cpp | 26 --
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/tests/fbo/fbo-blit-stretch.cpp b/tests/fbo/fbo-blit-stretch.cpp
> index 935d0d4..da85335 100644
> --- a/tests/fbo/fbo-blit-stretch.cpp
> +++ b/tests/fbo/fbo-blit-stretch.cpp
> @@ -321,22 +321,22 @@ run_test(const TestCase &test)
>  
>   GLboolean pass;
>  
> - GLuint tex;
> + GLuint rbo;
>   GLuint fbo;
>   GLenum status;
>  
>   glGenFramebuffers(1, &fbo);
>   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>  
> - tex = piglit_rgbw_texture(GL_RGBA, test.srcW, test.srcH, GL_FALSE, 
> GL_TRUE, GL_UNSIGNED_NORMALIZED);
> + glGenRenderbuffers(1, &rbo);
> + glBindRenderbuffer(GL_RENDERBUFFER, rbo);
> + glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA, test.srcW, test.srcH);
> + glBindRenderbuffer(GL_RENDERBUFFER, 0);
>  
> - glBindTexture(GL_TEXTURE_2D, tex);
> -
> - glFramebufferTexture2D(GL_FRAMEBUFFER,
> -GL_COLOR_ATTACHMENT0,
> -GL_TEXTURE_2D,
> -tex,
> -0);
> + glFramebufferRenderbuffer(GL_FRAMEBUFFER,
> +   GL_COLOR_ATTACHMENT0,
> +   GL_RENDERBUFFER,
> +   rbo);
>   if (!piglit_check_gl_error(GL_NO_ERROR))
>   piglit_report_result(PIGLIT_FAIL);
>  
> @@ -344,6 +344,12 @@ run_test(const TestCase &test)
>   if (status != GL_FRAMEBUFFER_COMPLETE) {
>   pass = GL_TRUE;
>   } else {
> + GLubyte *image = piglit_rgbw_image_ubyte(test.srcW, test.srcH,
> +  GL_TRUE);
> + glDrawPixels(test.srcW, test.srcH, GL_RGBA, GL_UNSIGNED_BYTE,
> +  image);
> + free(image);
> +
>   glViewport(0, 0, piglit_width, piglit_height);
>   piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
>  
> @@ -364,7 +370,7 @@ run_test(const TestCase &test)
>  
>   glBindFramebuffer(GL_FRAMEBUFFER, piglit_winsys_fbo);
>   glDeleteFramebuffers(1, &fbo);
> - glDeleteTextures(1, &tex);
> + glDeleteRenderbuffers(1, &rbo);
>  
>   return pass;
>  }
> -- 
> 2.9.4
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/2] shaders: set missing blue color to glsl-const-builtin-distance.shader_test

2017-05-22 Thread Mark Janes
Series is:

Tested-by: Mark Janes 


Samuel Pitoiset  writes:

> Since b741bc770 ("parser_utils: do not overwrite value when no
> digits are found "), the blue color has to be explicitly initialized
> to zero.
>
> Cc: Mark Janes 
> Signed-off-by: Samuel Pitoiset 
> ---
>  tests/shaders/glsl-const-builtin-distance.shader_test | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/shaders/glsl-const-builtin-distance.shader_test 
> b/tests/shaders/glsl-const-builtin-distance.shader_test
> index 460313c25..3ca9f8943 100644
> --- a/tests/shaders/glsl-const-builtin-distance.shader_test
> +++ b/tests/shaders/glsl-const-builtin-distance.shader_test
> @@ -19,4 +19,4 @@ void main()
>  
>  [test]
>  draw rect -1 -1 2 2
> -probe all rgb 0.73087868591526 0.73087868591526
> +probe all rgb 0.73087868591526 0.73087868591526 0.0
> -- 
> 2.13.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] getteximage-formats: Disable dithering during glDrawPixels

2017-01-30 Thread Mark Janes
Tested-by: Mark Janes 

Chad Versace  writes:

> The test generates a mipmap by "copying" a client-array of pixels to
> miplevel 0 with glDrawPixels, then calling glGenerateMipmap. The test
> expects the rendered level 0 to match the client-array source. Dithering
> during glDrawPixels inteferes with that expectation.
>
> Fixes on Intel Skylake:
>   spec.ext_framebuffer_object.getteximage-formats init-by-clear-and-render
>   spec.ext_framebuffer_object.getteximage-formats init-by-render
>
> Regressed by Mesa:
>   commit c4b87f129eb036c9615df3adcc1cebd9df10fc84
>   Author: Chad Versace 
>   Subject: meta: Disable dithering during glGenerateMipmap
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99266
> Cc: Mark Janes 
> Cc: Kenneth Graunke 
> ---
>  tests/texturing/getteximage-formats.c | 25 +
>  1 file changed, 25 insertions(+)
>
> diff --git a/tests/texturing/getteximage-formats.c 
> b/tests/texturing/getteximage-formats.c
> index 6f4fb029e..02d78d441 100644
> --- a/tests/texturing/getteximage-formats.c
> +++ b/tests/texturing/getteximage-formats.c
> @@ -110,8 +110,33 @@ make_texture_image(GLenum intFormat, GLubyte 
> upperRightTexel[4])
>  
>   glViewport(0, 0, TEX_SIZE, TEX_SIZE);
>  
> + /* Disable dithering during glDrawPixels to prevent
> +  * disagreement with glGenerateMipmap.  The spec requires
> +  * glDrawPixels to respect the dither state, but the spec
> +  * strongly implies that glGenerateMipmap should not dither.
> +  *
> +  * On dithering and glDrawPixels, see the OpenGL 4.5
> +  * Compatibility Specification, Section 18.1 Drawing Pixels,
> +  * p620:
> +  *
> +  *   Once pixels are transferred, DrawPixels performs final
> +  *   conversion on pixel values [...] which are processed in
> +  *   the same fashion as fragments generated by rasterization
> +  *   (see chapters 15 and 16).
> +  *
> +  * On dithering and glGenerateMipmap, see the Mesa commit
> +  * message in:
> +  *
> +  *   commit c4b87f129eb036c9615df3adcc1cebd9df10fc84
> +  *   Author: Chad Versace 
> +  *   Date:   Thu Dec 29 13:05:27 2016 -0800
> +  *   Subject: meta: Disable dithering during glGenerateMipmap
> +  */
> + glDisable(GL_DITHER);
>   glWindowPos2iARB(0, 0);
>   glDrawPixels(TEX_SIZE, TEX_SIZE, GL_RGBA, GL_UNSIGNED_BYTE, 
> tex);
> + glEnable(GL_DITHER);
> +
>   glGenerateMipmap(GL_TEXTURE_2D);
>  
>   glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, piglit_winsys_fbo);
> -- 
> 2.11.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] deqp: Search stdout and stderr for X connection failure

2017-01-06 Thread Mark Janes
Recent versions of the GL CTS report X connection errors on standard
out.  Support all variants by checking both stdout and stderr for the
error string.
---
 framework/test/deqp.py | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/framework/test/deqp.py b/framework/test/deqp.py
index 5b53efd..871ce25 100644
--- a/framework/test/deqp.py
+++ b/framework/test/deqp.py
@@ -220,7 +220,9 @@ class DEQPBaseTest(Test):
 """Rerun the command if X11 connection failure happens."""
 for _ in range(5):
 super(DEQPBaseTest, self)._run_command(*args, **kwargs)
-if "FATAL ERROR: Failed to open display" not in self.result.err:
-return
+x_err_msg = "FATAL ERROR: Failed to open display"
+if x_err_msg in self.result.err or x_err_msg in self.result.out:
+continue
+return
 
 raise TestRunError('Failed to connect to X server 5 times', 'fail')
-- 
2.10.2

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


Re: [Piglit] [PATCH] arb_shader_image_load_store/shader-mem-barrier: Add explicit memoryBarrier to volatile subtest.

2016-12-30 Thread Mark Janes
Tested-by: Mark Janes 

Francisco Jerez  writes:

> This makes sure that reads and writes of the volatile-qualified images
> occur in the expected order.  It's unclear whether the GLSL volatile
> qualifier was intended to have the required memory ordering
> implications as it does in C.  I've bugged Khronos about it [1], but
> because I haven't seen any update on the bug report for a while, this
> changes the test temporarily to behave as expected regardless of what
> the resolution is.  This avoids some annoying intermittent failures on
> the i965 driver which doesn't provide any special memory ordering
> guarantees for volatile-qualified memory.
>
> [1] https://cvs.khronos.org/bugzilla/show_bug.cgi?id=15875
> ---
>  tests/spec/arb_shader_image_load_store/shader-mem-barrier.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/spec/arb_shader_image_load_store/shader-mem-barrier.c 
> b/tests/spec/arb_shader_image_load_store/shader-mem-barrier.c
> index ed08640..0c83ab7 100644
> --- a/tests/spec/arb_shader_image_load_store/shader-mem-barrier.c
> +++ b/tests/spec/arb_shader_image_load_store/shader-mem-barrier.c
> @@ -31,8 +31,8 @@
>   * locations until an inconsistency is observed or the test runs to
>   * completion.
>   *
> - * The test is repeated for the "volatile" qualifier with no barriers,
> - * for all execution stages and for different relative arrangements of
> + * The test is repeated for the "volatile" qualifier, for all
> + * execution stages and for different relative arrangements of
>   * producer and monitor threads to account for implementations with
>   * varying levels of parallelism and with caches of different sizes.
>   *
> @@ -84,7 +84,7 @@ struct image_test_info {
>  const struct image_test_info image_tests[] = {
>  { "control", "", "", true },
>  { "'coherent' qualifier", "coherent", "memoryBarrier()", false },
> -{ "'volatile' qualifier", "volatile", "", false },
> +{ "'volatile' qualifier", "volatile", "memoryBarrier()", false },
>  { 0 }
>  };
>  
> -- 
> 2.10.2
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] fbo-maxsize: Try to avoid allocating miplevels.

2016-12-10 Thread Mark Janes
This patch allows the test to complete on a Braswell system with 1.5GB
memory.  Without it, the OOM killer is dispatched.

Tested-by: Mark Janes 

Kenneth Graunke  writes:

> A 16384x16384 RGBA buffer takes 1GB of RAM.  If the driver allocates
> mipmap levels, that increases our storage requirements to 1.5GB.  The
> test doesn't use mipmapping, so this seems like a waste.
>
> Disabling mipmap filtering before allocating the texture provides a hint
> to the driver, suggesting that it should only allocate space for the
> base level.  Most Mesa drivers (Gallium and i965) follow this practice.
>
> Signed-off-by: Kenneth Graunke 
> ---
>  tests/fbo/fbo-maxsize.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/tests/fbo/fbo-maxsize.c b/tests/fbo/fbo-maxsize.c
> index 64dcd99..cec6716 100644
> --- a/tests/fbo/fbo-maxsize.c
> +++ b/tests/fbo/fbo-maxsize.c
> @@ -107,6 +107,11 @@ static int create_fbo(void)
>   glGenTextures(1, &tex);
>   glBindTexture(GL_TEXTURE_2D, tex);
>  
> + /* Turn off mipmap filtering as a hint to the driver that we don't
> +  * need multiple mipmap levels (as those can increase the memory
> +  * requirements by 50%, and we don't need them in this test.
> +  */
> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>   glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, maxsize, maxsize,
>0, GL_RGBA, GL_UNSIGNED_BYTE, NULL);
>  
> -- 
> 2.10.2
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] util: link with WAYLAND_LIBRARIES when building for Wayland

2016-11-30 Thread Mark Janes
Tested-by: Mark Janes 

Tapani Pälli  writes:

> Signed-off-by: Tapani Pälli 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98769
> ---
>  tests/util/CMakeLists.txt | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/tests/util/CMakeLists.txt b/tests/util/CMakeLists.txt
> index f197d84..a9fc339 100644
> --- a/tests/util/CMakeLists.txt
> +++ b/tests/util/CMakeLists.txt
> @@ -153,6 +153,11 @@ if(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
>   if(PIGLIT_HAS_X11)
>   list(APPEND UTIL_GL_LIBS ${X11_X11_LIB})
>   endif()
> +
> +if(PIGLIT_HAS_WAYLAND)
> + list(APPEND UTIL_GL_LIBS ${WAYLAND_LIBRARIES})
> +endif()
> +
>  endif(${CMAKE_SYSTEM_NAME} MATCHES "Linux")
>  
>  piglit_include_target_api()
> -- 
> 2.7.4
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH v2 0/2] eventloop for Wayland platform

2016-11-18 Thread Mark Janes
This series breaks my compilation:

gcc -m64  -Wall -std=gnu99 -Werror=vla -Werror=pointer-arith 
-Werror=variadic-macros -g   
target_api/gl/tests/bugs/CMakeFiles/point-sprite.dir/point-sprite.c.o  -o 
bin/point-sprite  -rdynamic lib/libpiglitutil_gl.so.0 -lGL 
lib/libpiglitutil.so.0 -lrt -ldl -lxkbcommon -lpng -lz -lm -lEGL 
-L/tmp/build_root/m64/lib -lgbm -L/tmp/build_root/m64/lib/x86_64-linux-gnu 
-lwaffle-1 -ldrm -lxcb -lxcb-dri2 -ldrm_intel -ldrm -lxcb -lxcb-dri2 
-ldrm_intel -lX11 -lGL 
-Wl,-rpath,/home/majanes/src/jenkins/repos/piglit/build_m64/lib: && :

lib/libpiglitutil_gl.so.0: undefined reference to `wl_keyboard_interface'
lib/libpiglitutil_gl.so.0: undefined reference to `wl_display_dispatch'

adding -lwayland-client gets the link working.

-Mark

Tapani Pälli  writes:

> Round 2 *gonggg*
>
> Here's another try to implement eventloop for Wayland. I've done fixes
> based on Pekka's review. I did not attempt to support multiseat or multiple
> keyboards, such configurations are considered exotic for running Piglit and
> support can be added later. Main thing is to try to be consistent with 
> existing X11 backend.
>
> Thanks;
>
> Tapani Pälli (2):
>   cmake: require libxkbcommon when building with Wayland
>   util: implement eventloop for wayland platform
>
>  CMakeLists.txt |   7 +
>  tests/util/CMakeLists.txt  |   5 +
>  .../util/piglit-framework-gl/piglit_wl_framework.c | 253 
> +++--
>  3 files changed, 249 insertions(+), 16 deletions(-)
>
> -- 
> 2.7.4
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] framework/profile: don't deepcopy when copying profiles

2016-11-15 Thread Mark Janes
Tested-by: Mark Janes 
Reviewed-by: Mark Janes 

Dylan Baker  writes:

> This reduces memory consumption by a lot (about 60% on my system), which
> fixes some tests that fail sporadically on low memory systems.
>
> cc: Mark Janes 
> Signed-off-by: Dylan Baker 
> ---
>  framework/profile.py | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/framework/profile.py b/framework/profile.py
> index 23abc6d..56f017b 100644
> --- a/framework/profile.py
> +++ b/framework/profile.py
> @@ -297,12 +297,11 @@ class TestProfile(object):
>  """Create a copy of the TestProfile.
>  
>  This method creates a copy with references to the original instance
> -(using copy.copy), except for the test_list attribute, which is 
> copied
> -using copy.deepcopy. This allows profiles to be "subclassed" by other
> -profiles, without modifying the original.
> +using copy.copy.deepcopy. This allows profiles to be "subclassed" by
> +other profiles, without modifying the original.
>  """
>  new = copy.copy(self)
> -new.test_list = copy.deepcopy(self.test_list)
> +new.test_list = copy.copy(self.test_list)
>  new.forced_test_list = copy.copy(self.forced_test_list)
>  new.filters = copy.copy(self.filters)
>  return new
> -- 
> 2.10.2
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH v2] framework/backends/json: support non-piglit junit files

2016-10-26 Thread Mark Janes
The junit loader is unnecessarily strict with the input that it
accepts.  It expects input generated by piglit, but can be made to
handle junit from other test suites like crucible.

v2: Remove unnecessary initializers
Conditionally parse command for piglit tests
---
 framework/backends/junit.py | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/framework/backends/junit.py b/framework/backends/junit.py
index 4c9b7af..4032db9 100644
--- a/framework/backends/junit.py
+++ b/framework/backends/junit.py
@@ -361,14 +361,19 @@ def _load(results_file):
 else:
 run_result.name = 'junit result'
 
-tree = 
etree.parse(results_file).getroot().find('.//testsuite[@name="piglit"]')
+tree = etree.parse(results_file).getroot().find('.//testsuite')
 for test in tree.iterfind('testcase'):
 result = results.TestResult()
 # Take the class name minus the 'piglit.' element, replace junit's '.'
 # separator with piglit's separator, and join the group and test names
-name = test.attrib['classname'].split('.', 1)[1]
+name = test.attrib['name']
+if 'classname' in test.attrib:
+name = grouptools.join(test.attrib['classname'], name)
 name = name.replace('.', grouptools.SEPARATOR)
-name = grouptools.join(name, test.attrib['name'])
+is_piglit = False
+if name.startswith("piglit"):
+is_piglit = True
+name = name.split(grouptools.SEPARATOR, 1)[1]
 
 # Remove the trailing _ if they were added (such as to api and search)
 if name.endswith('_'):
@@ -378,14 +383,23 @@ def _load(results_file):
 
 # 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
+result.time = results.TimeAttribute()
+if 'time' in test.attrib:
+result.time = results.TimeAttribute(end=float(test.attrib['time']))
+syserr = test.find('system-err')
+if syserr is not None:
+result.err = syserr.text
 
 # The command is prepended to system-out, so we need to separate those
 # into two separate elements
-out = test.find('system-out').text.split('\n')
-result.command = out[0]
-result.out = '\n'.join(out[1:])
+out_tag = test.find('system-out')
+if out_tag is not None:
+if is_piglit:
+out = out_tag.text.split('\n')
+result.command = out[0]
+result.out = '\n'.join(out[1:])
+else:
+result.out = out_tag.text
 
 # Try to get the values in stderr for time and pid
 for line in result.err.split('\n'):
-- 
2.9.3

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


Re: [Piglit] [PATCH] framework/backends/json: support non-piglit junit files

2016-10-26 Thread Mark Janes
Dylan Baker  writes:

> Quoting Mark Janes (2016-10-26 16:30:12)
>> The junit loader is unnecessarily strict with the input that it
>> accepts.  It expects input generated by piglit, but can be made to
>> handle junit from other test suites like crucible.
>> ---
>>  framework/backends/junit.py | 28 
>>  1 file changed, 20 insertions(+), 8 deletions(-)
>> 
>> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
>> index 4c9b7af..ab14074 100644
>> --- a/framework/backends/junit.py
>> +++ b/framework/backends/junit.py
>> @@ -361,14 +361,17 @@ def _load(results_file):
>>  else:
>>  run_result.name = 'junit result'
>>  
>> -tree = 
>> etree.parse(results_file).getroot().find('.//testsuite[@name="piglit"]')
>> +tree = etree.parse(results_file).getroot().find('.//testsuite')
>>  for test in tree.iterfind('testcase'):
>>  result = results.TestResult()
>>  # Take the class name minus the 'piglit.' element, replace junit's 
>> '.'
>>  # separator with piglit's separator, and join the group and test 
>> names
>> -name = test.attrib['classname'].split('.', 1)[1]
>> +name = test.attrib['name']
>> +if 'classname' in test.attrib:
>> +name = grouptools.join(test.attrib['classname'], name)
>>  name = name.replace('.', grouptools.SEPARATOR)
>> -name = grouptools.join(name, test.attrib['name'])
>> +if name.startswith("piglit"):
>> +name = name.split(grouptools.SEPARATOR, 1)[1]
>
> I assumed that grouptools.split could do this, but apparently not.
>
>>  
>>  # Remove the trailing _ if they were added (such as to api and 
>> search)
>>  if name.endswith('_'):
>> @@ -378,14 +381,23 @@ def _load(results_file):
>>  
>>  # 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
>> +result.time = results.TimeAttribute()
>> +if 'time' in test.attrib:
>> +result.time = 
>> results.TimeAttribute(end=float(test.attrib['time']))
>> +result.err = ""
>
> result.err is initialized to "", so no need to set it.
>
>> +syserr = test.find('system-err')
>> +if syserr is not None:
>> +result.err = syserr.text
> 
>>  
>>  # The command is prepended to system-out, so we need to separate 
>> those
>>  # into two separate elements
>> -out = test.find('system-out').text.split('\n')
>> -result.command = out[0]
>> -result.out = '\n'.join(out[1:])
>> +out_tag = test.find('system-out')
>
>> +result.out = ""
>> +result.command = ""
>
> These are initialized to "" as well.
>
>> +if out_tag is not None:
>> +out = out_tag.text.split('\n')
>> +result.command = out[0]
>
> This isn't right, if the suite generates stdout it is assumed that the command
> will be the first line, but that isn't true unless piglit generates it. We
> probably need to do like we do we do with stderr and have a "command: ..." and
> search for that.

You are right.  We could use the earlier check for piglit as the first
component in the name to conditionally take the first line as the
command.  Is that acceptable?

Other suites will have an empty command in the field, which is still
usable.

>> +result.out = '\n'.join(out[1:])
>>  
>>  # Try to get the values in stderr for time and pid
>>  for line in result.err.split('\n'):
>> -- 
>> 2.9.3
>> 
>
> Dylan
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] framework/backends/json: support non-piglit junit files

2016-10-26 Thread Mark Janes
The junit loader is unnecessarily strict with the input that it
accepts.  It expects input generated by piglit, but can be made to
handle junit from other test suites like crucible.
---
 framework/backends/junit.py | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/framework/backends/junit.py b/framework/backends/junit.py
index 4c9b7af..ab14074 100644
--- a/framework/backends/junit.py
+++ b/framework/backends/junit.py
@@ -361,14 +361,17 @@ def _load(results_file):
 else:
 run_result.name = 'junit result'
 
-tree = 
etree.parse(results_file).getroot().find('.//testsuite[@name="piglit"]')
+tree = etree.parse(results_file).getroot().find('.//testsuite')
 for test in tree.iterfind('testcase'):
 result = results.TestResult()
 # Take the class name minus the 'piglit.' element, replace junit's '.'
 # separator with piglit's separator, and join the group and test names
-name = test.attrib['classname'].split('.', 1)[1]
+name = test.attrib['name']
+if 'classname' in test.attrib:
+name = grouptools.join(test.attrib['classname'], name)
 name = name.replace('.', grouptools.SEPARATOR)
-name = grouptools.join(name, test.attrib['name'])
+if name.startswith("piglit"):
+name = name.split(grouptools.SEPARATOR, 1)[1]
 
 # Remove the trailing _ if they were added (such as to api and search)
 if name.endswith('_'):
@@ -378,14 +381,23 @@ def _load(results_file):
 
 # 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
+result.time = results.TimeAttribute()
+if 'time' in test.attrib:
+result.time = results.TimeAttribute(end=float(test.attrib['time']))
+result.err = ""
+syserr = test.find('system-err')
+if syserr is not None:
+result.err = syserr.text
 
 # The command is prepended to system-out, so we need to separate those
 # into two separate elements
-out = test.find('system-out').text.split('\n')
-result.command = out[0]
-result.out = '\n'.join(out[1:])
+out_tag = test.find('system-out')
+result.out = ""
+result.command = ""
+if out_tag is not None:
+out = out_tag.text.split('\n')
+result.command = out[0]
+result.out = '\n'.join(out[1:])
 
 # Try to get the values in stderr for time and pid
 for line in result.err.split('\n'):
-- 
2.9.3

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


Re: [Piglit] [PATCH 0/3] Fix summary breakage

2016-10-25 Thread Mark Janes
Dylan Baker  writes:

> This little series fixes three bugs in the summary code:
>
> The first one fixes the argument help message being returned when an unrelated
> exception is raised.
>
> The second fixes summarizing JSON results, which were broken due to incorrect
> deserialization.
>
> The third fixes summarizing JUnit results, which were broken due to incorrect
> deserialization, this is a separate bug from the one that affected the JSON
> loader.
>
> This is available at my github:
> https://github.com/dcbaker/piglit wip/fix-summary
>
> Mark and Michel,
> I'd appreciate if you could test this and make sure it fixes your respective
> problems.

Tested-by: Mark Janes 

> Dylan Baker (3):
>   piglit: Only catch parsed.func AttributeError
>   framework: Properly load JSON into internal representation
>   framework: fix loading JUnit results
>
>  framework/backends/junit.py|  6 ++-
>  framework/programs/run.py  |  2 +-
>  framework/results.py   | 23 +++--
>  piglit |  4 +-
>  unittests/framework/backends/shared.py |  4 ++
>  unittests/framework/backends/test_json.py  |  3 +-
>  unittests/framework/backends/test_junit.py |  6 +--
>  unittests/framework/test_results.py| 77 
> ++
>  8 files changed, 61 insertions(+), 64 deletions(-)
>
> -- 
> 2.10.1
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] Newbie question: command to run tests that passed before?

2016-09-07 Thread Mark Janes
Dan Kegel  writes:

> Hrm.  A solution that involved Jenkins would be way overkill.
> I'm just looking for the little core idiom for running just the
> test of tests that passed in the, um, past.

Even if you don't want to set up a CI with jenkins, you can refer to the
automation that I've written at https://github.com/janesma/mesa_jenkins

Our approach is to list all failing/crashing tests in a config file, and
pass that to piglit.  Piglit converts "known failing" tests to a "skip"
status.  The only tests we can't run are the ones that have flaky
results or cause gpu hang.

For example:
https://github.com/janesma/mesa_jenkins/blob/master/piglit-test/bdw.conf

> Maybe that's not the way people do it?  Does everyone just compare
> logs against old logs?
>
> Thanks, and say hi to Ethyl for me.
> - Dan
>
> On Wed, Sep 7, 2016 at 4:08 PM, Ilia Mirkin  wrote:
>> [+janesma]
>>
>> Mark has set up a CI system at Intel, and I'm fairly sure did a
>> writeup about it and shared code, but I can't for the life of me
>> remember the details. Maybe a few Guinesses ago, but definitely not
>> now... hopefully he can share. [It involves Jenkins.]
>>
>>   -ilia
>>
>> On Wed, Sep 7, 2016 at 6:59 PM, Dan Kegel  wrote:
>>> Hi!
>>> I'd like to use piglit as a regression test for my opengl test rigs.
>>> My plan is to run piglit a few times on each OS/card combo I need to 
>>> support,
>>> get a list of tests that reliably pass on each platform, and then
>>> forevermore run just those tests.
>>>
>>> I know about the -t and -x options, and the --test-list option, and
>>> about writing test profiles,
>>> but am still not sure what the best way to generate a list of passing
>>> tests to pass to e.g. --test-list.
>>> Test names are a little hard for this newbie to identify in the output
>>> of piglit summary console.
>>>
>>> After looking at the source a bit, I tried generating a list of passed
>>> tests like this:
>>> ./piglit summary console results/quick | grep ': pass$' | sed
>>> 's/:.*//' | tr / @ > tests.list
>>> but that includes output
>>> spec@!opengl 1.1@max-texture-size-level
>>> spec@!opengl 1.1@max-texture-size@GL_PROXY_TEXTURE_1D-GL_RGBA16
>>>
>>> which leads to errors from --test-list tests.list like
>>> Fatal Error: Cannot reorder test: "spec@!opengl
>>> 1.1@max-texture-size@GL_PROXY_TEXTURE_1D-GL_RGBA16", it is not in the
>>> profile.
>>>
>>> Suggestions?
>>>
>>> p.s. Mesa Haswell is behaving very well on stock ubuntu 16.04, only 1%
>>> of quick tests fail, and there are no hangs or kernel complaints.
>>> ___
>>> Piglit mailing list
>>> Piglit@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH v3 0/4] Add support for subtests in JUnit as nested testsuite elements

2016-08-31 Thread Mark Janes
Before --junit-subtests is made the default, I need to be able to use
subtest names in:

 * --include-tests and --exclude-tests filters
 * [expected-failures] and [expected-crashes] sections of config files.

For filtering subtests, it is acceptable for include/exclude to apply to
the parent test.  I don't need to run a single subtest.

Series is

Reviewed-by: Mark Janes 
Tested-by: Mark Janes 

Dylan Baker  writes:

> This series refactors the JUnit backend so that the _write attribute is
> actually a callable class, instead of a method. This gives greater
> flexibility to handle subtests in the JUnit, or continue ignoring them.
>
> The reason to ignore them is that some xUnit implementations don't
> handle them properly (the JUnit plugin for Jenkins does not, for
> example, while the xUnit plugin does). Since piglit has a more than one
> consumer of the JUnit backend, it makes sense to be able to select the
> new backend or not.
>
> To enable this features add the --junit-subtests switch to the command
> line (obviously when using the junit backend), and watch magic happen.
> Each test with subtests will be recorded as a testsuite element, and the
> stderr and stdout will be attached to the testsuite rather than the
> testcase. However, when using the xUnit plugin for Jenkins it is
> rendered correctly.
>
> Changes in v2:
>  - Add error message explaining what happened if result != pass.
>  - Merge two patches together to maintain behavior for bisecting.
>  - Add skipped tag to tests without subtests (when using junit-subtests).
>  - Add classname to subtests, like is done for full tests.
>  - Add additional unittests.
>  - Call junit_escape on the testname as well as the classname.
>  - Reword the help message for the junit-subtests switch.
>  - Add a new patch which fixes a bug masking status changes if the test 
> becomes
>skip.
>
> Changes in v3:
>  - Fix classname in subtests to include the test name.
>  - Fix spelling errors in commit messages.
>
> Dylan Baker (4):
>   framework/backends/junit.py: Split _write into a separate class.
>   framework/backends/junit.py: Add a writer class that handles subtests
>   framework: add command line switch to enable junit subtests
>   framework/backends/junit: Don't let skip hide status changes
>
>  framework/backends/junit.py| 352 ++
>  framework/programs/run.py  |   8 +-
>  unittests/framework/backends/test_junit.py | 192 +++-
>  3 files changed, 414 insertions(+), 138 deletions(-)
>
> -- 
> git-series 0.8.7
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] Tests: Add integration for dEQP-EGL tests

2016-08-09 Thread Mark Janes
Some minor nits on the comments below.

Reviewed-by: Mark Janes 
Tested-by: Mark Janes 

Dylan Baker  writes:

> This adds the necessary bits to make dEQP-EGL work.
>
> Signed-off-by: Dylan Baker 
> CC: Mark Janes 
> ---
>  piglit.conf.example | 10 ++
>  tests/deqp_egl.py   | 54 
> +
>  2 files changed, 64 insertions(+)
>  create mode 100644 tests/deqp_egl.py
>
> diff --git a/piglit.conf.example b/piglit.conf.example
> index b555129..0a58d98 100644
> --- a/piglit.conf.example
> +++ b/piglit.conf.example
> @@ -37,6 +37,16 @@ testB
>  ; Options that affect all deqp based suites
>  ;extra_args=--deqp-visibility=hidden
>  
> +[deqp-egl]
> +; Path to the deqp-gles2 executable

wrong name:  ^^

> +; Can be overwritten by PIGLIT_DEQP_EGL_BIN environment variable
> +;bin=/home/knuth/deqp/modules/egl/deqp-egl
> +
> +; Space-separated list of extra command line arguments for deqp-egl. The
> +; option is not required. The environment variable PIGLIT_DEQP_EGL_EXTRA_ARGS
> +; overrides the value set here.
> +;extra_args=--deqp-visibility hidden
> +
>  [deqp-gles2]
>  ; Path to the deqp-gles2 executable
>  ; Can be overwritten by PIGLIT_DEQP_GLES2_BIN environment variable
> diff --git a/tests/deqp_egl.py b/tests/deqp_egl.py
> new file mode 100644
> index 000..27681d3
> --- /dev/null
> +++ b/tests/deqp_egl.py
> @@ -0,0 +1,54 @@
> +# Copyright 2015-2016 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 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.
> +
> +"""Piglit integrations for dEQP EGL tests."""
> +
> +from __future__ import (
> +absolute_import, division, print_function, unicode_literals
> +)
> +
> +from framework.test import deqp
> +
> +__all__ = ['profile']
> +
> +# Path to the deqp-gles2 executable.

wrong name:  ^^

> +_EGL_BIN = deqp.get_option('PIGLIT_DEQP_EGL_BIN',
> +   ('deqp-egl', 'bin'),
> +   required=True)
> +
> +_EXTRA_ARGS = deqp.get_option('PIGLIT_DEQP_EGL_EXTRA_ARGS',
> +  ('deqp-egl', 'extra_args'),
> +  default='').split()
> +
> +
> +class DEQPEGLTest(deqp.DEQPBaseTest):
> +deqp_bin = _EGL_BIN
> +
> +@property
> +def extra_args(self):
> +return super(DEQPEGLTest, self).extra_args + \
> +[x for x in _EXTRA_ARGS if not x.startswith('--deqp-case')]
> +
> +
> +profile = deqp.make_profile(  # pylint: disable=invalid-name
> +deqp.iter_deqp_test_cases(
> +deqp.gen_caselist_txt(_EGL_BIN, 'dEQP-EGL-cases.txt',
> +  _EXTRA_ARGS)),
> +DEQPEGLTest)
> -- 
> 2.9.2
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 2/2] shader_runner: Fix get_ints on 32-bit systems.

2016-06-06 Thread Mark Janes
Kenneth Graunke  writes:

> The new ARB_vertex_attrib_64bit tests specify integer uniform values
> as hex, such as 0xc21620c5.  As an integer value, this is beyond LONG_MAX
> on 32-bit systems.  The intent is to parse it as an unsigned hex value and
> bitcast it.
>
> However, we still need to handle parsing values with negative signs.
>
> Using strtoll and truncating works.
>
> Signed-off-by: Kenneth Graunke 
> ---
>  tests/shaders/shader_runner.c | 2 +-
>  tests/util/piglit-vbo.cpp | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index 94c7826..56fd97c 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -1398,7 +1398,7 @@ get_ints(const char *line, int *ints, unsigned count)
>   unsigned i;
>  
>   for (i = 0; i < count; i++)
> - ints[i] = strtol(line, (char **) &line, 0);
> + ints[i] = strtoll(line, (char **) &line, 0);
>  }
>  
>  
> diff --git a/tests/util/piglit-vbo.cpp b/tests/util/piglit-vbo.cpp
> index fd7e72a..50e6731 100644
> --- a/tests/util/piglit-vbo.cpp
> +++ b/tests/util/piglit-vbo.cpp
> @@ -387,8 +387,8 @@ vertex_attrib_description::parse_datum(const char **text, 
> void *data) const
>   break;
>   }
>   case GL_INT: {
> - long value = (long) strtoll(*text, &endptr, 0);
> - if (errno == ERANGE) {
> + long long value = strtoll(*text, &endptr, 0);
> + if (errno == ERANGE || (unsigned long long) value > 
> 0xull) {
^^^^^
with this check removed, the series corrects all 32 bit failures
introduced by b7eb469, and is

Tested-by: Mark Janes 

>   printf("Could not parse as signed integer\n");
>   return false;
>   }
> -- 
> 2.8.3
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] tests: Add integration with Khronos CTS OpenGL runner

2016-06-02 Thread Mark Janes
GS)),
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL30-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>> +deqp.iter_deqp_test_cases(
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL31-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>> +deqp.iter_deqp_test_cases(
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL32-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>> +deqp.iter_deqp_test_cases(
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL33-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>> +deqp.iter_deqp_test_cases(
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL40-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>> +deqp.iter_deqp_test_cases(
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL41-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>> +deqp.iter_deqp_test_cases(
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL42-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>>  deqp.iter_deqp_test_cases(
>> -deqp.gen_caselist_txt(_CTS_BIN, 'ES3-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL43-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>>  deqp.iter_deqp_test_cases(
>> -deqp.gen_caselist_txt(_CTS_BIN, 'ES31-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL44-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>>  deqp.iter_deqp_test_cases(
>> -    deqp.gen_caselist_txt(_CTS_BIN, 'ESEXT-CTS-cases.txt',
>> -  _EXTRA_ARGS)),
>> +deqp.gen_caselist_txt(_CTS_BIN, 'GL45-CTS-cases.txt', 
>> _EXTRA_ARGS)),
>>  ),
>>  DEQPCTSTest)
>> diff --git a/tests/cts.py b/tests/cts_gles31.py
>> similarity index 97%
>> rename from tests/cts.py
>> rename to tests/cts_gles31.py
>> index 0e64e1b..ad28e58 100644
>> --- a/tests/cts.py
>> +++ b/tests/cts_gles31.py
>> @@ -25,7 +25,7 @@ desiring to run only a subset of them should consider 
>> using the -t or -x
>>  options to include or exclude tests.
>>  
>>  For example:
>> -./piglit run cts -c foo -t ES3- would run only ES3 tests (note the dash to
>> +./piglit run cts_gles31 -c foo -t ES3- would run only ES3 tests (note the 
>> dash to
>>  exclude ES31 tests)
>>  
>>  This integration requires some configuration in piglit.conf, or the use of
>> -- 
>> 2.5.5
>> 
>> ___
>> Piglit mailing list
>> Piglit@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/piglit
>
> Other than the one nit this looks good. Please be sure to let Mark Janes
> (cc'd) or myself know when you push this, as we'll need to update our CI
> since this will change some profile names.

Whoops!  This was pushed on May 23, at which point our CI stopped
running the CTS.  And now we have a handful of regressions to bisect.

> Reviewed-by: Dylan Baker 
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] Nearly finished: shader_runner running THOUSANDS of tests per process

2016-05-27 Thread Mark Janes
Marek Olšák  writes:

> On Fri, May 27, 2016 at 3:18 AM, Mark Janes  wrote:
>> Marek Olšák  writes:
>>
>>> On Mon, Apr 18, 2016 at 6:45 PM, Dylan Baker  
>>> wrote:
>>>> Quoting Marek Olšák (2016-04-16 15:16:34)
>>>>> Hi,
>>>>>
>>>>> This makes shader_runner very fast. The expected result is 40%
>>>>> decrease in quick.py running time, or a 12x faster piglit run if you
>>>>> run shader tests alone.
>>>>>
>>>>> Branch:
>>>>> https://cgit.freedesktop.org/~mareko/piglit/log/?h=shader-runner
>>>>>
>>>>> Changes:
>>>>>
>>>>> 1) Any number of test files can be specified as command-line
>>>>> parameters. Those command lines can be insanely long.
>>>>>
>>>>> 2) shader_runner can re-create the window & GL context if test
>>>>> requirements demand different settings when going from one test to
>>>>> another.
>>>>>
>>>>> 3) all.py generates one shader_runner instance per group of tests
>>>>> (usually one or two directories - tests and generated_tests).
>>>>> Individual tests are reported as subtests.
>>>>>
>>>>> The shader_runner part is done. The python part needs more work.
>>>>>
>>>>>
>>>>> What's missing:
>>>>>
>>>>> Handling of crashes. If shader_runner crashes:
>>>>> - The crash is not shown in piglit results (other tests with subtests
>>>>> already have the same behavior)
>>>>> - The remaining tests will not be run.
>>>>>
>>>>> The ShaderTest python class has the list of all files and should be
>>>>> able to catch a crash, check how many test results have been written,
>>>>> and restart shader_runner with the remaining tests.
>>>>>
>>>>> shader_runner prints TEST %i: and then the subtest result. %i is the
>>>>> i-th file in the list. Python can parse that and re-run shader_runner
>>>>> with the first %i tests removed. (0..%i-1 -> parse subtest results; %i
>>>>> -> crash; %i+1.. -> run again)
>>>>>
>>>>> I'm by no means a python expert, so here's an alternative solution (for 
>>>>> me):
>>>>> - Catch crash signals in shader_runner.
>>>>> - In the single handler, re-run shader_runner with the remaining tests.
>>>>>
>>>>> Opinions welcome,
>>
>> Per-test process isolation is a key feature of Piglit that the Intel CI
>> relies upon.  If non-crash errors bleed into separate tests, results
>> will be unusable.
>>
>> In fact, we wrap all other test suites in piglit primarily to provide
>> them with per-test process isolation.
>>
>> For limiting test run-time, we shard tests into groups and run them on
>> parallel systems.  Currently this is only supported by dEQP features,
>> but it can make test time arbitrarily low if you have adequate hardware.
>>
>> For test suites that don't support sharding, I think it would be useful
>> to generate suites from start/end times that can run the maximal set of
>> tests in the targeted duration.
>>
>> I would be worried by complex handling of crashes.  It would be
>> preferable if separate suites were available to run with/without shader
>> runner process isolation.
>>
>> Users desiring faster execution can spend the saved time figuring out
>> which test crashed.
>
> I would say that the majority of upstream users care more about piglit
> running time and less about process isolation.
>
> Process isolation can be an optional piglit flag.

WFM.

>>
>>>>> Marek
>>>>> ___
>>>>> Piglit mailing list
>>>>> Piglit@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/piglit
>>>>
>>>> Thanks for working on this Marek,
>>>>
>>>> This has been discussed here several times amongst the intel group, and
>>>> the recurring problem to solve is crashing. I don't have a strong
>>>> opinion on python vs catching a fail in the signal handler, except that
>>>> handling in the python might be more robust, but I'm not really familiar
>>>> with what a C signal handler can recover from, so it may not.
>>>
>>> I can catch signals like exceptions and report 'crash'. Then I can
>>> open a new process from the handler to run the remaining tests, wait
>>> and exit.
>>
>> Will an intermittent crash be run again until it passes?
>
> No.
>
> Marek
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] Nearly finished: shader_runner running THOUSANDS of tests per process

2016-05-26 Thread Mark Janes
Marek Olšák  writes:

> On Mon, Apr 18, 2016 at 6:45 PM, Dylan Baker  wrote:
>> Quoting Marek Olšák (2016-04-16 15:16:34)
>>> Hi,
>>>
>>> This makes shader_runner very fast. The expected result is 40%
>>> decrease in quick.py running time, or a 12x faster piglit run if you
>>> run shader tests alone.
>>>
>>> Branch:
>>> https://cgit.freedesktop.org/~mareko/piglit/log/?h=shader-runner
>>>
>>> Changes:
>>>
>>> 1) Any number of test files can be specified as command-line
>>> parameters. Those command lines can be insanely long.
>>>
>>> 2) shader_runner can re-create the window & GL context if test
>>> requirements demand different settings when going from one test to
>>> another.
>>>
>>> 3) all.py generates one shader_runner instance per group of tests
>>> (usually one or two directories - tests and generated_tests).
>>> Individual tests are reported as subtests.
>>>
>>> The shader_runner part is done. The python part needs more work.
>>>
>>>
>>> What's missing:
>>>
>>> Handling of crashes. If shader_runner crashes:
>>> - The crash is not shown in piglit results (other tests with subtests
>>> already have the same behavior)
>>> - The remaining tests will not be run.
>>>
>>> The ShaderTest python class has the list of all files and should be
>>> able to catch a crash, check how many test results have been written,
>>> and restart shader_runner with the remaining tests.
>>>
>>> shader_runner prints TEST %i: and then the subtest result. %i is the
>>> i-th file in the list. Python can parse that and re-run shader_runner
>>> with the first %i tests removed. (0..%i-1 -> parse subtest results; %i
>>> -> crash; %i+1.. -> run again)
>>>
>>> I'm by no means a python expert, so here's an alternative solution (for me):
>>> - Catch crash signals in shader_runner.
>>> - In the single handler, re-run shader_runner with the remaining tests.
>>>
>>> Opinions welcome,

Per-test process isolation is a key feature of Piglit that the Intel CI
relies upon.  If non-crash errors bleed into separate tests, results
will be unusable.

In fact, we wrap all other test suites in piglit primarily to provide
them with per-test process isolation.

For limiting test run-time, we shard tests into groups and run them on
parallel systems.  Currently this is only supported by dEQP features,
but it can make test time arbitrarily low if you have adequate hardware.

For test suites that don't support sharding, I think it would be useful
to generate suites from start/end times that can run the maximal set of
tests in the targeted duration.

I would be worried by complex handling of crashes.  It would be
preferable if separate suites were available to run with/without shader
runner process isolation.

Users desiring faster execution can spend the saved time figuring out
which test crashed.

>>> Marek
>>> ___
>>> Piglit mailing list
>>> Piglit@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/piglit
>>
>> Thanks for working on this Marek,
>>
>> This has been discussed here several times amongst the intel group, and
>> the recurring problem to solve is crashing. I don't have a strong
>> opinion on python vs catching a fail in the signal handler, except that
>> handling in the python might be more robust, but I'm not really familiar
>> with what a C signal handler can recover from, so it may not.
>
> I can catch signals like exceptions and report 'crash'. Then I can
> open a new process from the handler to run the remaining tests, wait
> and exit.

Will an intermittent crash be run again until it passes?

> The signal catching won't work on Windows.
>
> Also, there are piglit GL framework changes that have only been tested
> with Waffle and may break other backends.
>
>>
>> The one concern I have is using subtests. There are a couple of
>> limitations to them, first we'll loose all of the per test stdout/stderr
>> data, and that seems less than optimal. I wonder if it would be better
>> to have shader runner print some sort of scissor to stdout and stderr
>> when it starts a test and when it finishes one, and then report results
>> as normal without the subtest. That would maintain the output of each
>> test file, which seems like what we want, otherwise the output will be
>
> That can be done easily in C.
>
>> jumbled. The other problem with subtests is that the JUnit backend
>> doesn't have a way to represent subtests at the moment. That would be
>> problematic for both us and for VMWare.
>
> I can't help with anything related to python.
>
> The goal is to make piglit faster for general regression testing.
> Other use cases can be affected negatively, but the time savings are
> worth it.
>
>>
>> Looking at the last patch the python isn't all correct there, it will
>> run in some cases and fail in others, particularly it will do something
>> odd if fast skipping is enabled, but I'm not sure exactly what. I think
>> it's worth measuring and seeing if the fast skipping path is even an
>> o

Re: [Piglit] [PATCH] framework/backends/junit: Fix invalid JUnit output

2016-05-06 Thread Mark Janes
Reviewed-by: Mark Janes 

Dylan Baker  writes:

> In commit b0d05323e code was added to produce clearer error messages for
> tests who's status changed from crash to fail and vice versa. There is a
> not so subtle bug in that patch, it adds a "crash" element, but that
> element should be an "error" element.
>
> This patch fixes that bug.
>
> cc: Kenneth Graunke 
> cc: Mark Janes 
> Signed-off-by: Dylan Baker  ---
>  framework/backends/junit.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
> index 8263d98..4b3d87e 100644
> --- a/framework/backends/junit.py
> +++ b/framework/backends/junit.py
> @@ -168,9 +168,9 @@ class JUnitBackend(FileBackend):
>  elif expected_result == 'failure':
>  err.text += \
>  "\n\nERROR: Test should have been failure but was 
> crash"
> -res = etree.SubElement(element, 'crash',
> +res = etree.SubElement(element, 'error',
> message='expected failure, but 
> got '
> -   'crash')
> +   'error')
>  else:
>  res = etree.SubElement(element, 'error')
>  
> -- 
> 2.8.2
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 2/2] framework/glslparsertest: Don't add exclude extensions to the required list

2016-04-13 Thread Mark Janes
Reviewed-by: Mark Janes 

Dylan Baker  writes:

> glslparsertest has a syntax for the 'require_extensions' option that is
> "!", when this is specified it means "This extensions is
> not supported". There is a bug in the fast skip code in
> glsl_parser_test.py that causes it to add these extensions to the fast
> skip required extensions list, which is obviously wrong.
>
> This patch adds a test for this behavior as well as fixing the behavior
> to work correctly.
>
> cc: Mark Janes 
> Signed-off-by: Dylan Baker 
> ---
>
> Mark, I hate to have to ask you to review this, but it's masking
> failures in jenkins a few cases (there aren't very many tests that use
> this feature, but the generator I submitted recently does).
>
>  framework/test/glsl_parser_test.py  |  3 ++-
>  unittests/glsl_parser_test_tests.py | 16 
>  2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/framework/test/glsl_parser_test.py 
> b/framework/test/glsl_parser_test.py
> index ce043a7..dcce8f8 100644
> --- a/framework/test/glsl_parser_test.py
> +++ b/framework/test/glsl_parser_test.py
> @@ -126,7 +126,8 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
>  
>  req = config.get('require_extensions')
>  if req:
> -self.gl_required = set(req.split())
> +self.gl_required = set(
> +r for r in req.split() if not r.startswith('!'))
>  
>  # If GLES is requested, but piglit was not built with a gles version,
>  # then ARB_ES3_compatibility is required. Add it to
> diff --git a/unittests/glsl_parser_test_tests.py 
> b/unittests/glsl_parser_test_tests.py
> index c732bec..048c5a4 100644
> --- a/unittests/glsl_parser_test_tests.py
> +++ b/unittests/glsl_parser_test_tests.py
> @@ -456,6 +456,22 @@ def test_set_gl_required():
>  nt.eq_(test.gl_required, set(['GL_ARB_foobar', 'GL_EXT_foobar']))
>  
>  
> +def test_set_exclude_gl_required():
> +"""test.glsl_parser_test.GLSLParserTest: doesn't add excludes to 
> gl_required"""
> +rt = {'require_extensions': 'GL_ARB_foobar !GL_EXT_foobar'}
> +with mock.patch.object(glsl.GLSLParserTest, '_GLSLParserTest__parser',
> +   mock.Mock(return_value=rt)):
> +with mock.patch.object(glsl.GLSLParserTest,
> +   '_GLSLParserTest__get_command',
> +   return_value=['foo']):
> +with mock.patch('framework.test.glsl_parser_test.open',
> +mock.mock_open(), create=True):
> +with mock.patch('framework.test.glsl_parser_test.os.stat',
> +mock.mock_open()):
> +test = glsl.GLSLParserTest('foo')
> +nt.eq_(test.gl_required, set(['GL_ARB_foobar']))
> +
> +
>  @mock.patch('framework.test.glsl_parser_test._HAS_GL_BIN', False)
>  @nt.raises(TestIsSkip)
>  def test_binary_skip():
> -- 
> 2.8.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] deqp: Rerun if tests fail to connect to X

2016-03-28 Thread Mark Janes
Reviewed-by: Mark Janes 

Dylan Baker  writes:

> A pretty common error it seems is to get a failure to open display when
> using X11. This adds an extra method to the DEQPBaseTest class that
> reruns the test is it finds that error in the stderr. This affects only
> deqp based suite integration.
>
> This fixes a number of sporadically failing tests from our CI.
> ---
>  framework/test/deqp.py | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/framework/test/deqp.py b/framework/test/deqp.py
> index 80ce156..c75577a 100644
> --- a/framework/test/deqp.py
> +++ b/framework/test/deqp.py
> @@ -26,10 +26,11 @@ import os
>  import subprocess
>  
>  import six
> +from six.moves import range
>  
>  from framework import core, grouptools, exceptions
>  from framework.profile import TestProfile
> -from framework.test.base import Test, is_crash_returncode
> +from framework.test.base import Test, is_crash_returncode, TestRunError
>  
>  __all__ = [
>  'DEQPBaseTest',
> @@ -177,3 +178,12 @@ class DEQPBaseTest(Test):
>  # We failed to parse the test output. Fallback to 'fail'.
>  if self.result.result == 'notrun':
>  self.result.result = 'fail'
> +
> +def _run_command(self):
> +"""Rerun the command if X11 connection failure happens."""
> +for _ in range(5):
> +super(DEQPBaseTest, self)._run_command()
> +if "FATAL ERROR: Failed to open display" not in self.result.err:
> +return
> +
> +raise TestRunError('Failed to connect to X server 5 times', 'fail')
> -- 
> 2.7.4
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] framework: Add handler for failure and error when expecting the other

2016-03-19 Thread Mark Janes
I don't need to see a v2

Acked-by: Mark Janes 

Dylan Baker  writes:

> Quoting Mark Janes (2016-03-15 17:40:16)
>> Dylan Baker  writes:
>> 
>> > Right now the JUnit backend knows what to do with a failure when it
>> > expects one (or a crash), but not what to do when it expects a failure
>> > and gets a crash (or vice versa).
>> >
>> > This patch teaches it what to do in that case, mark the test as a
>> > failure and give a message explaining why.
>> >
>> > cc: mark.a.ja...@intel.com
>> > signed-off-by: Dylan Baker 
>> > ---
>> >  framework/backends/junit.py | 12 
>> >  1 file changed, 12 insertions(+)
>> >
>> > diff --git a/framework/backends/junit.py b/framework/backends/junit.py
>> > index f9eec66..761b201 100644
>> > --- a/framework/backends/junit.py
>> > +++ b/framework/backends/junit.py
>> > @@ -151,6 +151,12 @@ class JUnitBackend(FileBackend):
>> >  err.text += "\n\nWARN: passing test as an expected 
>> > failure"
>> >  res = etree.SubElement(element, 'skipped',
>> > message='expected failure')
>> > +elif expected_result == 'error':
>> > +err.text += \
>> > +"\n\nERROR: Test should have been crash but was 
>> > failure"
>> > +res = etree.SubElement(element, 'failure',
>> > +   message='expected crash, but 
>> > got '
>> > +   'failure')
>> >  else:
>> >  res = etree.SubElement(element, 'failure')
>> >  
>> > @@ -159,6 +165,12 @@ class JUnitBackend(FileBackend):
>> >  err.text += "\n\nWARN: passing test as an expected 
>> > crash"
>> >  res = etree.SubElement(element, 'skipped',
>> > message='expected crash')
>> > +elif expected_result == 'failure':
>> > +err.text += \
>> > +"\n\nERROR: Test should have been failure but was 
>> > crash"
>> > +res = etree.SubElement(element, 'failure',
>> I think you want to insert an 'error' tag here  ^^^
>> > +   message='expected failure, but 
>> > got '
>> > +   'crash')
>> >  else:
>> >  res = etree.SubElement(element, 'error')
>> >  
>> > -- 
>> > 2.7.3
>
> After discussing this offline, you are right.
>
> I've made the change locally, do you want to see a v2?
>
> Dylan
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] deqp-integration: Handle ResourceError.

2016-03-15 Thread Mark Janes
Mark Janes  writes:

> This patch doesn't fix the issue I see on HSW with Resource errors.

I take back this statement.  I was confused because the tests went from
fail to crash, but there was no indication of this discrepancy in the
output.

Thanks for your recent patch to make this more clear.

Reviewed-by: Mark Janes 
Tested-by: Mark Janes 

>
> Dylan Baker  writes:
>
>> This is a previously unseen error type generated on by the Vulkan CTS
>> (which is dEQP based).
>>
>> Signed-off-by: Dylan Baker 
>> ---
>>
>> Mark, I think this will fix the that jenkins bug we're seeing.
>>
>>  framework/test/deqp.py  | 15 +--
>>  unittests/deqp_tests.py | 50 
>> -
>>  2 files changed, 38 insertions(+), 27 deletions(-)
>>
>> diff --git a/framework/test/deqp.py b/framework/test/deqp.py
>> index 4fd51ec..80ce156 100644
>> --- a/framework/test/deqp.py
>> +++ b/framework/test/deqp.py
>> @@ -115,12 +115,15 @@ def iter_deqp_test_cases(case_file):
>>  
>>  @six.add_metaclass(abc.ABCMeta)
>>  class DEQPBaseTest(Test):
>> -__RESULT_MAP = {"Pass": "pass",
>> -"Fail": "fail",
>> -"QualityWarning": "warn",
>> -"InternalError": "fail",
>> -"Crash": "crash",
>> -"NotSupported": "skip"}
>> +__RESULT_MAP = {
>> +"Pass": "pass",
>> +"Fail": "fail",
>> +"QualityWarning": "warn",
>> +"InternalError": "fail",
>> +"Crash": "crash",
>> +"NotSupported": "skip",
>> +"ResourceError": "crash",
>> +}
>>  
>>  @abc.abstractproperty
>>  def deqp_bin(self):
>> diff --git a/unittests/deqp_tests.py b/unittests/deqp_tests.py
>> index 7143da2..7119fed 100644
>> --- a/unittests/deqp_tests.py
>> +++ b/unittests/deqp_tests.py
>> @@ -28,6 +28,7 @@ tests
>>  from __future__ import (
>>  absolute_import, division, print_function, unicode_literals
>>  )
>> +import textwrap
>>  
>>  try:
>>  from unittest import mock
>> @@ -191,34 +192,35 @@ class TestDEQPBaseTestIntepretResultStatus(object):
>>  def __init__(self):
>>  self.inst = None
>>  
>> -__OUT = (
>> -"dEQP Core 2014.x (0xcafebabe) starting..\n"
>> -"arget implementation = 'DRM'\n"
>> -"\n"
>> -"Test case 
>> 'dEQP-GLES2.functional.shaders.conversions.vector_to_vector.vec3_to_ivec3_fragment'..\n"
>> -"Vertex shader compile time = 0.129000 ms\n"
>> -"Fragment shader compile time = 0.264000 ms\n"
>> -"Link time = 0.814000 ms\n"
>> -"Test case duration in microseconds = 487155 us\n"
>> -"{stat} ({stat})\n"
>> -"\n"
>> -"DONE!\n"
>> -"\n"
>> -"Test run totals:\n"
>> -"Passed:{pass_}/1 (100.0%)\n"
>> -"Failed:{fail}/1 (0.0%)\n"
>> -"Not supported: {supp}/1 (0.0%)\n"
>> -"Warnings:  {warn}/1 (0.0%)\n"
>> -)
>> +__OUT = textwrap.dedent("""\
>> +dEQP Core 2014.x (0xcafebabe) starting..
>> +  target implementation = 'DRM'
>> +
>> +Test case 
>> 'dEQP-GLES2.functional.shaders.conversions.vector_to_vector.vec3_to_ivec3_fragment'..
>> +Vertex shader compile time = 0.129000 ms
>> +Fragment shader compile time = 0.264000 ms
>> +Link time = 0.814000 ms
>> +Test case duration in microseconds = 487155 us
>> +  {stat} ({stat})
>> +
>> +DONE!
>> +
>> +Test run totals:
>> +  Passed:{pass_}/1 (100.0%)
>> +  Failed:{fail}/1 (0.0%)
>> +  Not supported: {supp}/1 (0.0%)
>> +  Warnings:  {warn}/1 (0.0%)
>> +Test run was ABORTED!
>> +""")
>>  
>>  def __gen_stdout(self, status):
>>  assert status in ['Fail', 'NotSupported

Re: [Piglit] [PATCH] framework: Add handler for failure and error when expecting the other

2016-03-15 Thread Mark Janes
Dylan Baker  writes:

> Right now the JUnit backend knows what to do with a failure when it
> expects one (or a crash), but not what to do when it expects a failure
> and gets a crash (or vice versa).
>
> This patch teaches it what to do in that case, mark the test as a
> failure and give a message explaining why.
>
> cc: mark.a.ja...@intel.com
> signed-off-by: Dylan Baker 
> ---
>  framework/backends/junit.py | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
> index f9eec66..761b201 100644
> --- a/framework/backends/junit.py
> +++ b/framework/backends/junit.py
> @@ -151,6 +151,12 @@ class JUnitBackend(FileBackend):
>  err.text += "\n\nWARN: passing test as an expected 
> failure"
>  res = etree.SubElement(element, 'skipped',
> message='expected failure')
> +elif expected_result == 'error':
> +err.text += \
> +"\n\nERROR: Test should have been crash but was 
> failure"
> +res = etree.SubElement(element, 'failure',
> +   message='expected crash, but got '
> +   'failure')
>  else:
>  res = etree.SubElement(element, 'failure')
>  
> @@ -159,6 +165,12 @@ class JUnitBackend(FileBackend):
>  err.text += "\n\nWARN: passing test as an expected crash"
>  res = etree.SubElement(element, 'skipped',
> message='expected crash')
> +elif expected_result == 'failure':
> +err.text += \
> +"\n\nERROR: Test should have been failure but was 
> crash"
> +res = etree.SubElement(element, 'failure',
I think you want to insert an 'error' tag here  ^^^
> +   message='expected failure, but 
> got '
> +   'crash')
>  else:
>  res = etree.SubElement(element, 'error')
>  
> -- 
> 2.7.3
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] deqp-integration: Handle ResourceError.

2016-03-15 Thread Mark Janes
This patch doesn't fix the issue I see on HSW with Resource errors.

Dylan Baker  writes:

> This is a previously unseen error type generated on by the Vulkan CTS
> (which is dEQP based).
>
> Signed-off-by: Dylan Baker 
> ---
>
> Mark, I think this will fix the that jenkins bug we're seeing.
>
>  framework/test/deqp.py  | 15 +--
>  unittests/deqp_tests.py | 50 
> -
>  2 files changed, 38 insertions(+), 27 deletions(-)
>
> diff --git a/framework/test/deqp.py b/framework/test/deqp.py
> index 4fd51ec..80ce156 100644
> --- a/framework/test/deqp.py
> +++ b/framework/test/deqp.py
> @@ -115,12 +115,15 @@ def iter_deqp_test_cases(case_file):
>  
>  @six.add_metaclass(abc.ABCMeta)
>  class DEQPBaseTest(Test):
> -__RESULT_MAP = {"Pass": "pass",
> -"Fail": "fail",
> -"QualityWarning": "warn",
> -"InternalError": "fail",
> -"Crash": "crash",
> -"NotSupported": "skip"}
> +__RESULT_MAP = {
> +"Pass": "pass",
> +"Fail": "fail",
> +"QualityWarning": "warn",
> +"InternalError": "fail",
> +"Crash": "crash",
> +"NotSupported": "skip",
> +"ResourceError": "crash",
> +}
>  
>  @abc.abstractproperty
>  def deqp_bin(self):
> diff --git a/unittests/deqp_tests.py b/unittests/deqp_tests.py
> index 7143da2..7119fed 100644
> --- a/unittests/deqp_tests.py
> +++ b/unittests/deqp_tests.py
> @@ -28,6 +28,7 @@ tests
>  from __future__ import (
>  absolute_import, division, print_function, unicode_literals
>  )
> +import textwrap
>  
>  try:
>  from unittest import mock
> @@ -191,34 +192,35 @@ class TestDEQPBaseTestIntepretResultStatus(object):
>  def __init__(self):
>  self.inst = None
>  
> -__OUT = (
> -"dEQP Core 2014.x (0xcafebabe) starting..\n"
> -"arget implementation = 'DRM'\n"
> -"\n"
> -"Test case 
> 'dEQP-GLES2.functional.shaders.conversions.vector_to_vector.vec3_to_ivec3_fragment'..\n"
> -"Vertex shader compile time = 0.129000 ms\n"
> -"Fragment shader compile time = 0.264000 ms\n"
> -"Link time = 0.814000 ms\n"
> -"Test case duration in microseconds = 487155 us\n"
> -"{stat} ({stat})\n"
> -"\n"
> -"DONE!\n"
> -"\n"
> -"Test run totals:\n"
> -"Passed:{pass_}/1 (100.0%)\n"
> -"Failed:{fail}/1 (0.0%)\n"
> -"Not supported: {supp}/1 (0.0%)\n"
> -"Warnings:  {warn}/1 (0.0%)\n"
> -)
> +__OUT = textwrap.dedent("""\
> +dEQP Core 2014.x (0xcafebabe) starting..
> +  target implementation = 'DRM'
> +
> +Test case 
> 'dEQP-GLES2.functional.shaders.conversions.vector_to_vector.vec3_to_ivec3_fragment'..
> +Vertex shader compile time = 0.129000 ms
> +Fragment shader compile time = 0.264000 ms
> +Link time = 0.814000 ms
> +Test case duration in microseconds = 487155 us
> +  {stat} ({stat})
> +
> +DONE!
> +
> +Test run totals:
> +  Passed:{pass_}/1 (100.0%)
> +  Failed:{fail}/1 (0.0%)
> +  Not supported: {supp}/1 (0.0%)
> +  Warnings:  {warn}/1 (0.0%)
> +Test run was ABORTED!
> +""")
>  
>  def __gen_stdout(self, status):
>  assert status in ['Fail', 'NotSupported', 'Pass', 'QualityWarning',
> -  'InternalError', 'Crash']
> +  'InternalError', 'Crash', 'ResourceError']
>  
>  return self.__OUT.format(
>  stat=status,
>  pass_=1 if status == 'Pass' else 0,
> -fail=1 if status in ['Crash', 'Fail'] else 0,
> +fail=1 if status in ['Crash', 'Fail', 'ResourceError'] else 0,
>  supp=1 if status == 'InternalError' else 0,
>  warn=1 if status == 'QualityWarning' else 0,
>  )
> @@ -262,3 +264,9 @@ class TestDEQPBaseTestIntepretResultStatus(object):
>  self.inst.result.out = self.__gen_stdout('NotSupported')
>  self.inst.interpret_result()
>  nt.eq_(self.inst.result.result, 'skip')
> +
> +def test_resourceerror(self):
> +"""test.deqp.DEQPBaseTest.interpret_result: when ResourceError in 
> result the result is 'crash'"""
> +self.inst.result.out = self.__gen_stdout('ResourceError')
> +self.inst.interpret_result()
> +nt.eq_(self.inst.result.result, 'crash')
> -- 
> 2.7.3
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] Fix invalid glShaderSource call in pipeline_stats_comp.

2016-02-10 Thread Mark Janes
Reviewed-by: Mark Janes 

Kenneth Graunke  writes:

> The test was passing a pointer to an uninitialized integer value as the
> glShaderSource length parameter.  This is meant to be an array
> containing the length of each shader string.
>
> Instead, just pass NULL, which makes the GL assume the strings are
> null-terminated (which they are).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93335
> Cc: Jordan Justen 
> Cc: Mark Janes 
> ---
>  tests/spec/arb_pipeline_statistics_query/pipeline_stats_comp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/tests/spec/arb_pipeline_statistics_query/pipeline_stats_comp.c 
> b/tests/spec/arb_pipeline_statistics_query/pipeline_stats_comp.c
> index 47a36b0..3c511f1 100644
> --- a/tests/spec/arb_pipeline_statistics_query/pipeline_stats_comp.c
> +++ b/tests/spec/arb_pipeline_statistics_query/pipeline_stats_comp.c
> @@ -79,7 +79,6 @@ static void
>  dispatch_size(uint32_t x, uint32_t y, uint32_t z)
>  {
>   char *compute_shader;
> - GLint shader_string_size;
>   GLuint shader = glCreateShader(GL_COMPUTE_SHADER);
>   GLint ok;
>   static GLint prog = 0;
> @@ -92,7 +91,7 @@ dispatch_size(uint32_t x, uint32_t y, uint32_t z)
>  
>   glShaderSource(shader, 1,
>  (const GLchar **) &compute_shader,
> -&shader_string_size);
> +NULL);
>  
>   glCompileShader(shader);
>  
> -- 
> 2.7.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] piglit-fbo: check the proper extension dependency when attaching a color texture

2016-01-15 Thread Mark Janes
Alejandro Piñeiro  writes:

> On 15/01/16 13:07, Mark Janes wrote:
>> This patch causes piglit.spec.ext_framebuffer_multisample.accuracy to
>> fail on g33 and g965.  Is that what you expected?
>
> No, I was not expecting any regression. It seemed a trivial change,
> based on that attach_x was using, and it didn't raise any regression on
> my haswell machine.
>
> Unfourtunately, I think that I don't have access to those old GPUs. The
> more similar thing is an Ironlake. Could you confirm if it is working on
> Ironlake? Could you share more details (stdout/stderr) of what it is
> failing?

It looks like the tests were previously skipping on these platforms.
There may be platform limitations that prevent the tests from passing,
now that they are enabled.

Sample output from one of the tests on g965:
/tmp/build_root/m64/lib/piglit/bin/ext_framebuffer_multisample-accuracy 
all_samples depth_draw depthstencil -auto -fbo
Pixels that should be unlit
  count = 137813
  RMS error = 0.241702
Pixels that should be totally lit
  count = 119798
  RMS error = 0.568553
The error threshold for unlit and totally lit pixels test is 0.016650
Pixels that should be partially lit
  count = 4533
  RMS error = 0.593167
The error threshold for partially lit pixels is 0.333000
Samples = 0, Result = fail

>
>
>> Alejandro Piñeiro  writes:
>>
>>> attach_color_texture uses TEXTURE_RECTANGLE, so it should check
>>> for GL_ARB_texture_rectangle.
>>>
>>> attach_multisample_color_texture uses multisample texture targets,
>>> so it should check for GL_ARB_texture_multisample.
>>>
>>> Before this patch, the dependency check was wrongly switched.
>>> ---
>>>  tests/util/piglit-fbo.cpp | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/util/piglit-fbo.cpp b/tests/util/piglit-fbo.cpp
>>> index bc705aa..1c4cd6c 100644
>>> --- a/tests/util/piglit-fbo.cpp
>>> +++ b/tests/util/piglit-fbo.cpp
>>> @@ -208,14 +208,14 @@ Fbo::try_setup(const FboConfig &new_config)
>>> if (config.num_samples == 0) {
>>>  
>>> /* Attach textures as color attachments */
>>> -   piglit_require_extension("GL_ARB_texture_multisample");
>>> +   piglit_require_extension("GL_ARB_texture_rectangle");
>>> for (int i = 0; i < config.num_tex_attachments; i++)
>>> attach_color_texture(new_config, i);
>>>  
>>> } else {
>>>  
>>> /* Attach multisample textures as color attachments */
>>> -   piglit_require_extension("GL_ARB_texture_rectangle");
>>> +   piglit_require_extension("GL_ARB_texture_multisample");
>>> for (int i = 0; i < config.num_tex_attachments; i++)
>>> attach_multisample_color_texture(new_config, i);
>>> }
>>> -- 
>>> 2.1.4
>>>
>>> ___
>>> Piglit mailing list
>>> Piglit@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/piglit
>
> -- 
> Alejandro Piñeiro (apinhe...@igalia.com)
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] piglit-fbo: check the proper extension dependency when attaching a color texture

2016-01-15 Thread Mark Janes
This patch causes piglit.spec.ext_framebuffer_multisample.accuracy to
fail on g33 and g965.  Is that what you expected?

Alejandro Piñeiro  writes:

> attach_color_texture uses TEXTURE_RECTANGLE, so it should check
> for GL_ARB_texture_rectangle.
>
> attach_multisample_color_texture uses multisample texture targets,
> so it should check for GL_ARB_texture_multisample.
>
> Before this patch, the dependency check was wrongly switched.
> ---
>  tests/util/piglit-fbo.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/util/piglit-fbo.cpp b/tests/util/piglit-fbo.cpp
> index bc705aa..1c4cd6c 100644
> --- a/tests/util/piglit-fbo.cpp
> +++ b/tests/util/piglit-fbo.cpp
> @@ -208,14 +208,14 @@ Fbo::try_setup(const FboConfig &new_config)
>   if (config.num_samples == 0) {
>  
>   /* Attach textures as color attachments */
> - piglit_require_extension("GL_ARB_texture_multisample");
> + piglit_require_extension("GL_ARB_texture_rectangle");
>   for (int i = 0; i < config.num_tex_attachments; i++)
>   attach_color_texture(new_config, i);
>  
>   } else {
>  
>   /* Attach multisample textures as color attachments */
> - piglit_require_extension("GL_ARB_texture_rectangle");
> + piglit_require_extension("GL_ARB_texture_multisample");
>   for (int i = 0; i < config.num_tex_attachments; i++)
>   attach_multisample_color_texture(new_config, i);
>   }
> -- 
> 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] igt: Restore handling of special igt error codes

2015-12-16 Thread Mark Janes
Reviewed-by: Mark Janes 

Daniel Vetter  writes:

> In
>
> commit 5cbc1834fd47e7f475afc85e1638762e5b221a81
> Author: Dylan Baker 
> Date:   Mon Dec 14 15:34:11 2015 -0800
>
> framework/test/base.py: Handle fail cases for tests.
>
> the Test baseclass was made more robust in it's default behaviour, but
> that totally broke igt error code handling.
>
> CI is ... rather unhappy about it, so fix it asap.
>
> Cc: Dylan Baker 
> Cc: Mark Janes 
> Signed-off-by: Daniel Vetter 
> ---
>  tests/igt.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/igt.py b/tests/igt.py
> index 4998ab203a23..074df08cc88c 100644
> --- a/tests/igt.py
> +++ b/tests/igt.py
> @@ -109,6 +109,8 @@ class IGTTest(Test):
>  self.timeout = 600
>  
>  def interpret_result(self):
> +super(IGTTest, self).interpret_result()
> +
>  if self.result.returncode == 0:
>  self.result.result = 'pass'
>  elif self.result.returncode == 77:
> @@ -118,8 +120,6 @@ class IGTTest(Test):
>  else:
>  self.result.result = 'fail'
>  
> -super(IGTTest, self).interpret_result()
> -
>  
>  def list_tests(listname):
>  """Parse igt test list and return them as a list."""
> -- 
> 2.6.4
___
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.

2015-12-14 Thread Mark Janes
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


Re: [Piglit] [PATCH 0/2] Handle subtests in JUnit output

2015-12-10 Thread Mark Janes
There are a few things with this patch that complicate my use of junit:

 1) the stdout for the parent test is in the stored in the testsuite tag
in the junit xml.  However, I can't access this data from the
jenkins ui.  If a subtest fails, I won't be able to write up a bug
from the Jenkins UI.

 2) the subtests reference the parent test by descriptive name (eg "I am
a subtest of imageAtomicMin"), but the user needs to find the test
in the junit hierarchy
(piglit.spec.arb_shader_image_load_store.atomicity)

 3) each subtest's testcase tag lacks the classname attribute, which
lets me identify the test uniquely.  To use this format, I would
have to check if each test is in a suite, then append the suite's
name.

one option for making this usable is to keep the structure flat (no
nesting of suites).  The parent test stays the same as today, and the
subtests are simply added as subsequent tests at the same level.  The
stdout for a subtest refers to the parent test in stdout, so the user
can find it in the jenkins UI.

It's unfortunate to have to hack around the details of how Jenkins
displays tests.  Maybe there is a better option...

-Mark

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

> From: Dylan Baker 
>
> This series changes the way the JUnit backend generates results for
> tests with subtests. Rather than generating them as a single test and
> ignoring the subtests it generates them as a testsuite, and then adds
> each subtest as a test. This allows subtests to be represented nicely
> and to avoid duplicating large amounts of data in the xml file.
>
> Dylan Baker (2):
>   framework/backends/junit.py: refactor _write into several helpers.
>   framework/backends/junit.py: Handle tests with subtests as testsuite
> elements
>
>  framework/backends/junit.py | 263 
> +++-
>  framework/tests/junit_backends_tests.py | 206 -
>  2 files changed, 393 insertions(+), 76 deletions(-)
>
> -- 
> 2.6.3
>
> ___
> 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] arb_draw_indirect: require extension in shared-binding test

2015-11-25 Thread Mark Janes
Reviewed-by: Mark Janes 

Tapani Palli  writes:

> Signed-off-by: Tapani Pälli 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93093
> ---
>  tests/spec/arb_draw_indirect/draw-arrays-shared-binding.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/spec/arb_draw_indirect/draw-arrays-shared-binding.c 
> b/tests/spec/arb_draw_indirect/draw-arrays-shared-binding.c
> index d73f5f8..d487e5d 100644
> --- a/tests/spec/arb_draw_indirect/draw-arrays-shared-binding.c
> +++ b/tests/spec/arb_draw_indirect/draw-arrays-shared-binding.c
> @@ -97,6 +97,7 @@ piglit_init(int argc, char **argv)
>   GLuint prog, vbo, indirect;
>  
>   piglit_require_GLSL();
> + piglit_require_extension("GL_ARB_draw_indirect");
>  
>   prog = piglit_build_simple_program(vs_text, fs_text);
>   glUseProgram(prog);
> -- 
> 2.5.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 00/13] Add more arb_shader_storage_buffer_object tests

2015-10-16 Thread Mark Janes
Emil Velikov  writes:

> On 25 September 2015 at 16:40, Mark Janes  wrote:
>> With the push of SSBO support in mesa, one of these tests continues to
>> fail, but only on broadwell:
>>
>> piglit.spec.arb_shader_storage_buffer_object.array-ssbo-binding
>>
>> Standard Output
>>
>> 
>> /tmp/build_root/m64/lib/piglit/bin/arb_shader_storage_buffer_object-array-ssbo-binding
>>  -auto -fbo
>> piglit: debug: Requested an OpenGL 3.2 Core Context, and received a 
>> matching 3.3 context
>>
>> Wrong 0 value in buffer[1]: 0.00
>>
> The test seems to be comparing a float with an integer there. Perhaps
> one should use {+,-}0.0f or lambda ?
>
> float *map;
> ...
> if (map[i] != 0) {
>printf("Wrong %d value in buffer[0]: %.2f\n",
>   i, map[i]);

The output of the test is confusing: a value of 0 or >10 is a failure.
The float comparison is not the issue.

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


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] Allow junit to be used for summary generation

2015-10-09 Thread Mark Janes
Jose Fonseca  writes:

> On 09/10/15 01:21, baker.dyla...@gmail.com wrote:
>> 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.
>>
>> 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).
>>
>
> I don't object this, but instead of this brittle testname (de)munge, 
> have you considered/tried using an additional XML attribute with 
> unmodified piglit test name?  I expect that Jenkins junit parser will 
> just outright ignore it.

My recollection is that we found the junit parser to be strict:

https://svn.jenkins-ci.org/trunk/hudson/dtkit/dtkit-format/dtkit-junit-model/src/main/resources/com/thalesgroup/dtkit/junit/model/xsd/junit-4.xsd

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


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

2015-10-09 Thread Mark Janes
Dylan Baker  writes:

> On Thu, Oct 8, 2015 at 6:09 PM, 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.
>>
>
> I think that we should make the filter work for both. I'm not sure yet what
> the right solution is for that, but I think it's worth making happen, I
> think there are a lot of reasons for it, but the main one I see is that it
> would eliminate distinctions between the backends.
>
> I also have a series I'm working on that uses json for the intermediate
> atomic writes, and then having a function that just converts the
> TestrunResult object generated at the end of the run into the output we
> want (like json or junit, or nunit, or whatever else). This reduces code
> duplication a lot and reduces the number of code paths that aren't being
> tested by me a lot. This is advantageous in unify the expected
> failure/crash code to use the native piglit names rather than the junit
> names, since we could apply it before converting the tests and writing them
> out. That feels like the best solution to me, though it may mean some work
> converting our existing crash/fail lists.
>
>
>>
>> > 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.
>>
>
> It might be, but I suspect that someone will try it at some point. I
> imagine a workflow like:
> 1) developer gets a report about breaking some system
> 2) developer makes changes, and runs subset of piglit on their system
> 3) developer wants to compare those changes to the original junit
>

For the example workflow, the developer needs the pass/fail
configuration files.  Without them, the local results will not
correspond to what the CI produces.  We can provide those files, but
will developers really want to use them for their test runs?  The
configurations change several times a day as bugs are fixed or
regressions are written up as bugs.

If you want to solve the general problem, I think the most sensible
thing is to place limits on test names:

 * xml compatible (eg '<\/>&')
 * json compatible (eg '{}:')
 * config file compatible (eg '[]=:;#')
 * jenkins compatible ("api","search")

 and possibly even
 * command-line friendly (eg '()> &*')

Then, alter all existing problematic test names and assert that all
future test names conform.  Munging test names just adds confusion:  to
bisect a test, I'll need to reverse the '.' -> '__' transformation
before using names in --include-tests.

Personally, I find it irritating that so many test names incorporate
exotic characters.  But is it really worth the effort to rename
everything?  Will developers agree to the restrictions?  What about
BuildBot or other CI restrictions?

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


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

2015-10-08 Thread Mark Janes
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.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [RESEND PATCH 1/4] framework: Add a TimeAttribute class

2015-10-08 Thread Mark Janes
Series is

Reviewed-by: Mark Janes 

dylanx.c.ba...@intel.com writes:

> From: Dylan Baker 
>
> This class will provide a new interface for time, one that stores the
> start and stop times, rather than the total time. It then provides
> methods for getting the total and the delta (as used in the summary).
>
> This commit adds the new class and tests for the class, but doesn't
> start plugging it in yet.
>
> The goal of this change is to allow post-processing to determine which
> tests were running in parallel.
>
> Signed-off-by: Dylan Baker 
> ---
>  framework/backends/json.py   |  1 +
>  framework/results.py | 38 ++
>  framework/tests/results_tests.py | 34 --
>  3 files changed, 71 insertions(+), 2 deletions(-)
>
> diff --git a/framework/backends/json.py b/framework/backends/json.py
> index 935480b..4cc7957 100644
> --- a/framework/backends/json.py
> +++ b/framework/backends/json.py
> @@ -51,6 +51,7 @@ _DECODER_TABLE = {
>  'Subtests': results.Subtests,
>  'TestResult': results.TestResult,
>  'TestrunResult': results.TestrunResult,
> +'TimeAttribute': results.TimeAttribute,
>  'Totals': results.Totals,
>  }
>  
> diff --git a/framework/results.py b/framework/results.py
> index 26f4380..7eca5bd 100644
> --- a/framework/results.py
> +++ b/framework/results.py
> @@ -25,6 +25,7 @@ from __future__ import print_function, absolute_import
>  
>  import collections
>  import copy
> +import datetime
>  
>  from framework import status, exceptions, grouptools
>  
> @@ -103,6 +104,43 @@ class StringDescriptor(object):  # pylint: 
> disable=too-few-public-methods
>  raise NotImplementedError
>  
>  
> +class TimeAttribute(object):
> +"""Attribute of TestResult for time.
> +
> +This attribute provides a couple of nice helpers. It stores the start and
> +end time and provides methods for getting the total and delta of the 
> times.
> +
> +"""
> +__slots__ = ['start', 'end']
> +
> +def __init__(self, start=0.0, end=0.0):
> +self.start = start
> +self.end = end
> +
> +@property
> +def total(self):
> +return self.end - self.start
> +
> +@property
> +def delta(self):
> +return str(datetime.timedelta(seconds=self.total))
> +
> +def to_json(self):
> +return {
> +'start': self.start,
> +'end': self.end,
> +'__type__': 'TimeAttribute',
> +}
> +
> +@classmethod
> +def from_dict(cls, dict_):
> +dict_ = copy.copy(dict_)
> +
> +if '__type__' in dict_:
> +del dict_['__type__']
> +return cls(**dict_)
> +
> +
>  class TestResult(object):
>  """An object represting the result of a single test."""
>  __slots__ = ['returncode', '_err', '_out', 'time', 'command', 
> 'traceback',
> diff --git a/framework/tests/results_tests.py 
> b/framework/tests/results_tests.py
> index 1b11df0..57ca646 100644
> --- a/framework/tests/results_tests.py
> +++ b/framework/tests/results_tests.py
> @@ -1,4 +1,4 @@
> -# Copyright (c) 2014 Intel Corporation
> +# Copyright (c) 2014, 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
> @@ -213,7 +213,7 @@ class TestTestResult_to_json(object):
>  """results.TestResult.to_json: sets the out correctly"""
>  nt.eq_(self.dict['out'], self.json['out'])
>  
> -def test_out(self):
> +def test_exception(self):
>  """results.TestResult.to_json: sets the exception correctly"""
>  nt.eq_(self.dict['exception'], self.json['exception'])
>  
> @@ -581,3 +581,33 @@ class TestTestrunResultFromDict(object):
>status.Status),
> msg='Subtests should be type Status, but was "{}"'.format(
> type(self.test.tests['subtest'].subtests['foo'])))
> +
> +
> +def test_TimeAttribute_to_json():
> +"""results.TimeAttribute.to_json(): returns expected dictionary"""
> +baseline = {'start': 0.1, 'end':

Re: [Piglit] [PATCH] glsl-es: Verify restrictions on global variable initializers

2015-10-07 Thread Mark Janes

Reviewed-by: Mark Janes 
Tested-by: Mark Janes 

Ian Romanick  writes:

> From: Ian Romanick 
>
> Section 4.3 (Storage Qualifiers) of the OpenGL ES 1.00.17 spec says:
>
> "Declarations of globals without a storage qualifier, or with just
> the const qualifier, may include initializers, in which case they
> will be initialized before the first line of main() is executed.
> Such initializers must be a constant expression."
>
> The GLSL ES 3.00.4 spec has similar language.  Desktop GLSL does not
> restrict initializers for global variables.  Now it's okay for Matt to
> be irritated.
>
> Signed-off-by: Ian Romanick 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92304
> Cc: Tapani Pälli 
> Cc: Mark Janes 
> Cc: Marta Lofstedt 
> ---
>  .../global-initializer/from-attribute.vert | 14 +
>  .../compiler/global-initializer/from-constant.frag | 14 +
>  .../compiler/global-initializer/from-constant.vert | 14 +
>  .../compiler/global-initializer/from-global.frag   | 14 +
>  .../compiler/global-initializer/from-global.vert   | 14 +
>  .../compiler/global-initializer/from-sequence.frag | 14 +
>  .../compiler/global-initializer/from-sequence.vert | 14 +
>  .../compiler/global-initializer/from-uniform.frag  | 14 +
>  .../compiler/global-initializer/from-uniform.vert  | 14 +
>  .../compiler/global-initializer/from-varying.frag  | 14 +
>  .../global-initializer/from-attribute.vert | 26 
>  .../compiler/global-initializer/from-constant.frag | 28 +
>  .../compiler/global-initializer/from-constant.vert | 26 
>  .../compiler/global-initializer/from-global.frag   | 28 +
>  .../compiler/global-initializer/from-global.vert   | 26 
>  .../compiler/global-initializer/from-sequence.frag | 31 +++
>  .../compiler/global-initializer/from-sequence.vert | 29 ++
>  .../compiler/global-initializer/from-uniform.frag  | 28 +
>  .../compiler/global-initializer/from-uniform.vert  | 26 
>  .../compiler/global-initializer/from-varying.frag  | 28 +
>  .../compiler/global-initializer/from-constant.frag | 28 +
>  .../compiler/global-initializer/from-constant.vert | 25 
>  .../compiler/global-initializer/from-global.frag   | 28 +
>  .../compiler/global-initializer/from-global.vert   | 25 
>  .../compiler/global-initializer/from-in.frag   | 28 +
>  .../compiler/global-initializer/from-in.vert   | 25 
>  .../compiler/global-initializer/from-sequence.frag | 35 
> ++
>  .../compiler/global-initializer/from-sequence.vert | 32 
>  .../compiler/global-initializer/from-uniform.frag  | 28 +
>  .../compiler/global-initializer/from-uniform.vert  | 25 
>  30 files changed, 695 insertions(+)
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-attribute.vert
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-constant.frag
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-constant.vert
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-global.frag
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-global.vert
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-sequence.frag
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-sequence.vert
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-uniform.frag
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-uniform.vert
>  create mode 100644 
> tests/spec/glsl-1.10/compiler/global-initializer/from-varying.frag
>  create mode 100644 
> tests/spec/glsl-es-1.00/compiler/global-initializer/from-attribute.vert
>  create mode 100644 
> tests/spec/glsl-es-1.00/compiler/global-initializer/from-constant.frag
>  create mode 100644 
> tests/spec/glsl-es-1.00/compiler/global-initializer/from-constant.vert
>  create mode 100644 
> tests/spec/glsl-es-1.00/compiler/global-initializer/from-global.frag
>  create mode 100644 
> tests/spec/glsl-es-1.00/compiler/global-initializer/from-global.vert
>  create mode 100644 
> tests/spec/glsl-es-1.00/compiler/global-initializer/from-sequence.frag
>  create mode 100644 
> tests/spec/glsl-es-1.00/compiler/global-initializer/from-sequence.vert
>  create mode 100644 
> tests/spec/glsl-es-1.00/compiler/global-initializer/from-uni

Re: [Piglit] [PATCH 1/2] framework: add exception back to TestResult class

2015-10-01 Thread Mark Janes
patch 1 is

Reviewed-by: Mark Janes 
Tested-by: Mark Janes 

Dylan Baker  writes:

> Like command, this somehow didn't get moved to the new TestResult class.
> Generally this isn't a problem since this is a very uncommon path, but
> in some cases this does get hit, and it would cause an exception in the
> runner threads. Python has very bad exception handling in threads,
> namely it doesn't have any; this results in an exception being raised
> and remaining uncaught, terminating the thread, which in turn results in
> the test result being incomplete, even though it should have been crash.
>
> This patch fixes that situation.
>
> cc: Mark Janes 
> Signed-off-by: Dylan Baker 
> ---
>
> Mark, this should fix the problem where clipflat returns an incomplete
> in jenkins. Basically what was happening is that it was returning
> invalid JSON, and then trying to write that into the TestResult, which
> it couldn't raising another exception.
>
>  framework/results.py |  7 +--
>  framework/tests/results_tests.py | 10 ++
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/framework/results.py b/framework/results.py
> index 8d3fe17..26f4380 100644
> --- a/framework/results.py
> +++ b/framework/results.py
> @@ -106,7 +106,8 @@ class StringDescriptor(object):  # pylint: 
> disable=too-few-public-methods
>  class TestResult(object):
>  """An object represting the result of a single test."""
>  __slots__ = ['returncode', '_err', '_out', 'time', 'command', 
> 'traceback',
> - 'environment', 'subtests', 'dmesg', '__result', 'images']
> + 'environment', 'subtests', 'dmesg', '__result', 'images',
> + 'exception']
>  err = StringDescriptor('_err')
>  out = StringDescriptor('_out')
>  
> @@ -119,6 +120,7 @@ class TestResult(object):
>  self.dmesg = str()
>  self.images = None
>  self.traceback = None
> +self.exception = None
>  if result:
>  self.result = result
>  else:
> @@ -155,6 +157,7 @@ class TestResult(object):
>  'returncode': self.returncode,
>  'subtests': self.subtests,
>  'time': self.time,
> +'exception': self.exception,
>  }
>  return obj
>  
> @@ -174,7 +177,7 @@ class TestResult(object):
>  inst = cls()
>  
>  # TODO: There's probably a more clever way to do this
> -for each in ['returncode', 'time', 'command',
> +for each in ['returncode', 'time', 'command', 'exception',
>   'environment', 'result', 'dmesg']:
>  if each in dict_:
>  setattr(inst, each, dict_[each])
> diff --git a/framework/tests/results_tests.py 
> b/framework/tests/results_tests.py
> index 728d479..1b11df0 100644
> --- a/framework/tests/results_tests.py
> +++ b/framework/tests/results_tests.py
> @@ -194,6 +194,7 @@ class TestTestResult_to_json(object):
>  'b': 'fail',
>  },
>  'result': 'crash',
> +'exception': 'an exception',
>  }
>  
>  test = results.TestResult.from_dict(cls.dict)
> @@ -212,6 +213,10 @@ class TestTestResult_to_json(object):
>  """results.TestResult.to_json: sets the out correctly"""
>  nt.eq_(self.dict['out'], self.json['out'])
>  
> +def test_out(self):
> +"""results.TestResult.to_json: sets the exception correctly"""
> +nt.eq_(self.dict['exception'], self.json['exception'])
> +
>  def test_time(self):
>  """results.TestResult.to_json: sets the time correctly"""
>  nt.eq_(self.dict['time'], self.json['time'])
> @@ -244,6 +249,7 @@ class TestTestResult_from_dict(object):
>  'b': 'fail',
>  },
>  'result': 'crash',
> +'exception': 'an exception',
>  }
>  
>  cls.test = results.TestResult.from_dict(cls.dict)
> @@ -268,6 +274,10 @@ class TestTestResult_from_dict(object):
>  """results.TestResult.from_dict: sets environment properly"""
>  nt.eq_(self.test.environment, self.dict['environment'])
>  
> +def test_exception(self):
> +"""results.TestResult.from_dict: sets exception properly"""
> +nt.eq_(self.test.exception, self.dict['exception'])
> +
>  def test_subtests(self):
>  """results.TestResult.from_dict: sets subtests properly"""
>  nt.eq_(self.test.subtests, self.dict['subtests'])
> -- 
> 2.6.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] tests: Add integration for khronos CTS runner.

2015-09-28 Thread Mark Janes

Reviewed-by: Mark Janes 

Dylan Baker  writes:

> This adds support for running Khronos' deqp-based conformance suite with
> piglit. This gives access to all of the powerful features of pigilt,
> per-process tests, the junit and json backends, the summary tools, and
> the familiar interface, with minimal fuss.
>
> This is a very small change, since it is deqp-based, and piglit has a
> framework for handling deqp based suites already.
>
> cc: Mark Janes 
> Signed-off-by: Dylan Baker 
> ---
>  piglit.conf.example | 10 +
>  tests/cts.py| 63 
> +
>  2 files changed, 73 insertions(+)
>  create mode 100644 tests/cts.py
>
> diff --git a/piglit.conf.example b/piglit.conf.example
> index e1f421c..0d85401 100644
> --- a/piglit.conf.example
> +++ b/piglit.conf.example
> @@ -71,6 +71,16 @@ testB
>  ; overrides the value set here.
>  ;extra_args=--deqp-visibility hidden
>  
> +[cts]
> +; path to the cts executable
> +; can be overwritten by PIGILT_CTS_BIN environment variable
> +;bin=/home/knuth/cts/cts/glcts
> +
> +; Space-separated list of extra command line arguments for cts. The
> +; option is not required. The environment variable PIGLIT_CTS_EXTRA_ARGS
> +; overrides the value set here.
> +;extra_args=--deqp-visibility hidden
> +
>  ; Section for specific oclconform test.  One of these sections is required 
> for
>  ; each test list in the oclconform section and must be called:
>  ; oclconform-$testname
> diff --git a/tests/cts.py b/tests/cts.py
> new file mode 100644
> index 000..5669f88
> --- /dev/null
> +++ b/tests/cts.py
> @@ -0,0 +1,63 @@
> +# Copyright (c) 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 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.
> +
> +"""Piglit integration for Khronos CTS tests.
> +
> +By default this will run GLES2, GLES3, GLES31, and GLESEXT test cases. Those
> +desiring to run only a subset of them should consider using the -t or -x
> +options to include or exclude tests.
> +
> +for example:
> +./piglit run cts -c foo -t ES3- would run only ES3 tests (note the dash to
> +exclude ES31 tests)
> +
> +"""
> +
> +from __future__ import absolute_import, division, print_function
> +import itertools
> +
> +from framework.test import deqp
> +
> +__all__ = ['profile']
> +
> +_CTS_BIN = deqp.get_option('PIGLIT_CTS_BIN', ('cts', 'bin'))
> +
> +_EXTRA_ARGS = deqp.get_option('PIGLIT_CTS_EXTRA_ARGS', ('cts', 'extra_args'),
> +  default='').split()
> +
> +
> +class DEQPCTSTest(deqp.DEQPBaseTest):
> +deqp_bin = _CTS_BIN
> +extra_args = [x for x in _EXTRA_ARGS if not x.startswith('--deqp-case')]
> +
> +
> +profile = deqp.make_profile(  # pylint: disable=invalid-name
> +itertools.chain(
> +deqp.iter_deqp_test_cases(
> +deqp.gen_caselist_txt(_CTS_BIN, 'ES2-CTS-cases.txt', 
> _EXTRA_ARGS)),
> +deqp.iter_deqp_test_cases(
> +deqp.gen_caselist_txt(_CTS_BIN, 'ES3-CTS-cases.txt', 
> _EXTRA_ARGS)),
> +deqp.iter_deqp_test_cases(
> +deqp.gen_caselist_txt(_CTS_BIN, 'ES31-CTS-cases.txt', 
> _EXTRA_ARGS)),
> +deqp.iter_deqp_test_cases(
> +deqp.gen_caselist_txt(_CTS_BIN, 'ESEXT-CTS-cases.txt',
> +  _EXTRA_ARGS)),
> +),
> +DEQPCTSTest)
> -- 
> 2.5.3
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 00/13] Add more arb_shader_storage_buffer_object tests

2015-09-25 Thread Mark Janes
Emil Velikov  writes:

> On 25 September 2015 at 16:40, Mark Janes  wrote:
>> With the push of SSBO support in mesa, one of these tests continues to
>> fail, but only on broadwell:
>>
>> piglit.spec.arb_shader_storage_buffer_object.array-ssbo-binding

A subsequent run on the CI shows this test to fail intermittently on
broadwell.  I'm supposed to be taking the day off today, but I'll look
at this more thoroughly on Monday.

>>
>> Standard Output
>>
>> 
>> /tmp/build_root/m64/lib/piglit/bin/arb_shader_storage_buffer_object-array-ssbo-binding
>>  -auto -fbo
>> piglit: debug: Requested an OpenGL 3.2 Core Context, and received a 
>> matching 3.3 context
>>
>> Wrong 0 value in buffer[1]: 0.00
>>
> The test seems to be comparing a float with an integer there. Perhaps
> one should use {+,-}0.0f or lambda ?
>
> float *map;
> ...
> if (map[i] != 0) {
>printf("Wrong %d value in buffer[0]: %.2f\n",
>   i, map[i]);
>
> -Emil
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 00/13] Add more arb_shader_storage_buffer_object tests

2015-09-25 Thread Mark Janes
With the push of SSBO support in mesa, one of these tests continues to
fail, but only on broadwell:

piglit.spec.arb_shader_storage_buffer_object.array-ssbo-binding

Standard Output


/tmp/build_root/m64/lib/piglit/bin/arb_shader_storage_buffer_object-array-ssbo-binding
 -auto -fbo
piglit: debug: Requested an OpenGL 3.2 Core Context, and received a 
matching 3.3 context

Wrong 0 value in buffer[1]: 0.00

Did you expect this test to pass on broadwell?

-Mark

Samuel Iglesias Gonsalvez  writes:

> This patchset adds piglit tests for arb_shader_storage_buffer_object
> extension.
>
> They are also available on this repository:
>
> $ git clone -b arb_shader_storage_buffer_object-v1 \
>   https://github.com/Igalia/piglit.git
>
> Thanks,
>
> Sam
>
> Samuel Iglesias Gonsalvez (13):
>   arb_shader_storage_buffer_object: Add preprocessor tests
>   arb_shader_storage_buffer_object: add compiler tests
>   arb_shader_storage_buffer_object: Add linker tests
>   arb_shader_storage_buffer: Add rendering test
>   arb_shader_storage_buffer_object: add test for
> GL_MAX_SHADER_STORAGE_BUFFER_SIZE
>   arb_shader_storage_buffer_object: add test to check glGetIntegeri_v()
> queries
>   arb_shader_storage_buffer_object: Add new test for glDeleteBuffers()
> behavior
>   arb_shader_storage_buffer_object: Add test to check maximum number of
> allowed SSBOs
>   arb_shader_storage_buffer_object: New test for setting/getting block
> bindings.
>   arb_shader_storage_buffer_object: Add test for setting binding point
> to an array of SSBOs
>   arb_shader_storage_buffer_object: add test to check SSBO writes with
> layout(std430)
>   arb_shader_storage_buffer_object: add test to check SSBO writes with
> layout(std140)
>   arb_shader_storage_buffer_object: Add test for buffer variable queries
>
>  tests/all.py   |  21 ++
>  .../CMakeLists.gl.txt  |  10 +
>  .../array-ssbo-binding.c   | 149 +
>  .../extension-disabled-shader-storage-block.frag   |  15 +
>  .../compiler/extension-disabled-std430.frag|  13 +
>  .../compiler/layout-std430-non-shader-storage.frag |  24 ++
>  .../compiler/layout-std430-within-block.frag   |  22 ++
>  .../compiler/shader-storage-block-initializer.frag |  21 ++
>  .../compiler/shader-storage-block-sampler.frag |  23 ++
>  .../compiler/shader-storage-outside-block.frag |  20 ++
>  .../compiler/unsized-array-argument-function.frag  |  29 ++
>  .../unsized-array-not-in-last-position.frag|  24 ++
>  .../deletebuffers.c| 105 ++
>  .../getintegeri_v.c| 120 +++
>  .../layout-std140-write-shader.c   | 161 +
>  .../layout-std430-write-shader.c   | 185 +++
>  ...hader-storage-block-different-def-2.shader_test |  40 +++
>  ...hader-storage-block-different-def-3.shader_test |  40 +++
>  .../shader-storage-block-different-def.shader_test |  38 +++
>  ...shader-storage-block-different-size.shader_test |  41 +++
>  .../arb_shader_storage_buffer_object/maxblocks.c   | 358 
> +
>  .../maxshaderstorageblocksize.c| 240 ++
>  .../preprocessor/define.frag   |  19 ++
>  .../preprocessor/define.vert   |  19 ++
>  .../program-interface-query.c  | 166 ++
>  .../arb_shader_storage_buffer_object/rendering.c   | 232 +
>  .../shaderstorageblockbinding.c| 125 +++
>  27 files changed, 2260 insertions(+)
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/array-ssbo-binding.c
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/compiler/extension-disabled-shader-storage-block.frag
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/compiler/extension-disabled-std430.frag
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/compiler/layout-std430-non-shader-storage.frag
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/compiler/layout-std430-within-block.frag
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/compiler/shader-storage-block-initializer.frag
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/compiler/shader-storage-block-sampler.frag
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/compiler/shader-storage-outside-block.frag
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/compiler/unsized-array-argument-function.frag
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/compiler/unsized-array-not-in-last-position.frag
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/deletebuffers.c
>  create mode 100644 
> tests/spec/arb_shader_storage_buffer_object/getintegeri_v.c
>  create mode 100644 
> tests/spec/a

Re: [Piglit] [PATCH] gl-3.2: fix layered-rendering test

2015-08-18 Thread Mark Janes
Tested-by: Mark Janes 
Timothy Arceri  writes:

> Samples must be greater than 0 and color formats must match.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91670
> ---
>  tests/spec/gl-3.2/layered-rendering/framebuffertexture.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/spec/gl-3.2/layered-rendering/framebuffertexture.c 
> b/tests/spec/gl-3.2/layered-rendering/framebuffertexture.c
> index d6eef1e..88599de 100644
> --- a/tests/spec/gl-3.2/layered-rendering/framebuffertexture.c
> +++ b/tests/spec/gl-3.2/layered-rendering/framebuffertexture.c
> @@ -137,10 +137,10 @@ create_bind_texture(GLenum textureType) {
>   }
>   break;
>   case GL_TEXTURE_2D_MULTISAMPLE:
> - glTexImage2DMultisample(textureType, 0, GL_RGB, 6, 6, GL_FALSE);
> + glTexImage2DMultisample(textureType, 1, GL_RGBA, 6, 6, 
> GL_FALSE);
>   break;
>   case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
> - glTexImage3DMultisample(textureType, 0, GL_RGB,
> + glTexImage3DMultisample(textureType, 1, GL_RGBA,
>   6, 6, 6, GL_FALSE);
>   break;
>   }
> -- 
> 2.4.3
>
> ___
> 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 v2] arb_program_interface_query: fix getprogramresourceiv errors

2015-08-12 Thread Mark Janes
Tested-by:  Mark Janes 

Tapani Pälli  writes:

> Move IS_PER_PATCH as part of tessellation queries, require
> GL_ARB_compute_shader for compute shader enum queries.
>
> Strictly speaking, spec would allow to query GL_REFERENCED_BY_COMPUTE_SHADER
> without having compute shaders but as it disallows GL_COMPUTE_SUBROUTINE and
> GL_COMPUTE_SUBROUTINE_UNIFORM if there is no compute shader support, it is
> likely that this is a bug in the spec.
>
> v2: add missing parenthesis! (spotted by Curro)
>
> Signed-off-by: Tapani Pälli 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91608
> ---
>  tests/spec/arb_program_interface_query/getprogramresourceiv.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/tests/spec/arb_program_interface_query/getprogramresourceiv.c 
> b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
> index 03f2fc6..769e29f 100755
> --- a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
> +++ b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
> @@ -571,16 +571,15 @@ check_extensions_prop(GLenum prop)
>   }
>  
>   if ((prop == GL_REFERENCED_BY_TESS_CONTROL_SHADER ||
> -  prop == GL_REFERENCED_BY_TESS_EVALUATION_SHADER) &&
> +  prop == GL_REFERENCED_BY_TESS_EVALUATION_SHADER ||
> + prop == GL_IS_PER_PATCH) &&
>!piglit_is_extension_supported("GL_ARB_tessellation_shader")) {
>return false;
>   }
>  
>   if ((prop == GL_REFERENCED_BY_COMPUTE_SHADER ||
> -  prop == GL_COMPUTE_SUBROUTINE_UNIFORM ||
> -  prop == GL_IS_PER_PATCH) &&
> -  !piglit_is_extension_supported("GL_ARB_compute_shader") &&
> -  !piglit_is_extension_supported("GL_ARB_shader_image_load_store")) {
> +  prop == GL_COMPUTE_SUBROUTINE_UNIFORM) &&
> +  !piglit_is_extension_supported("GL_ARB_compute_shader")) {
>return false;
>   }
>  
> -- 
> 2.1.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 2/2] arb_gpu_shader5: Fix rounding instability in UBO and sampler indexing vs-nonuniform-control-flow tests.

2015-06-30 Thread Mark Janes

Tested-by: Mark Janes 

Francisco Jerez  writes:

> Fixes a rounding instability that would cause shader_runner to probe
> pixels offset by one for some points close to the right edge of the
> window on systems using x87 floating point arithmetic with certain
> compiler versions (the test seemed to work fine when built with GCC
> v5.1.0, but failed on some systems with GCC v4.9.2).
>
> The reason for the instability was that the default window height and
> width of 250 pixels was evenly divisible by all fractions used as
> point coordinates, what would cause the coordinates passed to relative
> rgba probes to lie precisely on the boundary between four pixels,
> giving unpredictable results in presence of the slightest rounding
> error.
>
> Instead use a coprime of 10 as window size to guarantee that there's
> always one fragment closer than the other three.  The 1/250 half-pixel
> offset previously used in the vertex shader now becomes unnecessary
> because the different fragment locations are sufficient to guarantee
> consistent rasterization results.
>
> This gets rid of the points which had one of the coordinates equal to
> 0 and replaces them with points close to the top and right edges
> because they would have necessarily been at a half-integer distance
> from the closest fragments regardless of the window size.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90993
> Reported-by: Mark Janes 
> ---
>  .../vs-nonuniform-control-flow.shader_test | 53 
> --
>  .../vs-nonuniform-control-flow.shader_test | 53 
> --
>  2 files changed, 58 insertions(+), 48 deletions(-)
>
> diff --git 
> a/tests/spec/arb_gpu_shader5/execution/sampler_array_indexing/vs-nonuniform-control-flow.shader_test
>  
> b/tests/spec/arb_gpu_shader5/execution/sampler_array_indexing/vs-nonuniform-control-flow.shader_test
> index 1c81186..3945987 100644
> --- 
> a/tests/spec/arb_gpu_shader5/execution/sampler_array_indexing/vs-nonuniform-control-flow.shader_test
> +++ 
> b/tests/spec/arb_gpu_shader5/execution/sampler_array_indexing/vs-nonuniform-control-flow.shader_test
> @@ -17,6 +17,13 @@
>  GLSL >= 1.50
>  GL_ARB_gpu_shader5
>  
> +# Take a coprime of 10 as window size to guarantee that the decimal
> +# fractions used below as point coordinates don't evenly divide the
> +# framebuffer size, what would result in points ending up precisely
> +# halfway from the centers of two fragments causing rounding
> +# instability during rasterization and pixel probes.
> +SIZE 251 251
> +
>  [vertex shader]
>  #version 150
>  #extension GL_ARB_gpu_shader5: require
> @@ -37,9 +44,7 @@ void main()
> v[i] = texture(s[(n + i) % 4u], vec2(0.5, 0.5)).x;
>  }
>  
> -gl_Position = vec4(-1 + 1.0 / 250.0 + vertex.x * 2,
> -   -1 + 1.0 / 250.0 + vertex.y * 2,
> -   0, 1);
> +gl_Position = vec4(-1 + vertex.x * 2, -1 + vertex.y * 2, 0, 1);
>  color = v;
>  }
>  
> @@ -56,54 +61,54 @@ void main()
>  
>  [vertex data]
>  vertex/float/3
> -0.0 0.0 0.0
> -0.0 0.1 0.0
> -0.0 0.2 0.0
> -0.0 0.3 0.0
> -0.0 0.4 0.0
> -0.0 0.5 0.0
> -0.1 0.0 0.0
>  0.1 0.1 0.0
>  0.1 0.2 0.0
>  0.1 0.3 0.0
>  0.1 0.4 0.0
>  0.1 0.5 0.0
> -0.2 0.0 0.0
> +0.1 0.6 0.0
>  0.2 0.1 0.0
>  0.2 0.2 0.0
>  0.2 0.3 0.0
>  0.2 0.4 0.0
>  0.2 0.5 0.0
> -0.3 0.0 0.0
> +0.2 0.6 0.0
>  0.3 0.1 0.0
>  0.3 0.2 0.0
>  0.3 0.3 0.0
>  0.3 0.4 0.0
>  0.3 0.5 0.0
> -0.4 0.0 0.0
> +0.3 0.6 0.0
>  0.4 0.1 0.0
>  0.4 0.2 0.0
>  0.4 0.3 0.0
>  0.4 0.4 0.0
>  0.4 0.5 0.0
> -0.5 0.0 0.0
> +0.4 0.6 0.0
>  0.5 0.1 0.0
>  0.5 0.2 0.0
>  0.5 0.3 0.0
>  0.5 0.4 0.0
>  0.5 0.5 0.0
> -0.6 0.0 0.0
> +0.5 0.6 0.0
>  0.6 0.1 0.0
>  0.6 0.2 0.0
>  0.6 0.3 0.0
>  0.6 0.4 0.0
>  0.6 0.5 0.0
> -0.7 0.0 0.0
> +0.6 0.6 0.0
>  0.7 0.1 0.0
>  0.7 0.2 0.0
>  0.7 0.3 0.0
>  0.7 0.4 0.0
>  0.7 0.5 0.0
> +0.7 0.6 0.0
> +0.8 0.1 0.0
> +0.8 0.2 0.0
> +0.8 0.3 0.0
> +0.8 0.4 0.0
> +0.8 0.5 0.0
> +0.8 0.6 0.0
>  
>  [test]
>  clear color 0.2 0.2 0.2 0.2
> @@ -137,35 +142,35 @@ draw arrays GL_POINTS 0 48
>  # the implementation doesn't take this possibility into account.
>  # Probe a bunch of pixels for good measure.
>  #
> -relative probe rgba (0.0, 0.1) (0.4, 0.6, 0.8, 0.2)
> -relative probe rgba (0.0, 0.2) (0.4, 0.6, 0.8, 0.2)
> -relative probe rgba (0.0, 0.4) (0.4, 0.6, 0.8, 0.2)
> -relative probe rgba (0.0, 0.5) (0.4, 0.6, 0.8, 0.2)
> -relative probe rgba (0.1, 0.0) (0.4, 0.6, 0.8, 0.2)
>  relative probe rgba (0.1, 0.2) (

[Piglit] [PATCH v2] Ignore piglit warning status in JUnit

2015-06-26 Thread Mark Janes
JUnit has no concept of "warning".  It supports the following
statuses:

 - skip
 - success
 - fail
 - error

A test which emits a warning is more accurately represented as
"success" in JUnit.

v2: Continue to report failure for "dmesg-warn", which is more serious
than "warn".  (from Ilia Mirkin)
---
 framework/backends/junit.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/framework/backends/junit.py b/framework/backends/junit.py
index 632e516..8c8639e 100644
--- a/framework/backends/junit.py
+++ b/framework/backends/junit.py
@@ -142,8 +142,7 @@ class JUnitBackend(FileBackend):
 if data['result'] == 'skip':
 res = etree.SubElement(element, 'skipped')
 
-elif data['result'] in ['warn', 'fail', 'dmesg-warn',
-'dmesg-fail']:
+elif data['result'] in ['fail', 'dmesg-warn', 'dmesg-fail']:
 if expected_result == "failure":
 err.text += "\n\nWARN: passing test as an expected failure"
 res = etree.SubElement(element, 'skipped',
-- 
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-26 Thread Mark Janes
Ilia Mirkin  writes:

> On Fri, Jun 26, 2015 at 2:44 PM, Dylan Baker  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


Re: [Piglit] [PATCH] Ignore piglit warning status in JUnit

2015-06-26 Thread Mark Janes
Ilia Mirkin  writes:

> So a test that otherwise passes (i.e. has a "pass" result code) but
> generates errors in dmesg will get converted to a "dmesg-warn" code.
> You were previously treating those as failures, but now will treat
> them as success.

Yes.  I used the comment in framework/status.py to determine what
severities of piglit status correspond to JUnit failures:

Status ordering from best to worst:

pass
dmesg-warn
warn
dmesg-fail
fail
timeout
crash

If warn doesn't trigger a failure, then dmesg-warn shouldn't either.
>
> I don't feel strongly either way, but just wanted to point it out for
> your consideration. Also adding Jose who IIRC is also a junit user.
> [I, btw, am not.]

Thanks, I should have remembered to do that.  In my own tests, I found
the following tests that were warning:

HSW, IVB, SNB, BDW:
 ext_transform_feedback.tessellation quads flat_last
 ext_transform_feedback.tessellation quad_strip flat_last
 ext_transform_feedback.tessellation polygon flat_last

The tests that provided output indicated pixel color accuracy was beyond
the warning threshold.

G45, G965:
 !opengl 1_1.teximage-colors gl_r32f
 !opengl 1_1.teximage-colors gl_rg32f
 !opengl 1_1.teximage-colors gl_rgb32f
 !opengl 1_1.teximage-colors gl_rgba32f

No output was provided for these warnings, so they may have have
previously been dmesg-warn.

>
> On Fri, Jun 26, 2015 at 1:40 PM, Mark Janes  wrote:
>> JUnit has no concept of "warning".  It supports the following
>> statuses:
>>
>>  - skip
>>  - success
>>  - fail
>>  - error
>>
>> dEQP has been found to intermittently emit warnings for passed tests,
>> and this status is accurately represented in piglit json.  However,
>> current JUnit transforms them into failures.
>>
>> A test which emits a warning is more accurately represented as
>> "success" in JUnit.
>> ---
>>  framework/backends/junit.py | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
>> index 632e516..7499829 100644
>> --- a/framework/backends/junit.py
>> +++ b/framework/backends/junit.py
>> @@ -142,8 +142,7 @@ class JUnitBackend(FileBackend):
>>  if data['result'] == 'skip':
>>  res = etree.SubElement(element, 'skipped')
>>
>> -elif data['result'] in ['warn', 'fail', 'dmesg-warn',
>> -'dmesg-fail']:
>> +elif data['result'] in ['fail', 'dmesg-fail']:
>>  if expected_result == "failure":
>>  err.text += "\n\nWARN: passing test as an expected 
>> failure"
>>  res = etree.SubElement(element, 'skipped',
>> --
>> 2.1.4
>>
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] Ignore piglit warning status in JUnit

2015-06-26 Thread Mark Janes
JUnit has no concept of "warning".  It supports the following
statuses:

 - skip
 - success
 - fail
 - error

dEQP has been found to intermittently emit warnings for passed tests,
and this status is accurately represented in piglit json.  However,
current JUnit transforms them into failures.

A test which emits a warning is more accurately represented as
"success" in JUnit.
---
 framework/backends/junit.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/framework/backends/junit.py b/framework/backends/junit.py
index 632e516..7499829 100644
--- a/framework/backends/junit.py
+++ b/framework/backends/junit.py
@@ -142,8 +142,7 @@ class JUnitBackend(FileBackend):
 if data['result'] == 'skip':
 res = etree.SubElement(element, 'skipped')
 
-elif data['result'] in ['warn', 'fail', 'dmesg-warn',
-'dmesg-fail']:
+elif data['result'] in ['fail', 'dmesg-fail']:
 if expected_result == "failure":
 err.text += "\n\nWARN: passing test as an expected failure"
 res = etree.SubElement(element, 'skipped',
-- 
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-26 Thread Mark Janes
Ilia Mirkin  writes:

> On Thu, Jun 25, 2015 at 8:52 PM, Mark Janes  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


[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] framework/test/deqp: handle deqp extra args properly in more cases

2015-06-22 Thread Mark Janes
Reviewed-by: Mark Janes 

Dylan Baker  writes:

> Currently we blindly pass deqp extra args to each test case, and only to
> the test case.
>
> There are two cases where this is problematic:
> 1) if the extra args need to be passed to the deqp command that
>generates the test list (it isn't before this patch)
> 2) if there are any --deqp-case options passed into the Test derived
>classes this will cause the tests to fail (conflicting options)
>
> Both of these are resolved by this patch.
>
> CC: Mark Janes 
> Signed-off-by: Dylan Baker 
> ---
>  framework/test/deqp.py | 11 ---
>  tests/deqp_gles2.py| 12 +++-
>  tests/deqp_gles3.py| 11 +++
>  tests/deqp_gles31.py   | 12 +++-
>  4 files changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/framework/test/deqp.py b/framework/test/deqp.py
> index 843cde8..1462ca2 100644
> --- a/framework/test/deqp.py
> +++ b/framework/test/deqp.py
> @@ -61,8 +61,12 @@ def get_option(env_varname, config_option, default=None):
>  return opt or default
>  
>  
> -def gen_caselist_txt(bin_, caselist):
> -"""Generate a caselist.txt and return its path."""
> +def gen_caselist_txt(bin_, caselist, extra_args):
> +"""Generate a caselist.txt and return its path.
> +
> +Extra args should be a list of extra arguments to pass to deqp.
> +
> +"""
>  # dEQP is stupid (2014-12-07):
>  #   1. To generate the caselist file, dEQP requires that the process's
>  #  current directory must be that same as that of the executable.
> @@ -76,9 +80,10 @@ def gen_caselist_txt(bin_, caselist):
>  #  we must *dynamically* generate it during the testrun.
>  basedir = os.path.dirname(bin_)
>  caselist_path = os.path.join(basedir, caselist)
> +
>  # TODO: need to catch some exceptions here...
>  subprocess.check_call(
> -[bin_, '--deqp-runmode=txt-caselist'], cwd=basedir)
> +[bin_, '--deqp-runmode=txt-caselist'] + extra_args, cwd=basedir)
>  assert os.path.exists(caselist_path)
>  return caselist_path
>  
> diff --git a/tests/deqp_gles2.py b/tests/deqp_gles2.py
> index b97cb5d..c4bacb2 100644
> --- a/tests/deqp_gles2.py
> +++ b/tests/deqp_gles2.py
> @@ -24,20 +24,22 @@ from framework.test import deqp
>  
>  __all__ = ['profile']
>  
> -
>  # Path to the deqp-gles2 executable.
>  _DEQP_GLES2_BIN = deqp.get_option('PIGLIT_DEQP_GLES2_BIN',
>('deqp-gles2', 'bin'))
>  
> +_EXTRA_ARGS = deqp.get_option('PIGLIT_DEQP_GLES2_EXTRA_ARGS',
> +  ('deqp-gles2', 'extra_args'),
> +  default='').split()
> +
>  
>  class DEQPGLES2Test(deqp.DEQPBaseTest):
>  deqp_bin = _DEQP_GLES2_BIN
> -extra_args = deqp.get_option('PIGLIT_DEQP_GLES2_EXTRA_ARGS',
> - ('deqp-gles2', 'extra_args'),
> - default='').split()
> +extra_args = [x for x in _EXTRA_ARGS if not x.startswith('--deqp-case')]
>  
>  
>  profile = deqp.make_profile(  # pylint: disable=invalid-name
>  deqp.iter_deqp_test_cases(
> -deqp.gen_caselist_txt(_DEQP_GLES2_BIN, 'dEQP-GLES2-cases.txt')),
> +deqp.gen_caselist_txt(_DEQP_GLES2_BIN, 'dEQP-GLES2-cases.txt',
> +  _EXTRA_ARGS)),
>  DEQPGLES2Test)
> diff --git a/tests/deqp_gles3.py b/tests/deqp_gles3.py
> index dfb82c9..d353899 100644
> --- a/tests/deqp_gles3.py
> +++ b/tests/deqp_gles3.py
> @@ -35,6 +35,10 @@ _DEQP_GLES3_EXE = deqp.get_option('PIGLIT_DEQP_GLES3_EXE',
>  _DEQP_MUSTPASS = deqp.get_option('PIGLIT_DEQP_MUSTPASS',
>   ('deqp-gles3', 'mustpasslist'))
>  
> +_EXTRA_ARGS = deqp.get_option('PIGLIT_DEQP_GLES3_EXTRA_ARGS',
> +  ('deqp-gles3', 'extra_args'),
> +  default='').split()
> +
>  
>  def _get_test_case(root, root_group, outputfile):
>  """Parser the test case list of Google Android CTS,
> @@ -73,9 +77,7 @@ def filter_mustpass(caselist_path):
>  
>  class DEQPGLES3Test(deqp.DEQPBaseTest):
>  deqp_bin = _DEQP_GLES3_EXE
> -extra_args = deqp.get_option('PIGLIT_DEQP_GLES3_EXTRA_ARGS',
> - ('deqp-gles3', 'extra_args'),
> - defau

Re: [Piglit] [PATCH] glsl-es-3.10: dont run gles tests where restrictions apply

2015-06-17 Thread Mark Janes
Tested-by: Mark Janes 

Timothy Arceri  writes:

> In GLES vertex shader outputs cant be arrays of structs or structs that
> contain arrays or structs.
>
> The GLSL ES 3.0 spec says:
>
> "Vertex output variables output per-vertex data and are declared
> using the out storage qualifier or the centroid out storage qualifier.
> They can only be float, floating-point vectors, matrices, signed or
> unsigned integers or integer vectors, or arrays or structures of any
> these."
>
> The GLSL ES 3.1 is a bit clearer about this:
>
> It is a compile-time error to declare a vertex shader output with,
>  or that contains, any of the following types:
>
> * A boolean type
> * An opaque type
> * An array of arrays
> * An array of structures
> * A structure containing an array
> * A structure containing a structure
>
> CC: Mark Janes 
> ---
>  tests/all.py   |  13 +-
>  .../execution/varying-struct-arrays.shader_test| 237 
> -
>  2 files changed, 9 insertions(+), 241 deletions(-)
>  delete mode 100644 
> tests/spec/glsl-es-3.00/execution/varying-struct-arrays.shader_test
>
> diff --git a/tests/all.py b/tests/all.py
> index 2839059..47fe503 100755
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -3287,10 +3287,15 @@ with profile.group_manager(
>  
>  for api_suffix, possible_options in [('', [[], ['interface']]),
>   ('_gles3', [[]])]:
> -for subtest in ['basic-struct', 'struct-whole-array',
> -'struct-array-elem', 'array-struct',
> -'array-struct-whole-array', 
> 'array-struct-array-elem',
> -'struct-struct', 'array-struct-array-struct']:
> +if api_suffix == '_gles3':
> +subtest_list = ['basic-struct']
> +else:
> +subtest_list = ['basic-struct', 'struct-whole-array',
> +'struct-array-elem', 'array-struct',
> +'array-struct-whole-array',
> +'array-struct-array-elem', 'struct-struct',
> +'array-struct-array-struct']
> +for subtest in subtest_list:
>  for mode in ['error', 'get', 'run', 'run-no-fs']:
>  for options in possible_options:
>  
> g(['ext_transform_feedback-structs{0}'.format(api_suffix),
> diff --git 
> a/tests/spec/glsl-es-3.00/execution/varying-struct-arrays.shader_test 
> b/tests/spec/glsl-es-3.00/execution/varying-struct-arrays.shader_test
> deleted file mode 100644
> index 88b7a8d..000
> --- a/tests/spec/glsl-es-3.00/execution/varying-struct-arrays.shader_test
> +++ /dev/null
> @@ -1,237 +0,0 @@
> -# Test that varying structs work properly in conjunction with arrays.
> -#
> -# From the GLSL ES 3.00 specification, section 4.3.4 ("Input Variables"):
> -#
> -# Fragment inputs can only be signed and unsigned integers and
> -# integer vectors, float, floating-point vectors, matrices, or
> -# arrays or structures of these.
> -#
> -# And from section 4.3.6 ("Output Variables"):
> -#
> -# Vertex output variables ... can only be float, floating-point
> -# vectors, matrices, signed or unsigned integers or integer
> -# vectors, or arrays or structures of any these.
> -#
> -# This test verifies the proper functioning of varyings whose types
> -# are a struct containing an array, an array of structs, and various
> -# complex combinations of arrays and structs.
> -#
> -# Note: chapter 11 of the GLSL ES 3.00 spec ("Counting of Inputs and
> -# Outputs") specifies a packing algorithm which constitutes a minimum
> -# requirement for when a set of varyings must be supported by a
> -# conformant implementation.  Although that chapter has not yet been
> -# updated to reflect varying structs, Khronos's internal bugzilla
> -# indicates that structs should be flattened before applying the
> -# packing algorithm
> -# (https://cvs.khronos.org/bugzilla/show_bug.cgi?id=9828).  The
> -# varyings in this test flatten as follows:
> -#
> -# float s1.f; // A
> -# float[3]  s1.af[];  // B
> -# float[3]  as1[].f;  // C
> -# float[9]  as1[].af[];   // D
> -# float s2.s1.f;  // E
> -# float[3]  s2.s1.af[];   // F
> -# float[2]  s2.as1[].f;   // G
> -# float[6]  s2.as1[].af[];  

Re: [Piglit] [PATCH] dispatch: Fix lookup for core OpenGL symbols

2015-04-30 Thread Mark Janes
Tested-by: Mark Janes 

Chad Versace  writes:

> The following commit regressed lookup of some new OpenGL symbols:
>
> commit cb334db3f2bb7ddf9b2f43641f898896f99c69b1
> Author: Daniel Kurtz 
> Date:   Thu Oct 16 21:36:01 2014 +0800
> Subject: dispatch: Use dlsym to lookup core symbols for EGL (v2)
>
> It regressed these Piglit tests on i965:
> spec@arb_pipeline_statistics_query@arb_pipeline_statistics_query-vert_adj
> spec@arb_pipeline_statistics_query@arb_pipeline_statistics_query-geom
>
> Fix the tests by consistently using egl/glXGetProcAddress to lookup core
> OpenGL symbols instead of dlsym(), and add a lengthy comment explaining
> why. (For core OpenGL ES symbols, continue using dlsym() as in
> cb334db3).
>
> Cc: Marek Olšák 
> Cc: Mark Janes 
> Cc: Daniel Kurtz 
> ---
>  tests/util/piglit-dispatch-init.c | 42 
> +--
>  1 file changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/tests/util/piglit-dispatch-init.c 
> b/tests/util/piglit-dispatch-init.c
> index a47d4cc..ead6d2f 100644
> --- a/tests/util/piglit-dispatch-init.c
> +++ b/tests/util/piglit-dispatch-init.c
> @@ -157,13 +157,9 @@ get_core_proc_address(const char *function_name, int 
> gl_10x_version)
>  #else /* Linux */
>  
>  #if defined(PIGLIT_HAS_EGL)
> -#define GLX_LIB "libGL.so.1"
>  #define GLES1_LIB "libGLESv1_CM.so.1"
>  #define GLES2_LIB "libGLESv2.so.2"
>  
> -/** dlopen() return value for libGL.so.1 */
> -static void *glx_handle;
> -
>  /** dlopen() return value for libGLESv1_CM.so.1 */
>  static void *gles1_handle;
>  
> @@ -213,11 +209,32 @@ get_ext_proc_address(const char *function_name)
>   * This function is used to retrieve the address of core GL functions
>   * on Linux.
>   *
> - * eglGetProcAddress supports querying core functions only if EGL >= 1.5
> - * or if EGL_KHR_get_all_proc_addresses or
> - * EGL_KHR_client_get_all_proc_addresses is supported. Rather than worry
> - * about such details, when using EGL we consistently use dlsym() on the
> - * client library to lookup core functions.
> + * eglGetProcAddress supports querying core functions only if EGL >= 1.5 or 
> if
> + * EGL_KHR_get_all_proc_addresses or EGL_KHR_client_get_all_proc_addresses is
> + * supported. Rather than worry about such details, we consistently use 
> dlysm()
> + * to lookup core *OpenGL ES* functions on systems where EGL is available.
> + *
> + * Lookup for core *OpenGL* functions is more complicated because the EGL 1.4
> + * specification, the antiquated OpenGL ABI for Linux [1] from year 2000, and
> + * various libGL.so implementations all disagree on the set of symbols that
> + * libGL.so should statically expose and which are queryable with
> + * eglGetProcAddress.  The EGL 1.4 spec (as explained above) does not require
> + * eglGetProcAddress to work for core functions.  The OpenGL ABI spec 
> requires
> + * that libGL.so expose *no* symbols statically except those contained in GL
> + * 1.2 and select extensions. Actual driver vendors tend to expose most, if 
> not
> + *   all, symbols statically from libGL.so.
> + *
> + * Considering how messy this situation is, the best way to query a core 
> OpenGL
> + * function on EGL is eglGetProcAddress (or even glXGetProcAddress!). 
> Sometimes
> + * Mesa's libGL doesn't statically expose all OpenGL functions supported by 
> the
> + * driver, but Mesa's eglGetProcAddress does work for all GL functions, core
> + * and extension.  Some other vendors of desktop OpenGL drivers, such as
> + * Nvidia, do the same. (By coincidence, Mesa's glXGetProcAddress also 
> returns
> + * the same addresses as eglGetProcAddress). We don't need to worry about
> + * platforms on which eglGetProcAddress does not work for core functions, 
> such
> + * as Mali, because those platforms support only OpenGL ES.
> + *
> + * [1] https://www.opengl.org/registry/ABI/
>   */
>  static piglit_dispatch_function_ptr
>  get_core_proc_address(const char *function_name, int gl_10x_version)
> @@ -230,8 +247,11 @@ get_core_proc_address(const char *function_name, int 
> gl_10x_version)
>   return do_dlsym(&gles2_handle, GLES2_LIB, function_name);
>   case 10:
>   default:
> - /* GL does not have its own library, so use GLX */
> - return do_dlsym(&glx_handle, GLX_LIB, function_name);
> + /* We query the address of core OpenGL functions as if they
> +  * were extension functions. Read about the gory details
> +  * above. */
> + (void) gl_10x_version;
> + return get_ext_proc_address(function_name);
>   }
>  #else
>   /* We don't need to worry about the GL version, since when using GLX
> -- 
> 2.3.6
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] glsl-deriv-varyings: Migrated to shader_runner and corrected test expectations on deriv + abs cases

2015-04-28 Thread Mark Janes
Tested-by: Mark Janes 

Andres Gomez  writes:

> Old glsl-deriv-varyings test has been migrated to
> shader_runner and, also, splitted in four
> different tests:
> * glsl-derivs-varyings
> * glsl-derivs-sign
> * glsl-derivs-abs
> * glsl-derivs-abs-sign
>
> In addition, expectations for the tests involving
> the abs operator have been corrected.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87407
> Signed-off-by: Andres Gomez 
> ---
>  tests/all.py   |   1 -
>  tests/shaders/CMakeLists.gl.txt|   1 -
>  tests/shaders/glsl-deriv-varyings.c| 375 
> -
>  tests/shaders/glsl-derivs-abs-sign.shader_test |  56 
>  tests/shaders/glsl-derivs-abs.shader_test  |  55 
>  tests/shaders/glsl-derivs-sign.shader_test |  27 ++
>  tests/shaders/glsl-derivs-varyings.shader_test |  27 ++
>  7 files changed, 165 insertions(+), 377 deletions(-)
>  delete mode 100644 tests/shaders/glsl-deriv-varyings.c
>  create mode 100644 tests/shaders/glsl-derivs-abs-sign.shader_test
>  create mode 100644 tests/shaders/glsl-derivs-abs.shader_test
>  create mode 100644 tests/shaders/glsl-derivs-sign.shader_test
>  create mode 100644 tests/shaders/glsl-derivs-varyings.shader_test
>
> diff --git a/tests/all.py b/tests/all.py
> index 18124b7..5ada4d9 100755
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -542,7 +542,6 @@ with profile.group_manager(PiglitGLTest, 'shaders') as g:
>  g(['glsl-vs-statechange-1'])
>  g(['vp-combined-image-units'])
>  g(['glsl-derivs'])
> -g(['glsl-deriv-varyings'])
>  g(['glsl-fwidth'])
>  g(['glsl-lod-bias'])
>  g(['vp-ignore-input'])
> diff --git a/tests/shaders/CMakeLists.gl.txt b/tests/shaders/CMakeLists.gl.txt
> index 3efc6bf..35b4c6c 100644
> --- a/tests/shaders/CMakeLists.gl.txt
> +++ b/tests/shaders/CMakeLists.gl.txt
> @@ -131,7 +131,6 @@ piglit_add_executable (vp-clipdistance-04 
> vp-clipdistance-04.c)
>  piglit_add_executable (vpfp-generic vpfp-generic.cpp)
>  piglit_add_executable (vp-max-array vp-max-array.c)
>  piglit_add_executable (glsl-derivs glsl-derivs.c)
> -piglit_add_executable (glsl-deriv-varyings glsl-deriv-varyings.c)
>  piglit_add_executable (glsl-fs-discard-02 glsl-fs-discard-02.c)
>  piglit_add_executable (glsl-fwidth glsl-fwidth.c)
>  piglit_add_executable (glsl-lod-bias glsl-lod-bias.c)
> diff --git a/tests/shaders/glsl-deriv-varyings.c 
> b/tests/shaders/glsl-deriv-varyings.c
> deleted file mode 100644
> index ca663a5..000
> --- a/tests/shaders/glsl-deriv-varyings.c
> +++ /dev/null
> @@ -1,375 +0,0 @@
> -/*
> - * Copyright © 2009 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.
> - */
> -
> -// author: Ben Holmes
> -
> -/*
> - * This test uses the built-in glsl derivative functions (dFdx and dFdy)
> - * on varying values.
> - */
> -
> -#include "piglit-util-gl.h"
> -
> -static void compileLinkProg(void);
> -static void loadTex(void);
> -
> -PIGLIT_GL_TEST_CONFIG_BEGIN
> -
> - config.supports_gl_compat_version = 10;
> -
> - config.window_width = 600;
> - config.window_height = 300;
> - config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
> -
> -PIGLIT_GL_TEST_CONFIG_END
> -
> -static GLuint tex[1];
> -static GLint prog1;
> -static GLint vs1;
> -static GLint fs1;
> -static GLint prog2;
> -static GLint fs2;
> -static GLint prog3;
> -static GLint fs3;
> -st

Re: [Piglit] [PATCH] junit.py: Escape `api`/`search` in test's classname too.

2015-04-27 Thread Mark Janes
Reviewed-by: Mark Janes 

Jose Fonseca  writes:

> If `api` appears in the middle of the test classname, like it happens
> with all `spec/glsl-1.20/api/getactiveattrib *` tests, then Jenkins will
> intercept it making it impossible to see individual tests results.
> Therefore, just like the testname, it must be escaped.
>
> This escaping will affect the expected failure/crash test matching.  I
> can't think of another solution though...
> ---
>  framework/backends/junit.py | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
> index 3602f9e..c6219e2 100644
> --- a/framework/backends/junit.py
> +++ b/framework/backends/junit.py
> @@ -40,6 +40,15 @@ __all__ = [
>  ]
>  
>  
> +junitSpecialNames = ('api', 'search')
> +
> +def junitEscape(name):
> +name = name.replace('.', '_')
> +if name in junitSpecialNames:
> +name += '_'
> +return name
> +
> +
>  class JUnitBackend(FileBackend):
>  """ Backend that produces ANT JUnit XML
>  
> @@ -161,8 +170,9 @@ class JUnitBackend(FileBackend):
>  # classname), and replace piglits '/' separated groups with '.', 
> after
>  # replacing any '.' with '_' (so we don't get false groups).
>  classname, testname = grouptools.splitname(name)
> -classname = classname.replace('.', '_')
> -classname = classname.replace(grouptools.SEPARATOR, '.')
> +classname = classname.split(grouptools.SEPARATOR)
> +classname = map(junitEscape, classname)
> +classname = '.'.join(classname)
>  
>  # Add the test to the piglit group rather than directly to the root
>  # group, this allows piglit junit to be used in conjunction with 
> other
> @@ -177,7 +187,7 @@ class JUnitBackend(FileBackend):
>  # The testname variable is used in the calculate_result
>  # closure, and must not have the suffix appended.
>  full_test_name = testname + self._test_suffix
> -if full_test_name in ('api', 'search'):
> +if full_test_name in junitSpecialNames:
>  testname += '_'
>  full_test_name = testname + self._test_suffix
>  
> -- 
> 2.1.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 13/14] dispatch: Use dlsym to lookup core symbols for EGL

2015-04-27 Thread Mark Janes
Adding the list.

Mark Janes  writes:

> Hi Chad,
>
> This patch was recently pushed, and it caused piglit failures for
> arb_pipeline_statistics_query:
>
> 
> Standard Output
>
> /tmp/build_root/m32/lib/piglit/bin/arb_pipeline_statistics_query-vert_adj 
> -auto -fbo
> GetProcAddress failed for "glGetQueryObjectui64v"
>
> Standard Error
>
> glGetQueryObjectui64v() not found in libGL.so.1: 
> /tmp/build_root/m32/lib/libGL.so.1: undefined symbol: glGetQueryObjectui64v
> 
>
> Is this a bug in mesa?
>
> -Mark
>
> Chad Versace  writes:
>
>> On Mon 13 Apr 2015, Marek Olšák wrote:
>>> From: Daniel Kurtz 
>>>
>>> eglGetProcAddress() only supports extension functions.
>>> Therefore, we must use dlsym() directly on the GL client library to look
>>> up core functions.
>>>
>>> The implementation here is a much simplified version of the one in
>>> libepoxy.  Most of the simplification is because piglit dispatch already
>>> knows exactly for which GL API it is looking up a symbol.
>>>
>>> Signed-off-by: Daniel Kurtz 
>>> ---
>>>  tests/util/piglit-dispatch-init.c | 57 
>>> +--
>>>  1 file changed, 55 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/util/piglit-dispatch-init.c 
>>> b/tests/util/piglit-dispatch-init.c
>>> index cc8b684..b0e1649 100644
>>> --- a/tests/util/piglit-dispatch-init.c
>>> +++ b/tests/util/piglit-dispatch-init.c
>>> @@ -33,6 +33,8 @@
>>>
>>>  #else /* Linux */
>>>
>>> +#include 
>>> +
>>>  #if defined(PIGLIT_HAS_GLX)
>>>  #  include "glxew.h"
>>>  #elif defined(PIGLIT_HAS_EGL)
>>> @@ -154,6 +156,42 @@ get_core_proc_address(const char *function_name, int 
>>> gl_10x_version)
>>>
>>>  #else /* Linux */
>>>
>>> +#if defined(PIGLIT_HAS_EGL)
>>> +#define GLX_LIB "libGL.so.1"
>>> +#define GLES1_LIB "libGLESv1_CM.so.1"
>>> +#define GLES2_LIB "libGLESv2.so.2"
>>> +
>>> +/** dlopen() return value for libGL.so.1 */
>>> +static void *glx_handle;
>>> +
>>> +/** dlopen() return value for libGLESv1_CM.so.1 */
>>> +static void *gles1_handle;
>>> +
>>> +/** dlopen() return value for libGLESv2.so.2 */
>>> +static void *gles2_handle;
>>> +
>>> +static void *
>>> +do_dlsym(void **handle, const char *lib_name, const char *function_name)
>>> +{
>>> +void *result;
>>> +
>>> +if (!*handle)
>>> +*handle = dlopen(lib_name, RTLD_LAZY | RTLD_LOCAL);
>>> +
>>> +if (!*handle) {
>>> +fprintf(stderr, "Could not open %s: %s\n", lib_name, dlerror());
>>> +return NULL;
>>> +}
>>> +
>>> +result = dlsym(*handle, function_name);
>>> +if (!result)
>>> +fprintf(stderr, "%s() not found in %s: %s\n", function_name, 
>>> lib_name,
>>> +dlerror());
>>> +
>>> +return result;
>>> +}
>>> +#endif
>>> +
>>>  /**
>>>   * This function is used to retrieve the address of all GL functions
>>>   * on Linux.
>>
>> Everyting up to here looks good.
>>
>>> @@ -174,16 +212,31 @@ get_ext_proc_address(const char *function_name)
>>>  /**
>>>   * This function is used to retrieve the address of core GL functions
>>>   * on Linux.
>>> + *
>>> + * Since eglGetProcAddress() only supports extension functions, we must use
>>> + * dlsym() directly on the GL client library to lookup core functions.
>>
>> This comment isn't true. Sometimes eglGetProcAddress does supports core
>> functions. It should instead say:
>>
>>   """
>>   eglGetProcAddress supports querying core functions only if EGL >= 1.5
>>   or if EGL_KHR_get_all_proc_addresses or
>>   EGL_KHR_client_get_all_proc_addresses is supported. Rather than worry
>>   about such details, when using EGL we consistently use dlsym() on the
>>   client library to lookup core functions.
>>   """
>>
>> This works for libGLESv1_CM.so and libGLESv2.so. But libGL.so wasn't designed
>> with EGL in mind.  The antiquated OpenGL ABI for Linux [1] states:
>>
>>   3.4. [The OpenGL library] must export all OpenGL 1.2, [...] GLX 1.3, and
>

Re: [Piglit] Add incomplete file for tests as they start

2015-04-15 Thread Mark Janes
Series is

Reviewed-by: Mark Janes 

Dylan Baker  writes:

> One of the more interesting problems for piglit, especially with
> hardware bring-up where bad code may result in the system crashing, is
> to know what test actually crashed the system.
>
> This series is meant to address that problem. It does so by adding a new
> status, the incomplete status, which is used by the backend to write out
> a file for a test before the test starts running. When the test
> completes this status is overwritten with the final result. If the test
> is never overwritten (say the system crashes) it remains incomplete.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] glsl-1.50: check read only interfaces are read only

2015-03-27 Thread Mark Janes
Reviewed-by: Mark Janes 

Timothy Arceri  writes:

> Test results:
>
> AMD Radeon HD 6670 - Catalyst 14.501.1003 OpenGL 4.4
>
> interface-block-instance-name-input-read-only.frag - pass
> interface-block-input-read-only.frag - pass
> interface-block-instance-name-uniform-read-only.frag - fail
> interface-block-uniform-read-only.frag - fail
> ---
>  .../compiler/interface-block-input-read-only.frag| 20 
> 
>  ...nterface-block-instance-name-input-read-only.frag | 20 
> 
>  ...erface-block-instance-name-uniform-read-only.frag | 20 
> 
>  .../compiler/interface-block-uniform-read-only.frag  | 20 
> 
>  4 files changed, 80 insertions(+)
>  create mode 100644 
> tests/spec/glsl-1.50/compiler/interface-block-input-read-only.frag
>  create mode 100644 
> tests/spec/glsl-1.50/compiler/interface-block-instance-name-input-read-only.frag
>  create mode 100644 
> tests/spec/glsl-1.50/compiler/interface-block-instance-name-uniform-read-only.frag
>  create mode 100644 
> tests/spec/glsl-1.50/compiler/interface-block-uniform-read-only.frag
>
> diff --git 
> a/tests/spec/glsl-1.50/compiler/interface-block-input-read-only.frag 
> b/tests/spec/glsl-1.50/compiler/interface-block-input-read-only.frag
> new file mode 100644
> index 000..304aacf
> --- /dev/null
> +++ b/tests/spec/glsl-1.50/compiler/interface-block-input-read-only.frag
> @@ -0,0 +1,20 @@
> +// [config]
> +// expect_result: fail
> +// glsl_version: 1.50
> +// check_link: false
> +// [end config]
> +//
> +// Check that an error is generated when trying
> +// to change the value of a shader input.
> +
> +#version 150
> +
> +in Block {
> +  vec4 invar;
> +};
> +
> +void main()
> +{
> +  invar = vec4(1.0);
> +  gl_FragColor = invar;
> +}
> diff --git 
> a/tests/spec/glsl-1.50/compiler/interface-block-instance-name-input-read-only.frag
>  
> b/tests/spec/glsl-1.50/compiler/interface-block-instance-name-input-read-only.frag
> new file mode 100644
> index 000..d001db8
> --- /dev/null
> +++ 
> b/tests/spec/glsl-1.50/compiler/interface-block-instance-name-input-read-only.frag
> @@ -0,0 +1,20 @@
> +// [config]
> +// expect_result: fail
> +// glsl_version: 1.50
> +// check_link: false
> +// [end config]
> +//
> +// Check that an error is generated when trying
> +// to change the value of a shader input.
> +
> +#version 150
> +
> +in Block {
> +  vec4 invar;
> +} a;
> +
> +void main()
> +{
> +  a.invar = vec4(1.0);
> +  gl_FragColor = a.invar;
> +}
> diff --git 
> a/tests/spec/glsl-1.50/compiler/interface-block-instance-name-uniform-read-only.frag
>  
> b/tests/spec/glsl-1.50/compiler/interface-block-instance-name-uniform-read-only.frag
> new file mode 100644
> index 000..43909ba
> --- /dev/null
> +++ 
> b/tests/spec/glsl-1.50/compiler/interface-block-instance-name-uniform-read-only.frag
> @@ -0,0 +1,20 @@
> +// [config]
> +// expect_result: fail
> +// glsl_version: 1.50
> +// check_link: false
> +// [end config]
> +//
> +// Check that an error is generated when trying
> +// to change the value of a uniform.
> +
> +#version 150
> +
> +uniform Block {
> +  vec4 invar;
> +} a;
> +
> +void main()
> +{
> +  a.invar = vec4(1.0);
> +  gl_FragColor = a.invar;
> +}
> diff --git 
> a/tests/spec/glsl-1.50/compiler/interface-block-uniform-read-only.frag 
> b/tests/spec/glsl-1.50/compiler/interface-block-uniform-read-only.frag
> new file mode 100644
> index 000..aaea6b3
> --- /dev/null
> +++ b/tests/spec/glsl-1.50/compiler/interface-block-uniform-read-only.frag
> @@ -0,0 +1,20 @@
> +// [config]
> +// expect_result: fail
> +// glsl_version: 1.50
> +// check_link: false
> +// [end config]
> +//
> +// Check that an error is generated when trying
> +// to change the value of a uniform.
> +
> +#version 150
> +
> +uniform Block {
> +  vec4 invar;
> +};
> +
> +void main()
> +{
> +  invar = vec4(1.0);
> +  gl_FragColor = invar;
> +}
> -- 
> 2.1.0
>
> ___
> 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] tests/all.py: Fix typo that switched arb with ext

2015-03-05 Thread Mark Janes
Reviewed-by: Mark Janes 

Dylan Baker  writes:

> This fixes several tests on platforms that support
> arb_transform_feedback3.
>
> Signed-off-by: Dylan Baker 
> ---
>  tests/all.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/all.py b/tests/all.py
> index dc73c77..42fc826 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -3316,7 +3316,7 @@ with profile.group_manager(
>'draw-auto instanced', run_concurrent=False)
>  
>  with profile.group_manager(
> -PiglitGLTest, grouptools.join('spec', 'ext_transform_feedback3')) as 
> g:
> +PiglitGLTest, grouptools.join('spec', 'arb_transform_feedback3')) as 
> g:
>  g(['arb_transform_feedback3-bind_buffer_invalid_index'],
>'arb_transform_feedback3-bind_buffer_invalid_index', 
> run_concurrent=False)
>  g(['arb_transform_feedback3-query_with_invalid_index'],
> -- 
> 2.3.1
>
> ___
> 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 1/2] varying-packing: Use logical-OR instead of if statement to set failed.

2015-03-04 Thread Mark Janes
series Reviewed-by: Mark Janes 

Matt Turner  writes:

> By flattening the if statements we cut 288 instructions from the
> fragment shader (806 -> 518, or 35%), and we remove all register
> spilling on i965.
>
> Reduces runtime of "varying-packing-simple int separate" by
> -81.4453% +/- 0.255875% (n=483).
>
> Also consistently print 3 digits in the varying's names, to make
> reading/sorting simpler.
> ---
>  tests/spec/glsl-1.10/execution/varying-packing/simple.c | 17 
> +
>  1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/tests/spec/glsl-1.10/execution/varying-packing/simple.c 
> b/tests/spec/glsl-1.10/execution/varying-packing/simple.c
> index 618f325..e9935c7 100644
> --- a/tests/spec/glsl-1.10/execution/varying-packing/simple.c
> +++ b/tests/spec/glsl-1.10/execution/varying-packing/simple.c
> @@ -258,12 +258,12 @@ get_shader(bool is_vs, unsigned glsl_version, int 
> num_varyings,
>   if (varyings[i].type->base != BASE_TYPE_FLOAT)
>   opt_flat_keyword = "flat ";
>   if (varyings[i].array_elems != 0) {
> - text += sprintf(text, "%s%s %s var%u[%u];\n",
> + text += sprintf(text, "%s%s %s var%03u[%u];\n",
>   opt_flat_keyword, varying_keyword,
>   varyings[i].type->name, i,
>   varyings[i].array_elems);
>   } else {
> - text += sprintf(text, "%s%s %s var%u;\n",
> + text += sprintf(text, "%s%s %s var%03u;\n",
>   opt_flat_keyword, varying_keyword,
>   varyings[i].type->name, i);
>   }
> @@ -292,8 +292,8 @@ get_shader(bool is_vs, unsigned glsl_version, int 
> num_varyings,
>   for (l = 0; l < varyings[i].type->num_rows; 
> ++l) {
>   text += sprintf(text, "  ");
>   if (!is_vs)
> - text += sprintf(text, "if (");
> - text += sprintf(text, "var%u", i);
> + text += sprintf(text, "failed = 
> failed || ");
> + text += sprintf(text, "var%03u", i);
>   if (varyings[i].array_elems)
>   text += sprintf(text, "[%u]", 
> j);
>   if (varyings[i].type->num_cols > 1)
> @@ -304,16 +304,9 @@ get_shader(bool is_vs, unsigned glsl_version, int 
> num_varyings,
>   text += sprintf(text, " = ");
>   else
>   text += sprintf(text, " != ");
> - text += sprintf(text, "%s(i + %u)",
> + text += sprintf(text, "%s(i + %u);\n",
>   base_type_name,
>   offset++);
> - if (is_vs) {
> - text += sprintf(text, ";\n");
> - } else {
> - text += sprintf(text,
> - ")\n"
> - "failed = 
> true;\n");
> - }
>   }
>   }
>   }
> -- 
> 2.0.5
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] framework/backends/junit: Report expected failures/crashes as skipped.

2015-03-03 Thread Mark Janes
Jose Fonseca  writes:

> Thanks for the reviews.
>
>
> My setup is not complicated as yours.  Mostly because my main focus has 
> been llvmpipe, and we're not actively adding supporting to new OpenGL 
> extensions for it at this moment, so most of the new piglit tests either 
> skip or pass. New failures are relatively rare.

One of the lessons I learned with Jenkins is that if the automation is
complex, it should be in git and not in Jenkins projects.  Our Jenkins
jobs typically just execute a single script with a constrained set of
parameters.

This facilitates testing/debugging without invoking builds on the
Jenkins instance.  It also is much easier to handle development branches
by branching your scripts, as opposed to cloning Jenkins projects.

We orchestrate multi-platform jobs with python via Jenkins Remote Access
API.  Binaries and test results are communicated between builds via a
shared drive.

> Before I used 
> https://wiki.jenkins-ci.org/display/JENKINS/Email-ext+plugin -- it 
> allows to control if/when emails are sent with great flexibility. In 
> particular it allows to send emails for new regressions, but not for 
> tests that were failing previously.
>
> Still, nothing beats being able to look at a bunch of test jobs and 
> immediately tell all blue == all good.   Alhough I use jenkins for many 
> years now, this is a lesson I only learned recently -- it's better to 
> mask out expected failures somehow and get a boolean "all pass" or 
> "fail" for the whole test stuie, than trying to track pass/fail for 
> individual tests.  The latter just doesn't scale...
>
>
>
> We do have an internal branches where we run piglit.  I confess I don't 
> a good solution yet.  Your trick of maintaining a database of which git 
> commit tests were added is quite neat.  Another thing worth considering 
> would be to branch or tag/ piglit whenever Mesa is branched, and keep 
> using a matching (and unchanging) piglit commit.

Tagging piglit is a simpler solution than what I've done.  The primary
use case, though, is for developers to test their branches before they
send them to the mailing list.  Without a recent rebase, their branches
will typically report failures on tests which were fixed on master.

> We also run testsuites through different APIs (namely D3D9/10).  These 
> testsuite rarely get updated, and llvmpipe conformance is actually quite 
> good to start with, so it's easy to get "all pass" there.
>
>
> Piglit, by being continuously updated/extended, is indeed more of a 
> challenge than other testsuites.
>
>
> We also use piglit for testing our OpenGL guest driver, but we use an 
> internal testing infrastructure to driver, not Jenkins.  So our 
> experiences there don't apply.
>
>
> I also have a few benchmarks on jenkins.  Again, I only keep track of 
> performance metrics via Jenkins Plots and Measurements plugins, but I 
> don't produce pass/fail based on those metrics.  I am however 
> considering doing something of the sort -- e.g., getting the history of 
> the metrics via jenkins JSON API, fit into a probablylity distribution, 
> and fail when performance goes below a given percentile.

I'm curious to know which benchmarks you use.  We experience variability
in a lot of benchmarks, especially if they are cpu-heavy and running on
an under-powered system.  It would be great to have workloads that
produce reliable trends without having to run them repeatedly.

thanks for the details!

-Mark

> Jose
>
>
> On 03/03/15 18:34, Mark Janes wrote:
>> Thanks Jose! this is an improvement.
>>
>> In my experience, broken tests are introduced and fixed in mesa on a
>> daily basis.  This has a few consequences:
>>
>>   - On a daily basis, I look at failures and update the expected
>> pass/fails depending on whether it is a new test or a regression.
>> Much of this process is automated.
>>
>>   - Branches quickly diverge on the basis of passing/failing tests.
>> Having separate pass/fail configs on release branches is
>> unmanageable.  To account for this, my automation records the
>> relevant commit sha as the value in the config file (the key is the
>> test name).  I post-process the junit xml to filter out test failures
>> with commits that occurred after the branch point.
>>
>>   - for platforms that are too slow to build each checkin, I run an
>> automated bisect which builds/tests in jenkins, then updates config
>> files.
>>
>>   - Our platform matrix generates over 350k unskipped tests for each
>> build.  We filter out skipped tests due to the memory consumption on
>> jenkins when displaying

Re: [Piglit] [PATCH] framework/backends/junit: Report expected failures/crashes as skipped.

2015-03-03 Thread Mark Janes
Thanks Jose! this is an improvement.

In my experience, broken tests are introduced and fixed in mesa on a
daily basis.  This has a few consequences:

 - On a daily basis, I look at failures and update the expected
   pass/fails depending on whether it is a new test or a regression.
   Much of this process is automated.

 - Branches quickly diverge on the basis of passing/failing tests.
   Having separate pass/fail configs on release branches is
   unmanageable.  To account for this, my automation records the
   relevant commit sha as the value in the config file (the key is the
   test name).  I post-process the junit xml to filter out test failures
   with commits that occurred after the branch point.

 - for platforms that are too slow to build each checkin, I run an
   automated bisect which builds/tests in jenkins, then updates config
   files.

 - Our platform matrix generates over 350k unskipped tests for each
   build.  We filter out skipped tests due to the memory consumption on
   jenkins when displaying this many tests.

I am interested in learning more about your test system, and sharing
lessons learned / techniques.

-Mark

Reviewed-by: Mark Janes 

Jose Fonseca  writes:

> I recently tried the junit backend's ability to ignore expected
> failures/crashes and found it a godsend -- instead of having to look as
> test graph results periodically, I can just tell jenkins to email me
> when things go south.
>
> The only drawback is that by reporting the expected issues as passing it
> makes it too easy to forget about them and misinterpret the pass-rates.
> So this change modifies the junit backend to report the expected issues
> as skipped, making it more obvious when looking at the test graphs that
> these tests are not really passing, and that whatever functionality they
> target is not being fully covered.
>
> This change also makes use of the junit `message` attribute to explain
> the reason of the skip.  (In fact, we could consider using the `message`
> attribute on other kind of failures to inform the piglit result, instead
> of using the non-standard `type`.)
> ---
>  framework/backends/junit.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
> index 82f9c29..53b6086 100644
> --- a/framework/backends/junit.py
> +++ b/framework/backends/junit.py
> @@ -129,17 +129,19 @@ class JUnitBackend(FileBackend):
>  # Add relevant result value, if the result is pass then it 
> doesn't
>  # need one of these statuses
>  if data['result'] == 'skip':
> -etree.SubElement(element, 'skipped')
> +res = etree.SubElement(element, 'skipped')
>  
>  elif data['result'] in ['warn', 'fail', 'dmesg-warn', 
> 'dmesg-fail']:
>  if expected_result == "failure":
>  err.text += "\n\nWARN: passing test as an expected 
> failure"
> +res = etree.SubElement(element, 'skipped', 
> message='expected failure')
>  else:
>  res = etree.SubElement(element, 'failure')
>  
>  elif data['result'] == 'crash':
>  if expected_result == "error":
>  err.text += "\n\nWARN: passing test as an expected crash"
> +res = etree.SubElement(element, 'skipped', 
> message='expected crash')
>  else:
>  res = etree.SubElement(element, 'error')
>  
> -- 
> 2.1.0
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/2] glsl-1.50: check array sizes when instance name differ

2015-03-02 Thread Mark Janes
series is

Reviewed-by: Mark Janes 

Timothy Arceri  writes:

> Uniform interface blocks can have different instance names across shaders. 
> These tests check that array rules are applied correctly.
>
> Test results:
>
> Intel Ivybridge - Mesa 10.6
>
> uniform-block-array-instance-name-mismatch - pass
> uniform-block-array-size-and-instance-name-mismatch - pass
>
> AMD Radeon HD 6670 - Catalyst 14.501.1003 OpenGL 4.4
>
> uniform-block-array-instance-name-mismatch - pass
> uniform-block-array-size-and-instance-name-mismatch - fail
> ---
>  ...-block-array-instance-name-mismatch.shader_test | 28 +
>  ...ray-size-and-instance-name-mismatch.shader_test | 36 
> ++
>  2 files changed, 72 insertions(+)
>  create mode 100644 
> tests/spec/glsl-1.50/linker/uniform-block-array-instance-name-mismatch.shader_test
>  create mode 100644 
> tests/spec/glsl-1.50/linker/uniform-block-array-size-and-instance-name-mismatch.shader_test
>
> diff --git 
> a/tests/spec/glsl-1.50/linker/uniform-block-array-instance-name-mismatch.shader_test
>  
> b/tests/spec/glsl-1.50/linker/uniform-block-array-instance-name-mismatch.shader_test
> new file mode 100644
> index 000..b7066a4
> --- /dev/null
> +++ 
> b/tests/spec/glsl-1.50/linker/uniform-block-array-instance-name-mismatch.shader_test
> @@ -0,0 +1,28 @@
> +// Instance names for uniforms can differ. Check
> +// that the shaders link successfully.
> +
> +[require]
> +GLSL >= 1.50
> +
> +[vertex shader]
> +uniform Foo {
> +  vec4 x;
> +} foo[3];
> +
> +void main()
> +{
> +  gl_Position = vec4(foo[0].x);
> +}
> +
> +[fragment shader]
> +uniform Foo {
> +  vec4 x;
> +} bar[3];
> +
> +void main()
> +{
> +  gl_FragColor = bar[0].x;
> +}
> +
> +[test]
> +link success
> diff --git 
> a/tests/spec/glsl-1.50/linker/uniform-block-array-size-and-instance-name-mismatch.shader_test
>  
> b/tests/spec/glsl-1.50/linker/uniform-block-array-size-and-instance-name-mismatch.shader_test
> new file mode 100644
> index 000..f2a7c70
> --- /dev/null
> +++ 
> b/tests/spec/glsl-1.50/linker/uniform-block-array-size-and-instance-name-mismatch.shader_test
> @@ -0,0 +1,36 @@
> +// From the GLSL 1.50 spec, section 4.3.7 (Interface Blocks):
> +//
> +// Furthermore, if a matching block is declared as an array, then
> +// the array sizes must also match (or follow array matching rules
> +// for the interface between a vertex and a geometry shader).
> +//
> +// In this test, we create a uniform block array in both
> +// the vertex and fragment shaders, using different array sizes. The
> +// instance name of the interface differs across shaders. Then we
> +// check that the implementation correctly reported an error.
> +
> +[require]
> +GLSL >= 1.50
> +
> +[vertex shader]
> +uniform Foo {
> +  vec4 x;
> +} foo[3];
> +
> +void main()
> +{
> +  gl_Position = vec4(foo[0].x);
> +}
> +
> +[fragment shader]
> +uniform Foo {
> +  vec4 x;
> +} bar[2];
> +
> +void main()
> +{
> +  gl_FragColor = bar[0].x;
> +}
> +
> +[test]
> +link error
> -- 
> 2.1.0
>
> ___
> 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 1/2] profile_tests.py: Convert generator test into a class test

2015-02-24 Thread Mark Janes
Dylan Baker  writes:

> I've been resistant to use classes for tests because its very easy to
> end up bending and abusing classes in that context, but sometimes a
> class is exactly what you want, a structure that shares its data between
> several unique tests.
>
> This replaces a generator that does the same thing with a class. It ends
> up being less code and I think that the resulting code is more readable.
>
> Signed-off-by: Dylan Baker 
> ---
>  framework/tests/profile_tests.py | 117 
> +--
>  1 file changed, 51 insertions(+), 66 deletions(-)
>
> diff --git a/framework/tests/profile_tests.py 
> b/framework/tests/profile_tests.py
> index 183f526..1f94b19 100644
> --- a/framework/tests/profile_tests.py
> +++ b/framework/tests/profile_tests.py
> @@ -21,6 +21,7 @@
>  """ Provides test for the framework.profile modules """
>  
>  from __future__ import print_function, absolute_import
> +import sys
>  import copy
>  import platform
>  
> @@ -32,6 +33,9 @@ import framework.dmesg as dmesg
>  import framework.profile as profile
>  from framework.tests import utils
>  
> +# Don't print sys.stderr to the console
> +sys.stderr = sys.stdout
> +
>  
>  def test_initialize_testprofile():
>  """ TestProfile initializes """
> @@ -194,87 +198,68 @@ def check_mixed_flatten(tests, testlist):
>  nt.assert_dict_equal(profile_.test_list, baseline)
>  
>  
> -def generate_prepare_test_list_test_test_matches():
> -""" Generate tests for TestProfile.perpare_test_list filtering """
typo: ^^^

Other than that, the series is:
Reviewed-by: Mark Janes 

> -data = {'group1/test1': 'thingy', 'group1/group3/test2': 'thing',
> -'group3/test5': 'other'}
> -
> -test_matches_filter_mar_1.description = (
> -"TestProfile.prepare_test_list: "
> -"'not env.filter or matches_any_regex() env.filter is False")
> -yield test_matches_filter_mar_1, data
> -
> -test_matches_filter_mar_2.description = (
> -"TestProfile.prepare_test_list: "
> -"Tests 'not env.filter or matches_any_regex() mar is False")
> -yield test_matches_filter_mar_2, data
> -
> -test_matches_env_exclude.description = (
> -"TestProfile.prepare_test_list: "
> -"Tests 'not path in env.exclude_tests' is True")
> -yield test_matches_env_exclude, data
> -
> -test_matches_exclude_mar.description = \
> -"TestProfile.prepare_test_list: Tests 'not matches_any_regex"
> -yield test_matches_exclude_mar, data
> +class TestPrepareTestListMatches(object):
> +"""Create tests for TestProfile.perpare_test_list filtering"""
> +def __init__(self):
> +self.data = {
> +'group1/test1': 'thingy',
> +'group1/group3/test2': 'thing',
> +'group3/test5': 'other'
> +}
>  
> +def test_matches_filter_mar_1(self):
> +"""TestProfile.prepare_test_list: 'not env.filter or
> +matches_any_regex() env.filter is False
>  
> -@nt.nottest
> -def test_matches_filter_mar_1(data):
> -""" Tests 'not env.filter or matches_any_regex() env.filter is False
> -
> -Nothing should be filtered.
> -
> -"""
> -env = core.Options()
> -
> -profile_ = profile.TestProfile()
> -profile_.test_list = data
> -profile_._prepare_test_list(env)
> +Nothing should be filtered.
>  
> -nt.assert_dict_equal(profile_.test_list, data)
> +"""
> +env = core.Options()
>  
> +profile_ = profile.TestProfile()
> +profile_.test_list = self.data
> +profile_._prepare_test_list(env)
>  
> -@nt.nottest
> -def test_matches_filter_mar_2(data):
> -""" Tests 'not env.filter or matches_any_regex() mar is False"""
> -env = core.Options(include_filter=['test5'])
> +nt.assert_dict_equal(profile_.test_list, self.data)
>  
> -profile_ = profile.TestProfile()
> -profile_.test_list = data
> -profile_._prepare_test_list(env)
> +def test_matches_filter_mar_2(self):
> +"""TestProfile.prepare_test_list: 'not env.filter or 
> matches_any_regex()
> +mar is False
>  
> -baseline = 

[Piglit] [PATCH] junit.py: Fix handling of special test names.

2015-02-20 Thread Mark Janes
In junit.py, the testname variable is used by a closure within the
write_test() scope.  Modifying it breaks the filtering of expected
failures.
---
 framework/backends/junit.py | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/framework/backends/junit.py b/framework/backends/junit.py
index ddaf826..d4b5041 100644
--- a/framework/backends/junit.py
+++ b/framework/backends/junit.py
@@ -166,16 +166,17 @@ class JUnitBackend(FileBackend):
 # set different root names.
 classname = 'piglit.' + classname
 
-testname += self._test_suffix
-
 # Jenkins will display special pages when the test has certain names.
 # https://jenkins-ci.org/issue/18062
 # https://jenkins-ci.org/issue/19810
-if testname in ('api', 'search'):
+# If there is a suffix, then the test link will not be one of
+# the reserved names.  Don't modify testname, as it is used in
+# the calculate_result closure.
+if not self._test_suffix and testname in ('api', 'search'):
 testname += '_'
 
 # Create the root element
-element = etree.Element('testcase', name=testname,
+element = etree.Element('testcase', name=testname + self._test_suffix,
 classname=classname,
 time=str(data['time']),
 status=str(data['result']))
-- 
2.1.4

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


Re: [Piglit] [PATCH] junit.py: Avoid special test names.

2015-02-20 Thread Mark Janes
Jose Fonseca  writes:

> On 20/02/15 20:50, Mark Janes wrote:
>> We support using the test names as displayed in jenkins to start bisect
>> jobs.  If a test fails, you can copy/paste the name into a job, and it
>> will find the commit that broke it.
>>
>> With this change, special-cased test names can't be used with
>> --include-tests anymore.
>>
>
> Couldn't the bisect job easily trim trailing underscores to the test 
> name that is copy'n'pasted?

Yes, I could do that.  As it turns out, I won't need to.  Because we
use suffixes on all our tests, we will never have a test named "api"

The underscore will interfere with matching test names for
expected-failures in a config file.  Again, it won't matter for us, and
AFAIK we are the only ones using that feature.

>  > Instead of adding an underscore, can we change one of the characters to
>  > ".", so that the expression matching will still work?
>
> '.' is special for Jenkins -- it's equivalent to / in a path.  So I 
> suspect springing dots in the name will cause problems.

We currently append ".{hardware_suffix}" to each test name, so we can
run the same tests dozens of times on different platforms and display
all the results together.  The '.' is handled without error.

Since it won't matter to us, and it is such a tiny corner case for
everyone else, I'm fine with it.  

Reviewed-by: Mark Janes 

> Another solution is to would be to dump the pristine name (or even 
> better, a properly quoted regex suitable to be passed in 
> -include-tests  ) in the junit test result (stdout or stderr). e.g.:
>
>"
>
>To run only this test pass --include-tests '^foo/doo/boo$'"
>
>
> Jose
>
>
>>
>> -Mark
>>
>> Dylan Baker  writes:
>>
>>> This looks fine to me, though I haven't tested it.
>>>
>>> Mark, does this seem good to you?
>>>
>>> Reviewed-by: Dylan Baker 
>>>
>>> On Fri, Feb 20, 2015 at 11:53:50AM +, Jose Fonseca wrote:
>>>> For example, Jenkins was displaying its REST API page when one navigated
>>>> to  .../testReport/piglit.spec/ARB_occlusion_query2/api/ result.
>>>> ---
>>>>   framework/backends/junit.py | 10 +-
>>>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
>>>> index 839c99a..ddaf826 100644
>>>> --- a/framework/backends/junit.py
>>>> +++ b/framework/backends/junit.py
>>>> @@ -166,8 +166,16 @@ class JUnitBackend(FileBackend):
>>>>   # set different root names.
>>>>   classname = 'piglit.' + classname
>>>>
>>>> +testname += self._test_suffix
>>>> +
>>>> +# Jenkins will display special pages when the test has certain 
>>>> names.
>>>> +# 
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__jenkins-2Dci.org_issue_18062&d=AwIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=6iqiAc1uzBPZFCypdivM6WaoDheu37Q45KJTMIFulgo&s=3OKSPXH947_Gwt5zZ4v0DIJRLWf5-u6P4a2yc3cY1AU&e=
>>>> +# 
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__jenkins-2Dci.org_issue_19810&d=AwIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=6iqiAc1uzBPZFCypdivM6WaoDheu37Q45KJTMIFulgo&s=Kr5YcnwFlFnWhiOHklE4CT5QTR7A5PiMJFoNuK4dhaM&e=
>>>> +if testname in ('api', 'search'):
>>>> +testname += '_'
>>>> +
>>>>   # Create the root element
>>>> -element = etree.Element('testcase', name=testname + 
>>>> self._test_suffix,
>>>> +element = etree.Element('testcase', name=testname,
>>>>   classname=classname,
>>>>   time=str(data['time']),
>>>>   status=str(data['result']))
>>>> --
>>>> 2.1.0
>>>>
>>>> ___
>>>> Piglit mailing list
>>>> Piglit@lists.freedesktop.org
>>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_piglit&d=AwIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=zfmBZnnVGHeYde45pMKNnVyzeaZbdIqVLprmZCM2zzE&m=6iqiAc1uzBPZFCypdivM6WaoDheu37Q45KJTMIFulgo&s=KdZJAJSf6EP4bKM8xp2zYCOpqQ8WY14LPCxx90LhrdY&e=
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] junit.py: Avoid special test names.

2015-02-20 Thread Mark Janes
We support using the test names as displayed in jenkins to start bisect
jobs.  If a test fails, you can copy/paste the name into a job, and it
will find the commit that broke it.

With this change, special-cased test names can't be used with
--include-tests anymore.

Instead of adding an underscore, can we change one of the characters to
".", so that the expression matching will still work?

-Mark

Dylan Baker  writes:

> This looks fine to me, though I haven't tested it.
>
> Mark, does this seem good to you?
>
> Reviewed-by: Dylan Baker 
>
> On Fri, Feb 20, 2015 at 11:53:50AM +, Jose Fonseca wrote:
>> For example, Jenkins was displaying its REST API page when one navigated
>> to  .../testReport/piglit.spec/ARB_occlusion_query2/api/ result.
>> ---
>>  framework/backends/junit.py | 10 +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/framework/backends/junit.py b/framework/backends/junit.py
>> index 839c99a..ddaf826 100644
>> --- a/framework/backends/junit.py
>> +++ b/framework/backends/junit.py
>> @@ -166,8 +166,16 @@ class JUnitBackend(FileBackend):
>>  # set different root names.
>>  classname = 'piglit.' + classname
>>  
>> +testname += self._test_suffix
>> +
>> +# Jenkins will display special pages when the test has certain 
>> names.
>> +# https://jenkins-ci.org/issue/18062
>> +# https://jenkins-ci.org/issue/19810
>> +if testname in ('api', 'search'):
>> +testname += '_'
>> +
>>  # Create the root element
>> -element = etree.Element('testcase', name=testname + 
>> self._test_suffix,
>> +element = etree.Element('testcase', name=testname,
>>  classname=classname,
>>  time=str(data['time']),
>>  status=str(data['result']))
>> -- 
>> 2.1.0
>> 
>> ___
>> 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] [Mesa-dev] [PATCH] arb_occlusion_query2: expect an error when target mismatch in glBeginQuery()

2015-02-18 Thread Mark Janes
Hi Eduardo,

I track piglit regressions for the mesa team at Intel.  It would really
help me if you put a note in your commit message when a new test is
known to fail on any platform.

thanks,

Mark

Eduardo Lima Mitev  writes:

> As a heads-up, with this patch piglit fails the test for current Mesa.
> But I'm about to send another series of patches for dEQP tests which
> include a fix in Mesa for this issue.
>
> The piglit test in question is:
>
> bin/arb_occlusion_query2-api -auto -fbo
>
> Eduardo
>
> On 02/10/2015 08:48 AM, Eduardo Lima Mitev wrote:
>> From the OpenGL ES 3.0.0 spec, section "2.13. ASYNCHRONOUS QUERIES",
>> page 82:
>>
>> "BeginQuery generates an INVALID_OPERATION error if any of the
>>  following conditions hold: [...]; id is the name of an existing
>>  query object whose type does not match target; [...]
>>
>> OpenGL 3.3 spec has similar wording at section "2.14. ASYNCHRONOUS
>> QUERIES", page 94.
>>
>> Hence, trying to call BeginQuery on a query object which has already
>> been bound to a different target should return GL_INVALID_OPERATION.
>> ---
>>  tests/spec/arb_occlusion_query2/api.c | 49
> +++
>>  1 file changed, 49 insertions(+)
>>
>
> ___
> mesa-dev mailing list
> mesa-...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [Patch v2] cmake: Fix ee8579b452f0 problem

2015-01-19 Thread Mark Janes
Dylan Baker  writes:

> Apparently find_package() and pkg_check_modules() return different
> variables, while piglit still builds on my system without this patch if
> fails on many other systems. This problem was doubly masked by the fact
> that in the original series there is an additional patch to use waffle's
> cmake config file instead of pkg_config. That patch is waiting for the
> next cmake release.
>
> v2: Rebase on top of Jose's patch.
>
> Signed-off-by: Dylan Baker 
> ---
>  CMakeLists.txt| 6 +++---
>  tests/util/CMakeLists.txt | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index b65f990..b7874cf 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -67,13 +67,13 @@ if(PIGLIT_USE_WAFFLE)
>   endif()
>   else ()
>   find_path(Waffle_INCLUDE_DIRS waffle.h)
> -find_library(Waffle_LIBRARIES waffle-1)
> - if(Waffle_INCLUDE_DIRS AND Waffle_LIBRARIES)
> + find_library(Waffle_LDFLAGS waffle-1)
> + if(Waffle_INCLUDE_DIRS AND Waffle_LDFLAGS)
>   set(Waffle_FOUND TRUE)
>   else()
>   message(FATAL_ERROR "Failed to find Waffle. Get and 
> build Waffle from "
>   "http://www.waffle-gl.org and set 
> Waffle_INCLUDE_DIRS and "
> - "Waffle_LIBRARIES variables accordingly."
> + "Waffle_LDFLAGS variables accordingly."
>   )
>   endif()
>   endif ()
> diff --git a/tests/util/CMakeLists.txt b/tests/util/CMakeLists.txt
> index 4c1bad7..45fd25f 100644
> --- a/tests/util/CMakeLists.txt
> +++ b/tests/util/CMakeLists.txt
> @@ -86,7 +86,7 @@ if(PIGLIT_USE_WAFFLE)
>   endif()
>  
>   list(APPEND UTIL_GL_LIBS
> - ${Waffle_LIBRARIES}
> + ${Waffle_LDFLAGS}
>   )
>   IF(PIGLIT_BUILD_GLX_TESTS)
>   list(APPEND UTIL_GL_LIBS
> -- 
> 2.2.1
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit

Tested-by: Mark Janes 

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


Re: [Piglit] [PATCH] texture-packed-formats: Don't try bother with ABGR_EXT formats

2015-01-12 Thread Mark Janes
Tested-by: Mark Janes 

Jason Ekstrand  writes:

> Since GL_ABGR_EXT was extension number 1 to the GL spec, it didn't take
> packed formats into account.  As far as I can tell from the way the packed
> formats extensions are written, packed formats with GL_ABGR_EXT isn't
> allowed by the spec.  NVIDIA allows it but AMD doesn't and our driver
> hasn't allowed ith with UNSIGNED_INT_5_5_5_1 as of c471b09bf4.  Let's stop
> testing invalid things.
> ---
>  tests/texturing/texture-packed-formats.c | 13 -
>  1 file changed, 13 deletions(-)
>
> diff --git a/tests/texturing/texture-packed-formats.c 
> b/tests/texturing/texture-packed-formats.c
> index 1c92b1c..87da808 100644
> --- a/tests/texturing/texture-packed-formats.c
> +++ b/tests/texturing/texture-packed-formats.c
> @@ -89,19 +89,6 @@ static const struct pixel_format Formats[] = {
>   { "GL_BGRA/GL_UNSIGNED_SHORT_1_5_5_5_REV",
> GL_BGRA, GL_UNSIGNED_SHORT_1_5_5_5_REV, 2, 0xfc00, 0x83e0 },
>  
> - { "GL_ABGR_EXT/GL_UNSIGNED_INT_8_8_8_8",
> -   GL_ABGR_EXT, GL_UNSIGNED_INT_8_8_8_8, 4, 0x80ff, 0x8000ff00 },
> - { "GL_ABGR_EXT/GL_UNSIGNED_INT_8_8_8_8_REV",
> -   GL_ABGR_EXT, GL_UNSIGNED_INT_8_8_8_8_REV, 4, 0xff80, 0x00ff0080 },
> - { "GL_ABGR_EXT/GL_UNSIGNED_SHORT_4_4_4_4",
> -   GL_ABGR_EXT, GL_UNSIGNED_SHORT_4_4_4_4, 2, 0x800f, 0x80f0 },
> - { "GL_ABGR_EXT/GL_UNSIGNED_SHORT_4_4_4_4_REV",
> -   GL_ABGR_EXT, GL_UNSIGNED_SHORT_4_4_4_4_REV, 2, 0xf008, 0x0f08 },
> - { "GL_ABGR_EXT/GL_UNSIGNED_SHORT_5_5_5_1",
> -   GL_ABGR_EXT, GL_UNSIGNED_SHORT_5_5_5_1, 2, 0xf801, 0xf83e },
> - { "GL_ABGR_EXT/GL_UNSIGNED_SHORT_1_5_5_5_REV",
> -   GL_ABGR_EXT, GL_UNSIGNED_SHORT_1_5_5_5_REV, 2, 0x800f, 0x7c0f },
> -
>   { "GL_RGB/GL_UNSIGNED_SHORT_5_6_5",
> GL_RGB, GL_UNSIGNED_SHORT_5_6_5, 2, 0xf800, 0x7e0 },
>   { "GL_RGB/GL_UNSIGNED_SHORT_5_6_5_REV",
> -- 
> 2.2.0
>
> ___
> 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] tests/all.py: fix typo in recent list conversion

2015-01-09 Thread Mark Janes
Dylan Baker  writes:

> In one case the comma was put on the inside of the quote, resulting in
> two strings being combined into a single string rather than being two
> elements in a list.
>
> Signed-off-by: Dylan Baker 
> ---
>  tests/all.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/all.py b/tests/all.py
> index 87ff8e9..696ba45 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -1513,7 +1513,7 @@ for stage in ['vs', 'gs', 'fs']:
>  spec[grouptools.join(
>  'glsl-{}'.format(version), 'execution', 'texelFetchOffset',
>  '{}-{}'.format(stage, sampler))] = PiglitGLTest(
> -['texelFetch', 'offset,' '140', stage, sampler],
> +    ['texelFetch', 'offset', '140', stage, sampler],
>  run_concurrent=True)
>  
>  spec['glsl-1.50'] = {}
> -- 
> 2.2.1

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


Re: [Piglit] [PATCH] generate-cl-store-tests.py: fix copy and paste error from 68829470

2014-12-19 Thread Mark Janes
Reviewed-by: Mark Janes 

Dylan Baker  writes:

> bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87513
> Signed-off-by: Dylan Baker 
> ---
>  generated_tests/generate-cl-store-tests.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/generated_tests/generate-cl-store-tests.py 
> b/generated_tests/generate-cl-store-tests.py
> index 586c4f0..e1bc06d 100644
> --- a/generated_tests/generate-cl-store-tests.py
> +++ b/generated_tests/generate-cl-store-tests.py
> @@ -32,7 +32,7 @@ TYPES = ['char', 'uchar', 'short', 'ushort', 'int', 'uint', 
> 'long', 'ulong', 'fl
>  VEC_SIZES = ['', '2', '4', '8', '16']
>  
>  dirName = os.path.join("cl", "store")
> -utils.safe_makedirs(dirname)
> +utils.safe_makedirs(dirName)
>  
>  
>  def gen_array(size):
> -- 
> 2.2.0
>
> ___
> 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 v3] generated_tests: Actually catch exception in os.makedirs

2014-12-19 Thread Mark Janes
Dylan Baker  writes:

> Patch b59ff71eb was supposed to fix os.makedirs exceptions, but falls
> short because of a missing else statement that causes all of the caught
> exceptions to fall back to raise.
>
> This corrects, it also pulls the duplicate functions out into a shared
> module.
>
> v2: - remove accidentally included hunk (Mark)
> v3: - Actually include the modules directory in the commit (Mark)

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


Re: [Piglit] [Patch v2] generated_tests: Actually catch exception in os.makedirs

2014-12-19 Thread Mark Janes
It looks like this patch omits the utils.py module which is being
called.

This would have been hard to spot in a build-test, because the pyc from
your previous patch was still in place.

-Mark
Dylan Baker  writes:

> Patch b59ff71eb was supposed to fix os.makedirs exceptions, but falls
> short because of a missing else statement that causes all of the caught
> exceptions to fall back to raise.
>
> This corrects, it also pulls the duplicate functions out into a shared
> module.
>
> v2: - remove accidentally included hunk (Mark)
>
> Signed-off-by: Dylan Baker 
> ---
>  generated_tests/gen_builtin_packing_tests.py |  9 ++---
>  generated_tests/gen_builtin_uniform_tests.py | 12 
> +++-
>  generated_tests/gen_builtin_uniform_tests_fp64.py| 12 
> +++-
>  generated_tests/gen_const_builtin_equal_tests.py |  9 ++---
>  generated_tests/gen_constant_array_size_tests.py | 11 ++-
>  generated_tests/gen_constant_array_size_tests_fp64.py| 11 ++-
>  generated_tests/gen_interpolation_tests.py   | 11 ++-
>  generated_tests/gen_non-lvalue_tests.py  |  4 ++--
>  generated_tests/gen_outerproduct_invalid_params.py   |  9 ++---
>  generated_tests/gen_outerproduct_tests.py| 10 +++---
>  generated_tests/gen_shader_bit_encoding_tests.py |  9 ++---
>  generated_tests/gen_shader_image_load_store_tests.py | 10 +++---
>  generated_tests/gen_texture_lod_tests.py |  9 ++---
>  generated_tests/gen_texture_query_lod_tests.py   |  9 ++---
>  generated_tests/gen_uniform_initializer_tests.py | 12 
> +++-
>  generated_tests/generate-cl-store-tests.py   | 10 +++---
>  generated_tests/interpolation-qualifier-built-in-variable.py |  9 ++---
>  17 files changed, 40 insertions(+), 126 deletions(-)
>
> diff --git a/generated_tests/gen_builtin_packing_tests.py 
> b/generated_tests/gen_builtin_packing_tests.py
> index 5ca727a..901f493 100644
> --- a/generated_tests/gen_builtin_packing_tests.py
> +++ b/generated_tests/gen_builtin_packing_tests.py
> @@ -43,6 +43,7 @@ from math import copysign, fabs, fmod, frexp, isinf, isnan, 
> modf
>  from numpy import int8, int16, uint8, uint16, uint32, float32
>  
>  from templates import template_dir
> +from modules import utils
>  
>  TEMPLATES = template_dir(os.path.basename(os.path.splitext(__file__)[0]))
>  
> @@ -1027,13 +1028,7 @@ class ShaderTest(object):
>  
>  def write_file(self):
>  dirname = os.path.dirname(self.filename)
> -if not os.path.exists(dirname):
> -try:
> -os.makedirs(dirname)
> -except OSError as e:
> -if e.errno == 17:  # file exists
> -pass
> -raise
> +utils.safe_makedirs(dirname)
>  
>  with open(self.filename, "w") as f:
>  f.write(self.__template.render(func=self.__func_info))
> diff --git a/generated_tests/gen_builtin_uniform_tests.py 
> b/generated_tests/gen_builtin_uniform_tests.py
> index d3a7816..9bb3075 100644
> --- a/generated_tests/gen_builtin_uniform_tests.py
> +++ b/generated_tests/gen_builtin_uniform_tests.py
> @@ -52,6 +52,8 @@ import os
>  import os.path
>  import sys
>  
> +from modules import utils
> +
>  
>  def compute_offset_and_scale(test_vectors):
>  """Compute scale and offset values such that for each result in
> @@ -537,15 +539,7 @@ class ShaderTest(object):
>  shader_test += self.make_test()
>  filename = self.filename()
>  dirname = os.path.dirname(filename)
> -
> -if not os.path.exists(dirname):
> -try:
> -os.makedirs(dirname)
> -except OSError as e:
> -if e.errno == 17:  # file exists
> -pass
> -raise
> -
> +utils.safe_makedirs(dirname)
>  with open(filename, 'w') as f:
>  f.write(shader_test)
>  
> diff --git a/generated_tests/gen_builtin_uniform_tests_fp64.py 
> b/generated_tests/gen_builtin_uniform_tests_fp64.py
> index cf10c85..23032bd 100644
> --- a/generated_tests/gen_builtin_uniform_tests_fp64.py
> +++ b/generated_tests/gen_builtin_uniform_tests_fp64.py
> @@ -52,6 +52,8 @@ import os
>  import os.path
>  import sys
>  
> +from modules import utils
> +
>  
>  def compute_offset_and_scale(test_vectors):
>  """Compute scale and offset values such that for each result in
> @@ -507,15 +509,7 @@ class ShaderTest(object):
>  shader_test += self.make_test()
>  filename = self.filename()
>  dirname = os.path.dirname(filename)
> -
> -if not os.path.exists(dirname):
> -try:
> -os.makedirs(dirname)
> -except OSError as e:
> -if e.errno == 17:  # file exists
> -pa

Re: [Piglit] [PATCH] generated_tests: Actually catch exception in os.makedirs

2014-12-17 Thread Mark Janes
It seems like you unintentionally included an extra file:
generated_tests/genclbuiltins.py

The deltas are not related to os.makedirs.

Dylan Baker  writes:

>  generated_tests/genclbuiltins.py   | 54 
> +-

> diff --git a/generated_tests/genclbuiltins.py 
> b/generated_tests/genclbuiltins.py
> index 7e85a51..681aa29 100644
> --- a/generated_tests/genclbuiltins.py
> +++ b/generated_tests/genclbuiltins.py
> @@ -3,6 +3,8 @@ __all__ = ['gen', 'DATA_SIZES', 'MAX_VALUES', 'MAX', 'MIN', 
> 'BMIN', 'BMAX',
>  
>  import os
>  
> +from modules import utils
> +
>  
>  DATA_SIZES = {
>  'char': 8,
> @@ -139,44 +141,44 @@ def gen_kernel_1_arg(f, fnName, inType, outType):
>  gen_kernel(f, fnName, [inType], outType, [vecSize], '')
>  
>  
> -#  2 argument kernel with input types that match their vector size
> -def gen_kernel_2_arg_same_size(f, fnName, inTypes, outType):
> +#  2 argument kernel with input types that match
> +def gen_kernel_2_arg_same_type(f, fnName, inType, outType):
>  for vecSize in ALL_WIDTHS:
> -gen_kernel(f, fnName, inTypes, outType, [vecSize, vecSize],
> +gen_kernel(f, fnName, [inType, inType], outType, [vecSize, vecSize],
> '')
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [Patch v2] generated_tests: Catch exception in os.makedirs

2014-12-12 Thread Mark Janes
Thanks, this is a superior implementation imho.

-Mark

Dylan Baker  writes:

> +
> +def safe_makedirs(dirs):
> +"""A safe function for creating a directory tree.
> +
> +This function wraps os.makedirs, and provides a couple of sanity checks,
> +first, if the directory already exists it doesn't try to create it, and
> +second if the directory comes into existence between the check and 
> creation
> +time (like if another generator creates it) then the exception will be
> +caught.
> +
> +"""
> +if not os.path.exists(dirs):
> +try:
> +os.makedirs(dirs)
> +except OSError as e:
> +if e.errno == errno.EEXIST:
> +pass
> +raise

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


Re: [Piglit] [PATCH] generated_tests: Except error for os.makedirs

2014-12-12 Thread Mark Janes
Thanks  for making this fix.

Dylan Baker  writes:
> -os.makedirs(dirname)
> +try:
> +os.makedirs(dirname)
> +except OSError as e:
> +if e.errno == 17:  # file exists
> +pass
> +raise

This works for me, but I think it might be more portable if you compare
to errno.EEXIST.  

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


Re: [Piglit] [PATCH v2] util/gl: link against OPENGL_gl_LIBRARY on waffle aware systems

2014-11-26 Thread Mark Janes
This patch fixes all issues with linking piglit gl/gles tests.

Thanks for addressing it.

-Mark

Emil Velikov  writes:

> Similar to the earlier patch for waffle-less (glut) builds. The need
> for this patch became apparent as we removed the libGL link dependency
> in waffle.
>
> v2: Try to handle both gl and gles*.
>
> Cc: Mark Janes 
> Reported-by: Mark Janes 
> Signed-off-by: Emil Velikov 
> ---
>  tests/util/CMakeLists.txt | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/tests/util/CMakeLists.txt b/tests/util/CMakeLists.txt
> index 98eedd0..b95ed2f 100644
> --- a/tests/util/CMakeLists.txt
> +++ b/tests/util/CMakeLists.txt
> @@ -83,6 +83,11 @@ if(PIGLIT_USE_WAFFLE)
>   list(APPEND UTIL_GL_LIBS
>   ${WAFFLE_LDFLAGS}
>   )
> + IF(PIGLIT_BUILD_GLX_TESTS)
> + list(APPEND UTIL_GL_LIBS
> + ${OPENGL_gl_LIBRARY}
> + )
> + ENDIF(PIGLIT_BUILD_GLX_TESTS)
>  else()
>   list(APPEND UTIL_GL_SOURCES
>   piglit-framework-gl/piglit_glut_framework.c
> -- 
> 2.1.3
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] util/gl: link against OPENGL_gl_LIBRARY on waffle aware systems

2014-11-25 Thread Mark Janes
This patched fixed the gl tests, but I encountered the same error for
gles1/gles2/gles3.

Piglit built for me when I added the same link instruction to:

tests/util/CMakeLists.gles1.txt
tests/util/CMakeLists.gles2.txt
tests/util/CMakeLists.gles3.txt

Is there a more specific (GLES) link instruction that should be used,
instead of linking the gles tests against GL?

-Mark

Emil Velikov  writes:

> Similar to the earlier patch for waffle-less (glut) builds. The need
> for this patch became apparent as we removed the libGL link dependency
> in waffle.
>
> Cc: Mark Janes 
> Reported-by: Mark Janes 
> Signed-off-by: Emil Velikov 
> ---
>  tests/util/CMakeLists.gl.txt | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/util/CMakeLists.gl.txt b/tests/util/CMakeLists.gl.txt
> index 34ad63e..ecd9be3 100644
> --- a/tests/util/CMakeLists.gl.txt
> +++ b/tests/util/CMakeLists.gl.txt
> @@ -30,6 +30,10 @@ IF(PIGLIT_BUILD_GLX_TESTS)
>   ${UTIL_GL_SOURCES}
>   piglit-glx-util.c
>   )
> + link_libraries(
> + ${OPENGL_gl_LIBRARY}
> + )
> +
>  ENDIF(PIGLIT_BUILD_GLX_TESTS)
>  
>  piglit_add_library (piglitutil_${piglit_target_api}
> -- 
> 2.1.3
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 2/2] framework/backends/junit: Prepend command line to stdout.

2014-11-21 Thread Mark Janes
Dylan Baker  writes:

> On Friday, November 21, 2014 07:37:20 PM Jose Fonseca wrote:
>> On 20/11/14 12:06, jfons...@vmware.com wrote:
>> > From: José Fonseca 
>> >
>> > Showing the command line of the test is quite useful, specially when
>> > diagnosing failures.
>
> If Marks okay with it.

I like this change.  I regularly find myself running the other backend
manually to get the command.
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH v2] Remove ARB_timer_query / EXT_timer_query from quick.py

2014-11-20 Thread Mark Janes
Ben Widawsky  writes:

> On Tue, Nov 18, 2014 at 02:48:09PM -0800, Mark Janes wrote:
>> Ilia Mirkin  writes:
>> 
>> > FWIW, on nv50 and nvc0, EXT_timer_query time-elapsed and
>> > ARB_timer_query query GL_TIMESTAMP reliably fail, and the rest
>> > reliably pass. I don't remember ever seeing inconsistent behaviour.
>> > FWIW I don't really know what these extensions do, or what the tests
>> > check for, but just thought I'd provide the data point.
>> 
>> We probably see the failure more often because we run test with gbm, so
>> the CPU is consistently pegged on all cores.  Comparing cpu times with
>> gpu times is likely to be error-prone in this situation.
>> 
>> Since it only affects our automated system, we'll exclude it when we
>> invoke piglit.
>> 
>> -Mark
>
> I think we should change the "Couldn't find appropriate number of iterations" 
> to
> a skip. That seems to be the common failure on GBM (for me). All other 
> failures
> are likely to be real^winteresting failures.

The failure I've seen on the older machines is "GPU time didn't match
CPU time".  I've already disabled this test in our environment, so it's
a non-issue.

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


Re: [Piglit] [PATCH v2] Remove ARB_timer_query / EXT_timer_query from quick.py

2014-11-18 Thread Mark Janes
Ilia Mirkin  writes:

> FWIW, on nv50 and nvc0, EXT_timer_query time-elapsed and
> ARB_timer_query query GL_TIMESTAMP reliably fail, and the rest
> reliably pass. I don't remember ever seeing inconsistent behaviour.
> FWIW I don't really know what these extensions do, or what the tests
> check for, but just thought I'd provide the data point.

We probably see the failure more often because we run test with gbm, so
the CPU is consistently pegged on all cores.  Comparing cpu times with
gpu times is likely to be error-prone in this situation.

Since it only affects our automated system, we'll exclude it when we
invoke piglit.

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


[Piglit] [PATCH v2] Remove ARB_timer_query / EXT_timer_query from quick.py

2014-11-18 Thread Mark Janes
EXT_timer_query and ARB_timer_query tests fail intermittently, causing
confusion for developers running quick.py to find regressions.  These
tests have always been intermittent, and people generally know to
ignore them when they fail.

However, if everyone ignores a test, there is no point in running it
all the time.
---
 tests/quick.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/quick.py b/tests/quick.py
index 8762d7d..0856f75 100644
--- a/tests/quick.py
+++ b/tests/quick.py
@@ -12,3 +12,5 @@ del profile.tests['shaders']['glsl-fs-inline-explosion']
 del profile.tests['shaders']['glsl-fs-unroll-explosion']
 del profile.tests['shaders']['glsl-vs-inline-explosion']
 del profile.tests['shaders']['glsl-vs-unroll-explosion']
+del profile.tests['spec']['EXT_timer_query']
+del profile.tests['spec']['ARB_timer_query']
-- 
2.1.3

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


  1   2   >