Re: [Piglit] [PATCH 4/4] piglit-print-commands: Add --format option

2016-05-06 Thread Brian Paul

On 05/06/2016 02:23 PM, Dylan Baker wrote:

Quoting Brian Paul (2016-05-03 15:50:01)

On 05/03/2016 03:59 PM, Dylan Baker wrote:

This option allows the format of the output string to be modified by
passing a command line argument. This allows for specialized formats to
be printed for other uses than the original usage that print-commands
was designed for.


Can you give an example of what the --format option would look like?

I think the help message below ("Format will be called with the {name}
will be replaced with the name of the test, and {command} with the test
command") is a bit scrambled.

-Brian





Signed-off-by: Dylan Baker 
---
   framework/programs/print_commands.py | 13 -
   1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/framework/programs/print_commands.py 
b/framework/programs/print_commands.py
index 06bd004..c7ccf15 100755
--- a/framework/programs/print_commands.py
+++ b/framework/programs/print_commands.py
@@ -68,6 +68,15 @@ def main(input_):
   metavar="",
   help="Exclude matching tests (can be used more than "
"once)")
+parser.add_argument("--format",
+dest="format_string",
+default="{name} ::: {command}",
+action="store",
+help="A python format string to be passed to "
+ "str.format. Format will be called with the "
+ "{name} will be replaced with the name of the "
+ "test, and {command} with the test command. "
+ "Neither are required.")
   parser.add_argument("testProfile",
   metavar="",
   help="Path to results folder")
@@ -85,4 +94,6 @@ def main(input_):
   profile_._prepare_test_list()
   for name, test in six.iteritems(profile_.test_list):
   assert isinstance(test, Test)
-print(name, ':::', get_command(test, piglit_dir))
+print(args.format_string.format(
+name=name,
+command=get_command(test, piglit_dir)))





What about something like this:

A template string that defines the output format. It has two replacement tokens
that can be provided, along with an arbitrary text, which will be printed
verbatim. The two tokens are '{name}', which will be replaced with the name of
the test; and '{command}', which will be replaced with the command to run the
test.



Sounds great.  Thanks.

-Brian

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


Re: [Piglit] [PATCH 2/2] cl: check for image support using util/ functions

2016-05-06 Thread Jan Vesely
On Fri, 2016-05-06 at 13:11 +0200, Serge Martin wrote:
> ---
>  tests/cl/api/create-image.c   | 19 +--
>  tests/cl/api/create-sampler.c | 18 +-
>  tests/cl/api/enqueue-fill-image.c | 10 +-
>  tests/cl/api/get-image-info.c | 19 +--
>  tests/cl/api/set-kernel-arg.c | 18 +-
>  5 files changed, 5 insertions(+), 79 deletions(-)
> 
> diff --git a/tests/cl/api/create-image.c b/tests/cl/api/create-
> image.c
> index 1ee5f71..29e552c 100644
> --- a/tests/cl/api/create-image.c
> +++ b/tests/cl/api/create-image.c
> @@ -38,23 +38,6 @@ PIGLIT_CL_API_TEST_CONFIG_BEGIN
>  
>  PIGLIT_CL_API_TEST_CONFIG_END
>  
> -static bool context_has_image_support(const piglit_cl_context ctx)
> -{
> - unsigned i;
> - for(i = 0; i < ctx->num_devices; i++) {
> - int *image_support =
> - piglit_cl_get_device_info(ctx-
> >device_ids[i],
> - CL_DEVICE_IMAGE_SUPP
> ORT);
> - if (*image_support) {
> - free(image_support);
> - return true;
> - }
> -
> - free(image_support);
> - }
> - return false;
> -}
> -
>  static void
>  no_image_check_invalid(
>   cl_int errcode_ret,
> @@ -107,7 +90,7 @@ piglit_cl_test(const int argc,
> const struct piglit_cl_api_test_config* config,
> const struct piglit_cl_api_test_env* env)
>  {
> - if (!context_has_image_support(env->context)) {
> + if (!piglit_cl_get_context_image_support(env->context)) {
>   return no_image_tests(env);
>   } else {
>   return PIGLIT_PASS;
> diff --git a/tests/cl/api/create-sampler.c b/tests/cl/api/create-
> sampler.c
> index dcdef05..d51fe47 100644
> --- a/tests/cl/api/create-sampler.c
> +++ b/tests/cl/api/create-sampler.c
> @@ -36,22 +36,6 @@ PIGLIT_CL_API_TEST_CONFIG_BEGIN
>  
>  PIGLIT_CL_API_TEST_CONFIG_END
>  
> -static bool context_has_image_support(const piglit_cl_context ctx)
> -{
> - unsigned i;
> - for(i = 0; i < ctx->num_devices; i++) {
> - int *image_support =
> - piglit_cl_get_device_info(ctx-
> >device_ids[i],
> - CL_DEVICE_IMAGE_SUPP
> ORT);
> - if (*image_support) {
> - free(image_support);
> - return true;
> - }
> - free(image_support);
> - }
> - return false;
> -}
> -
>  static enum piglit_result
>  no_image_tests(const struct piglit_cl_api_test_env* env)
>  {
> @@ -80,7 +64,7 @@ piglit_cl_test(const int argc,
> const struct piglit_cl_api_test_config* config,
> const struct piglit_cl_api_test_env* env)
>  {
> - if (!context_has_image_support(env->context)) {
> + if (!piglit_cl_get_context_image_support(env->context)) {
>   return no_image_tests(env);
>   } else {
>   return PIGLIT_PASS;
> diff --git a/tests/cl/api/enqueue-fill-image.c
> b/tests/cl/api/enqueue-fill-image.c
> index 2839b67..4de5dca 100644
> --- a/tests/cl/api/enqueue-fill-image.c
> +++ b/tests/cl/api/enqueue-fill-image.c
> @@ -104,19 +104,11 @@ piglit_cl_test(const int argc,
>   cl_command_queue queue = env->context->command_queues[0];
>   int i;
>  
> - cl_bool *image_support =
> - piglit_cl_get_device_info(env->context-
> >device_ids[0],
> -   CL_DEVICE_IMAGE_SUPPORT);
> -
> - if (!*image_support) {
> + if (!piglit_cl_get_device_image_support(env->context-
> >device_ids[0])) {
>   fprintf(stderr, "No image support\n");
> - free(image_support);
>   return PIGLIT_SKIP;
>   }
>  
> - free(image_support);
> - image_support = NULL;
> -
>   img_format.image_channel_order = CL_RGBA;
>   img_format.image_channel_data_type = CL_UNSIGNED_INT8;
>   img_desc.image_type = CL_MEM_OBJECT_IMAGE2D;
> diff --git a/tests/cl/api/get-image-info.c b/tests/cl/api/get-image-
> info.c
> index a8b5bec..d4dc842 100644
> --- a/tests/cl/api/get-image-info.c
> +++ b/tests/cl/api/get-image-info.c
> @@ -46,23 +46,6 @@ PIGLIT_CL_API_TEST_CONFIG_BEGIN
>  
>  PIGLIT_CL_API_TEST_CONFIG_END
>  
> -static bool context_has_image_support(const piglit_cl_context ctx)
> -{
> - int ret = 0;
> - unsigned i;
> - for(i = 0; i < ctx->num_devices; i++) {
> - int *image_support =
> - piglit_cl_get_device_info(ctx-
> >device_ids[i],
> - CL_DEVICE_IMAGE_SUPP
> ORT);
> - if (image_support)
> - ret |= *image_support;
> -
> - free(image_support);
> - }
> - return ret;
> -}
> -
> -
>  enum piglit_result
>  piglit_cl_test(const int argc,
> const char** argv,
> @@ -79,7 +62,7 @@ piglit_cl_test(const int argc,
>     

[Piglit] [PATCH] spec/glsl-es-1.00/linker: test pass cases for invariant conditions

2016-05-06 Thread Lars Hamre
These test the cases where gl_FragCoord and gl_PointCoord are
allowed to be declared invariant.

Signed-off-by: Lars Hamre 

---

NOTE: someone with access will need to commit this after the
  review process

 .../linker/glsl-fcoord-invariant-pass.shader_test  | 27 +
 .../linker/glsl-pcoord-invariant-pass.shader_test  | 28 ++
 2 files changed, 55 insertions(+)
 create mode 100644 
tests/spec/glsl-es-1.00/linker/glsl-fcoord-invariant-pass.shader_test
 create mode 100644 
tests/spec/glsl-es-1.00/linker/glsl-pcoord-invariant-pass.shader_test

diff --git 
a/tests/spec/glsl-es-1.00/linker/glsl-fcoord-invariant-pass.shader_test 
b/tests/spec/glsl-es-1.00/linker/glsl-fcoord-invariant-pass.shader_test
new file mode 100644
index 000..76348a6
--- /dev/null
+++ b/tests/spec/glsl-es-1.00/linker/glsl-fcoord-invariant-pass.shader_test
@@ -0,0 +1,27 @@
+#
+# OpenGL ES 1.00 specification "Invariance and linkage":
+#
+#"For the built-in special variables, gl_FragCoord can
+#only be declared invariant if and only if gl_Position is
+#declared invariant. Similarly gl_PointCoord can only be
+#declared invariant if and only if gl_PointSize is declared
+#invariant. It is an error to declare gl_FrontFacing as invariant."
+#
+[require]
+GL ES >= 2.0
+GLSL ES >= 1.00
+
+[vertex shader]
+invariant gl_Position;
+void main() {
+   gl_Position = vec4(0.0);
+}
+
+[fragment shader]
+invariant gl_FragCoord;
+void main() {
+   gl_FragColor = vec4(gl_FragCoord.x);
+}
+
+[test]
+link success
diff --git 
a/tests/spec/glsl-es-1.00/linker/glsl-pcoord-invariant-pass.shader_test 
b/tests/spec/glsl-es-1.00/linker/glsl-pcoord-invariant-pass.shader_test
new file mode 100644
index 000..c0e9673
--- /dev/null
+++ b/tests/spec/glsl-es-1.00/linker/glsl-pcoord-invariant-pass.shader_test
@@ -0,0 +1,28 @@
+#
+# OpenGL ES 1.00 specification "Invariance and linkage":
+#
+#"For the built-in special variables, gl_FragCoord can
+#only be declared invariant if and only if gl_Position is
+#declared invariant. Similarly gl_PointCoord can only be
+#declared invariant if and only if gl_PointSize is declared
+#invariant. It is an error to declare gl_FrontFacing as invariant."
+#
+[require]
+GL ES >= 2.0
+GLSL ES >= 1.00
+
+[vertex shader]
+invariant gl_PointSize;
+void main() {
+   gl_PointSize = 1.0;
+   gl_Position = vec4(0.0);
+}
+
+[fragment shader]
+invariant gl_PointCoord;
+void main() {
+   gl_FragColor = vec4(gl_PointCoord.x);
+}
+
+[test]
+link success
--
2.5.5

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


[Piglit] [PATCH v2] glean/tfragprog1: port ADD tests to shader_runner

2016-05-06 Thread Dylan Baker
This ports the following tests:
- ADD test
- ADD with saturation
- ADD an immediate
- ADD negative immediate

It does not port ADD negative "immediate (2)", which adds MOV, MUL, and
swizzling. It might be a useful test but it's not really testing ADD
other than that it tests that ADD can be used with a swizzle.

cc: Topi Pohjolainen 
Signed-off-by: Dylan Baker 

v2: - Cover more cases in the ADD_SAT test
---
 tests/all.py   |  5 --
 tests/glean/tfragprog1.cpp | 75 --
 .../built-in-functions/add-immediate.shader_test   | 16 +
 .../add-negative-immediate.shader_test | 16 +
 .../built-in-functions/add.shader_test | 18 ++
 .../built-in-functions/add_sat.shader_test | 22 +++
 6 files changed, 72 insertions(+), 80 deletions(-)
 create mode 100644 
tests/spec/arb_fragment_program/built-in-functions/add-immediate.shader_test
 create mode 100644 
tests/spec/arb_fragment_program/built-in-functions/add-negative-immediate.shader_test
 create mode 100644 
tests/spec/arb_fragment_program/built-in-functions/add.shader_test
 create mode 100644 
tests/spec/arb_fragment_program/built-in-functions/add_sat.shader_test

diff --git a/tests/all.py b/tests/all.py
index 217216e..70be904 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -376,11 +376,6 @@ glean_glsl_tests = ['Primary plus secondary color',
 'texcoord varying']
 
 glean_fp_tests = [
-  'ADD test',
-  'ADD with saturation',
-  'ADD an immediate',
-  'ADD negative immediate',
-  'ADD negative immediate (2)',
   'CMP test',
   'COS test',
   'COS test 2',
diff --git a/tests/glean/tfragprog1.cpp b/tests/glean/tfragprog1.cpp
index bc340ed..1cbe243 100644
--- a/tests/glean/tfragprog1.cpp
+++ b/tests/glean/tfragprog1.cpp
@@ -82,81 +82,6 @@ static GLfloat FogCoord = 50.0;  /* Between FogStart and 
FogEnd */
 // Alphabetical order, please
 static const FragmentProgram Programs[] = {
{
-   "ADD test",
-   "!!ARBfp1.0\n"
-   "PARAM p = program.local[1]; \n"
-   "ADD result.color, fragment.color, p; \n"
-   "END \n",
-   { CLAMP01(FragColor[0] + Param1[0]),
- CLAMP01(FragColor[1] + Param1[1]),
- CLAMP01(FragColor[2] + Param1[2]),
- CLAMP01(FragColor[3] + Param1[3])
-   },
-   DONT_CARE_Z
-   },
-   {
-   "ADD with saturation",
-   "!!ARBfp1.0\n"
-   "PARAM p = program.local[1]; \n"
-"TEMP t; \n"
-"ADD t, p, p; \n"
-   "ADD_SAT result.color, t, p; \n"
-   "END \n",
-   { CLAMP01(Param1[0] + Param1[0] + Param1[0]),
- CLAMP01(Param1[1] + Param1[1] + Param1[1]),
- CLAMP01(Param1[2] + Param1[2] + Param1[2]),
- CLAMP01(Param1[3] + Param1[3] + Param1[3]),
-   },
-   DONT_CARE_Z
-   },
-
-   {
-   "ADD an immediate",
-   "!!ARBfp1.0\n"
-   "PARAM p = program.local[1]; \n"
-   "ADD result.color, p, {0.25, 0.0, 0.5, 0.25}; \n"
-   "END \n",
-   { CLAMP01(Param1[0] + 0.25),
- CLAMP01(Param1[1] + 0.0),
- CLAMP01(Param1[2] + 0.5),
- CLAMP01(Param1[3] + 0.25),
-   },
-   DONT_CARE_Z
-   },
-
-   {
-   "ADD negative immediate",
-   "!!ARBfp1.0\n"
-   "PARAM p = program.local[1]; \n"
-   "ADD result.color, p, {-0.25, -0.2, 0.0, -0.25}; \n"
-   "END \n",
-   { CLAMP01(Param1[0] - 0.25),
- CLAMP01(Param1[1] - 0.2),
- CLAMP01(Param1[2] - 0.0),
- CLAMP01(Param1[3] - 0.25),
-   },
-   DONT_CARE_Z
-   },
-
-   {
-   "ADD negative immediate (2)",
-   "!!ARBfp1.0\n"
-   "PARAM p = program.local[1]; \n"
-   "TEMP t; \n"
-   "MOV t, p; \n"
-   "MUL t.xyz, t, 2.0; \n"
-   "ADD t.xyz, t, -1.0; \n"
-   "MOV result.color, t; \n"
-   "END \n",
-   { CLAMP01(Param1[0] * 2.0 - 1.0),
- CLAMP01(Param1[1] * 2.0 - 1.0),
- CLAMP01(Param1[2] * 2.0 - 1.0),
- CLAMP01(Param1[3] ),
-   },
-   DONT_CARE_Z
-   },
-
-   {
"CMP test",
"!!ARBfp1.0\n"
"PARAM zero = program.local[0]; \n"
diff --git 
a/tests/spec/arb_fragment_program/built-in-functions/add-immediate.shader_test 
b/tests/spec/arb_fragment_program/built-in-functions/add-immediate.shader_test
new file mode 100644
inde

Re: [Piglit] [PATCH 03/37] glean/tfragprog1: port ADD tests to shader_runner

2016-05-06 Thread Dylan Baker
Quoting Pohjolainen, Topi (2016-05-04 10:25:31)
[sni[
> Okay, I don't how I mis-read the original tests so badly - CLAMP01 is the part
> that defines the expected value. Anyway, if we want ADD_SAT to kick in, we 
> need
> to change the input so that the result > 1.
> You have tried to define the inputs in such a way that yields all green in
> succees case. This is how most tests are setup. However, there are also tests
> that use other combinations and it might be justified here - otherwise we have
> just easy values of zero in all other channels other than green. If we
> wanted to keep strictly to the original, we would write:
> 
> fragment program]
> ARBfp1.0
> PARAM p = program.local[0];
> TEMP s;
> ADD s, p, p;
> ADD_SAT result.color, p, s;
> END
> 
> [test]
> clear color 0.5 0.5 0.5 0.5
> clear
> 
> parameter local_fp 0 (0.5, 0.25, 1.0, 0.5)
> draw rect -1 -1 2 2
> probe all rgba 1.0 0.75 1.0 1.0
> 
> 
> 
> Original had:
> 
> #define PARAM1 { 0.5, 0.25, 1.0, 0.5 }
> static const GLfloat Param1[4] = PARAM1;

Right, but that's not strictly true either because the original defines
a value for result.color. I have a v2 of this patch I'll send that
covers the "x < -1", "-1 < x < 0", "1 > x > 0", and the "x > 1" case.


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


Re: [Piglit] [PATCH 4/4] piglit-print-commands: Add --format option

2016-05-06 Thread Dylan Baker
Quoting Brian Paul (2016-05-03 15:50:01)
> On 05/03/2016 03:59 PM, Dylan Baker wrote:
> > This option allows the format of the output string to be modified by
> > passing a command line argument. This allows for specialized formats to
> > be printed for other uses than the original usage that print-commands
> > was designed for.
> 
> Can you give an example of what the --format option would look like?
> 
> I think the help message below ("Format will be called with the {name} 
> will be replaced with the name of the test, and {command} with the test 
> command") is a bit scrambled.
> 
> -Brian
> 
> 
> 
> >
> > Signed-off-by: Dylan Baker 
> > ---
> >   framework/programs/print_commands.py | 13 -
> >   1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/framework/programs/print_commands.py 
> > b/framework/programs/print_commands.py
> > index 06bd004..c7ccf15 100755
> > --- a/framework/programs/print_commands.py
> > +++ b/framework/programs/print_commands.py
> > @@ -68,6 +68,15 @@ def main(input_):
> >   metavar="",
> >   help="Exclude matching tests (can be used more 
> > than "
> >"once)")
> > +parser.add_argument("--format",
> > +dest="format_string",
> > +default="{name} ::: {command}",
> > +action="store",
> > +help="A python format string to be passed to "
> > + "str.format. Format will be called with the "
> > + "{name} will be replaced with the name of the 
> > "
> > + "test, and {command} with the test command. "
> > + "Neither are required.")
> >   parser.add_argument("testProfile",
> >   metavar="",
> >   help="Path to results folder")
> > @@ -85,4 +94,6 @@ def main(input_):
> >   profile_._prepare_test_list()
> >   for name, test in six.iteritems(profile_.test_list):
> >   assert isinstance(test, Test)
> > -print(name, ':::', get_command(test, piglit_dir))
> > +print(args.format_string.format(
> > +name=name,
> > +command=get_command(test, piglit_dir)))
> >
> 

What about something like this:

A template string that defines the output format. It has two replacement tokens
that can be provided, along with an arbitrary text, which will be printed
verbatim. The two tokens are '{name}', which will be replaced with the name of
the test; and '{command}', which will be replaced with the command to run the
test.


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


[Piglit] [PATCH 13/14] framework/summary/html_.py: use core.check_dir

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This just reduces some code duplication.

Signed-off-by: Dylan Baker 
---
 framework/summary/html_.py | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/framework/summary/html_.py b/framework/summary/html_.py
index 80bee82..f7fdc85 100644
--- a/framework/summary/html_.py
+++ b/framework/summary/html_.py
@@ -38,7 +38,7 @@ import six
 
 # a local variable status exists, prevent accidental overloading by renaming
 # the module
-from framework import backends, exceptions
+from framework import backends, exceptions, core
 
 from .common import Results, escape_filename, escape_pathname
 from .feature import FeatResults
@@ -85,16 +85,13 @@ def _make_testrun_info(results, destination, exclude=None):
 for each in results.results:
 name = escape_pathname(each.name)
 try:
-os.mkdir(os.path.join(destination, name))
-except OSError as e:
-if e.errno == errno.EEXIST:
-raise exceptions.PiglitFatalError(
-'Two or more of your results have the same "name" '
-'attribute. Try changing one or more of the "name" '
-'values in your json files.\n'
-'Duplicate value: {}'.format(name))
-else:
-raise e
+core.check_dir(os.path.join(destination, name), True)
+except exceptions.PiglitException:
+raise exceptions.PiglitFatalError(
+'Two or more of your results have the same "name" '
+'attribute. Try changing one or more of the "name" '
+'values in your json files.\n'
+'Duplicate value: {}'.format(name))
 
 with open(os.path.join(destination, name, "index.html"), 'wb') as out:
 out.write(_TEMPLATES.get_template('testrun_info.mako').render(
@@ -114,11 +111,7 @@ def _make_testrun_info(results, destination, exclude=None):
 temp_path = os.path.dirname(html_path)
 
 if value.result not in exclude:
-# os.makedirs is very annoying, it throws an OSError if
-# the path requested already exists, so do this check to
-# ensure that it doesn't
-if not os.path.exists(temp_path):
-os.makedirs(temp_path)
+core.check_dir(temp_path)
 
 with open(html_path, 'wb') as out:
 out.write(_TEMPLATES.get_template(
-- 
2.8.2

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


[Piglit] [PATCH 14/14] unittests/core_tests.py: Add tests that only apply to python3

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

There are some Exceptions that changed in python 3.x, this adds a test
that will only by run on python 3

Signed-off-by: Dylan Baker 
---
 unittests/core_tests.py | 12 +++-
 unittests/utils.py  | 25 +
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/unittests/core_tests.py b/unittests/core_tests.py
index 8cbac8e..825fb73 100644
--- a/unittests/core_tests.py
+++ b/unittests/core_tests.py
@@ -295,7 +295,7 @@ def test_check_dir_stat_ENOENT():
 
 
 def test_check_dir_stat_ENOTDIR():
-"""core.check_dir: if the directory exists (ENOTDIR) and failifexsits is 
False continue"""
+"""core.check_dir: if a file exists (ENOTDIR) and failifexsits is False 
continue"""
 with mock.patch('framework.core.os.stat',
 mock.Mock(side_effect=OSError('foo', errno.ENOTDIR))):
 with mock.patch('framework.core.os.makedirs') as makedirs:
@@ -330,3 +330,13 @@ def test_check_dir_handler():
 mock.Mock(side_effect=OSError('foo', errno.ENOTDIR))):
 core.check_dir('foo',
handler=mock.Mock(side_effect=utils.SentinalException))
+
+
+@utils.skip(not six.PY3, 'Test is only relevant on python 3.x')
+def test_check_dir_stat_FileNotFoundError():
+"""core.check_dir: FileNotFoundError is raised and failifexsits is False 
continue"""
+with mock.patch('framework.core.os.stat',
+mock.Mock(side_effect=FileNotFoundError)):
+with mock.patch('framework.core.os.makedirs') as makedirs:
+core.check_dir('foo', False)
+nt.eq_(makedirs.called, 1)
diff --git a/unittests/utils.py b/unittests/utils.py
index 255f253..99cf482 100644
--- a/unittests/utils.py
+++ b/unittests/utils.py
@@ -541,3 +541,28 @@ def unset_compression():
 os.environ['PIGLIT_COMPRESSION'] = _SAVED_COMPRESSION
 else:
 del os.environ['PIGLIT_COMPRESSION']
+
+
+def skip(condition, description):
+"""Skip a test if the condition is met.
+
+Arguments:
+condition -- If this is truthy then the test will be skippped
+description -- the message that SkipTest will display if the test is
+   skipped
+
+"""
+
+def _wrapper(func):
+"""The function that acutally does the wrapping."""
+
+@functools.wraps(func)
+def _inner(*args, **kwargs):
+"""The function that is actually called."""
+if condition:
+raise SkipTest(description)
+return func(*args, **kwargs)
+
+return _inner
+
+return _wrapper
-- 
2.8.2

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


[Piglit] [PATCH 07/14] framework: rename core.checkDir to core.check_dir

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This is the PEP8 proper format for the name, and since piglit tries to
conform to PEP8 as practical it makes sense to rename this. It is pretty
much the non-PEP8 named function in piglit.

Signed-off-by: Dylan Baker 
---
 framework/core.py |  2 +-
 framework/programs/summary.py |  2 +-
 unittests/core_tests.py   | 10 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/framework/core.py b/framework/core.py
index d052bd5..dfab227 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -115,7 +115,7 @@ def get_config(arg=None):
 
 
 # Ensure the given directory exists
-def checkDir(dirname, failifexists):
+def check_dir(dirname, failifexists):
 try:
 os.stat(dirname)
 except OSError as e:
diff --git a/framework/programs/summary.py b/framework/programs/summary.py
index 13368e4..c297a7b 100644
--- a/framework/programs/summary.py
+++ b/framework/programs/summary.py
@@ -102,7 +102,7 @@ def html(input_):
 
 # If the requested directory doesn't exist, create it or throw an error
 try:
-core.checkDir(args.summaryDir, not args.overwrite)
+core.check_dir(args.summaryDir, not args.overwrite)
 except exceptions.PiglitException:
 raise exceptions.PiglitFatalError(
 '{} already exists.\n'
diff --git a/unittests/core_tests.py b/unittests/core_tests.py
index e152336..c0ea523 100644
--- a/unittests/core_tests.py
+++ b/unittests/core_tests.py
@@ -282,7 +282,7 @@ class TestPiglitConfig(object):
 def test_check_dir_exists_fail():
 """core.check_dir: if the directory exists and failifexsits is True fail"""
 with mock.patch('framework.core.os.stat', mock.Mock(side_effect=OSError)):
-core.checkDir('foo', True)
+core.check_dir('foo', True)
 
 
 def test_check_dir_stat_ENOENT():
@@ -290,7 +290,7 @@ def test_check_dir_stat_ENOENT():
 with mock.patch('framework.core.os.stat',
 mock.Mock(side_effect=OSError('foo', errno.ENOENT))):
 with mock.patch('framework.core.os.makedirs') as makedirs:
-core.checkDir('foo', False)
+core.check_dir('foo', False)
 nt.eq_(makedirs.called, 1)
 
 
@@ -299,7 +299,7 @@ def test_check_dir_stat_ENOTDIR():
 with mock.patch('framework.core.os.stat',
 mock.Mock(side_effect=OSError('foo', errno.ENOTDIR))):
 with mock.patch('framework.core.os.makedirs') as makedirs:
-core.checkDir('foo', False)
+core.check_dir('foo', False)
 nt.eq_(makedirs.called, 1)
 
 
@@ -309,7 +309,7 @@ def test_check_dir_makedirs_pass():
 with mock.patch('framework.core.os.stat', mock.Mock()):
 with mock.patch('framework.core.os.makedirs',
 mock.Mock(side_effect=OSError(errno.EEXIST, 'foo'))):
-core.checkDir('foo', False)
+core.check_dir('foo', False)
 
 
 @nt.raises(OSError)
@@ -320,4 +320,4 @@ def test_check_dir_makedirs_fail():
 mock.Mock(return_value=False)):
 with mock.patch('framework.core.os.makedirs',
 mock.Mock(side_effect=OSError)):
-core.checkDir('foo', False)
+core.check_dir('foo', False)
-- 
2.8.2

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


[Piglit] [PATCH 02/14] core.py: use 'in list' instead of a == b or a == c...

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This is cleaner and idiomatic.

Signed-off-by: Dylan Baker 
---
 framework/core.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/core.py b/framework/core.py
index fe4e4a4..870a708 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -121,7 +121,7 @@ def checkDir(dirname, failifexists):
 try:
 os.stat(dirname)
 except OSError as e:
-if e.errno == errno.ENOENT or e.errno == errno.ENOTDIR:
+if e.errno in [errno.ENOENT, errno.ENOTDIR]:
 exists = False
 
 if exists and failifexists:
-- 
2.8.2

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


[Piglit] [PATCH 08/14] core.py: make second argument to check_dir a keyword argument

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This allows callers that don't want to fail to leave the keyword out. It
doesn't affect those that set it to true.

Signed-off-by: Dylan Baker 
---
 framework/core.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/core.py b/framework/core.py
index dfab227..97b9a4b 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -115,7 +115,7 @@ def get_config(arg=None):
 
 
 # Ensure the given directory exists
-def check_dir(dirname, failifexists):
+def check_dir(dirname, failifexists=False):
 try:
 os.stat(dirname)
 except OSError as e:
-- 
2.8.2

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


[Piglit] [PATCH 12/14] framework/core.py: add module docstring

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

Signed-off-by: Dylan Baker 
---
 framework/core.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/framework/core.py b/framework/core.py
index f76fb8b..c2de5d0 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -18,6 +18,13 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
+"""A collection of various bits that don't seem to belong anywhere else.
+
+This is the classic catchall "utils" module from most projects, that for
+historically reasons is called "core" in piglit.
+
+"""
+
 from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
-- 
2.8.2

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


[Piglit] [PATCH 09/14] core.py: add docstring to check_dir

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

Signed-off-by: Dylan Baker 
---
 framework/core.py | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/framework/core.py b/framework/core.py
index 97b9a4b..60213c2 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -114,8 +114,24 @@ def get_config(arg=None):
 pass
 
 
-# Ensure the given directory exists
 def check_dir(dirname, failifexists=False):
+"""Check for the existance of a directory and create it if possible.
+
+This function will check for the existance of a directory. If that
+directory doesn't exist it will try to create it. If the directory does
+exist than it does one of two things.
+1) If "failifexists" is False (default): it will just return
+2) If "failifexists" is True it will raise an PiglitException, it is the
+job of the caller using failifexists=True to handle this exception
+
+Arguments:
+dirname -- the name of the directory to check
+
+Keyword Arguments:
+failifexists -- If True and the directory exists then PiglitException will
+be raised (default: False)
+
+"""
 try:
 os.stat(dirname)
 except OSError as e:
-- 
2.8.2

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


[Piglit] [PATCH 11/14] framework/core.py: Make copyright header match others in the framework

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

The copyright notice was slightly different than most of the framework,
(mostly things like "author(s)" vs "authors or copyright holders"), and
the line length was different.

Signed-off-by: Dylan Baker 
---
 framework/core.py | 42 +++---
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/framework/core.py b/framework/core.py
index 6832c0a..f76fb8b 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -1,26 +1,22 @@
-
-# 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:
-#
-# 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 AUTHOR(S) 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 core
+# Copyright (c) 2014-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.
 
 from __future__ import (
 absolute_import, division, print_function, unicode_literals
-- 
2.8.2

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


[Piglit] [PATCH 10/14] framework/core: extend check_dir to take a handler callable

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This allows check_dir to fulfill the needs of the overwrite switch in
the main run function.

Signed-off-by: Dylan Baker 
---
 framework/core.py | 18 ++
 framework/programs/run.py | 27 ---
 unittests/core_tests.py   |  9 +
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/framework/core.py b/framework/core.py
index 60213c2..6832c0a 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -114,8 +114,8 @@ def get_config(arg=None):
 pass
 
 
-def check_dir(dirname, failifexists=False):
-"""Check for the existance of a directory and create it if possible.
+def check_dir(dirname, failifexists=False, handler=None):
+"""Check for the existence of a directory and create it if possible.
 
 This function will check for the existance of a directory. If that
 directory doesn't exist it will try to create it. If the directory does
@@ -124,21 +124,31 @@ def check_dir(dirname, failifexists=False):
 2) If "failifexists" is True it will raise an PiglitException, it is the
 job of the caller using failifexists=True to handle this exception
 
+Both failifexists and handler can be passed, but failifexists will have
+precedence.
+
 Arguments:
 dirname -- the name of the directory to check
 
 Keyword Arguments:
 failifexists -- If True and the directory exists then PiglitException will
 be raised (default: False)
+handler -- a callable that is passed dirname if the thing to check exists.
 
 """
 try:
 os.stat(dirname)
 except OSError as e:
-if e.errno not in [errno.ENOENT, errno.ENOTDIR] and failifexists:
-raise exceptions.PiglitException
+# If the error is not "no file or directory" or "not a dir", then
+# either raise an exception, call the handler function, or return
+if e.errno not in [errno.ENOENT, errno.ENOTDIR]:
+if failifexists:
+raise exceptions.PiglitException
+elif handler is not None:
+handler(dirname)
 
 try:
+# makedirs is expensive, so check before # calling it.
 if not os.path.exists(dirname):
 os.makedirs(dirname)
 except OSError as e:
diff --git a/framework/programs/run.py b/framework/programs/run.py
index dff9f49..f1b237d 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -212,6 +212,14 @@ def _disable_windows_exception_messages():
 ctypes.windll.kernel32.SetErrorMode(uMode)
 
 
+def _results_handler(path):
+"""Handler for core.check_dir."""
+if os.path.isdir(path):
+shutil.rmtree(path)
+else:
+os.unlink(path)
+
+
 @exceptions.handler
 def run(input_):
 """ Function for piglit run command
@@ -246,17 +254,14 @@ def run(input_):
 
 # If the results directory already exists and if overwrite was set, then
 # clear the directory. If it wasn't set, then raise fatal error.
-if os.path.exists(args.results_path):
-if args.overwrite:
-if os.path.isdir(args.results_path):
-shutil.rmtree(args.results_path)
-else:
-os.unlink(args.results_path)
-else:
-raise exceptions.PiglitFatalError(
-'Cannot overwrite existing folder without the -o/--overwrite '
-'option being set.')
-os.makedirs(args.results_path)
+try:
+core.check_dir(args.results_path,
+   failifexists=args.overwrite,
+   handler=_results_handler)
+except exceptions.PiglitException:
+raise exceptions.PiglitFatalError(
+'Cannot overwrite existing folder without the -o/--overwrite '
+'option being set.')
 
 results = framework.results.TestrunResult()
 backends.set_meta(args.backend, results)
diff --git a/unittests/core_tests.py b/unittests/core_tests.py
index c0ea523..8cbac8e 100644
--- a/unittests/core_tests.py
+++ b/unittests/core_tests.py
@@ -321,3 +321,12 @@ def test_check_dir_makedirs_fail():
 with mock.patch('framework.core.os.makedirs',
 mock.Mock(side_effect=OSError)):
 core.check_dir('foo', False)
+
+
+@nt.raises(utils.SentinalException)
+def test_check_dir_handler():
+"""core.check_dir: Handler is called if not failifexists."""
+with mock.patch('framework.core.os.stat',
+mock.Mock(side_effect=OSError('foo', errno.ENOTDIR))):
+core.check_dir('foo',
+   handler=mock.Mock(side_effect=utils.SentinalException))
-- 
2.8.2

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


[Piglit] [PATCH 06/14] framework: core.checkDir raises PiglitException

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This requires each caller that sets failifexists=True to handle the
error, but it allows them to set a custom message or handle the failure
themselves, rather than just bailing. This makes the function more
generically useful, and removes a layering violation.

Signed-off-by: Dylan Baker 
---
 framework/core.py | 4 +---
 framework/programs/summary.py | 8 +++-
 unittests/core_tests.py   | 2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/framework/core.py b/framework/core.py
index a75e598..d052bd5 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -120,9 +120,7 @@ def checkDir(dirname, failifexists):
 os.stat(dirname)
 except OSError as e:
 if e.errno not in [errno.ENOENT, errno.ENOTDIR] and failifexists:
-raise exceptions.PiglitFatalError(
-"%(dirname)s exists already.\nUse --overwrite if "
-"you want to overwrite it.\n" % locals())
+raise exceptions.PiglitException
 
 try:
 if not os.path.exists(dirname):
diff --git a/framework/programs/summary.py b/framework/programs/summary.py
index b42675f..13368e4 100644
--- a/framework/programs/summary.py
+++ b/framework/programs/summary.py
@@ -101,7 +101,13 @@ def html(input_):
 shutil.rmtree(args.summaryDir)
 
 # If the requested directory doesn't exist, create it or throw an error
-core.checkDir(args.summaryDir, not args.overwrite)
+try:
+core.checkDir(args.summaryDir, not args.overwrite)
+except exceptions.PiglitException:
+raise exceptions.PiglitFatalError(
+'{} already exists.\n'
+'use -o/--overwrite if you want to overwrite it.'.format(
+args.summaryDir))
 
 # Merge args.list and args.resultsFiles
 if args.list:
diff --git a/unittests/core_tests.py b/unittests/core_tests.py
index 043440b..e152336 100644
--- a/unittests/core_tests.py
+++ b/unittests/core_tests.py
@@ -278,7 +278,7 @@ class TestPiglitConfig(object):
 
 
 @utils.capture_stderr
-@nt.raises(exceptions.PiglitFatalError)
+@nt.raises(exceptions.PiglitException)
 def test_check_dir_exists_fail():
 """core.check_dir: if the directory exists and failifexsits is True fail"""
 with mock.patch('framework.core.os.stat', mock.Mock(side_effect=OSError)):
-- 
2.8.2

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


[Piglit] [PATCH 01/14] unittests/core_tests.py: add tests for core.checkDir

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This is groundwork for the rest of the series, which refactors the
checkDir functionality. This has helped to catch regressions in the
behavior.

Signed-off-by: Dylan Baker 
---
 unittests/core_tests.py | 53 +
 1 file changed, 53 insertions(+)

diff --git a/unittests/core_tests.py b/unittests/core_tests.py
index a28ebfb..55cf7e1 100644
--- a/unittests/core_tests.py
+++ b/unittests/core_tests.py
@@ -24,11 +24,20 @@ from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
 import collections
+import errno
 import functools
 import os
 import shutil
 import textwrap
 
+# There is a very high potential that one of these will raise an ImportError
+# pylint: disable=import-error
+try:
+from unittest import mock
+except ImportError:
+import mock
+# pylint: enable=import-error
+
 import nose.tools as nt
 import six
 try:
@@ -266,3 +275,47 @@ class TestPiglitConfig(object):
 def test_safe_get_fallback(self):
 """core.PiglitConfig: safe_get returns the value of fallback when the 
section or option is missing"""
 nt.eq_(self.conf.safe_get('invalid', 'invalid', fallback='foo'), 'foo')
+
+
+@utils.capture_stderr
+@nt.raises(SystemExit)
+def test_check_dir_exists_fail():
+"""core.check_dir: if the directory exists and failifexsits is True fail"""
+with mock.patch('framework.core.os.stat', mock.Mock(side_effect=OSError)):
+core.checkDir('foo', True)
+
+
+def test_check_dir_stat_ENOENT():
+"""core.check_dir: if the directory exists (ENOENT) and failifexsits is 
False continue"""
+with mock.patch('framework.core.os.stat',
+mock.Mock(side_effect=OSError('foo', errno.ENOENT))):
+with mock.patch('framework.core.os.makedirs') as makedirs:
+core.checkDir('foo', False)
+nt.eq_(makedirs.called, 1)
+
+
+def test_check_dir_stat_ENOTDIR():
+"""core.check_dir: if the directory exists (ENOTDIR) and failifexsits is 
False continue"""
+with mock.patch('framework.core.os.stat',
+mock.Mock(side_effect=OSError('foo', errno.ENOTDIR))):
+with mock.patch('framework.core.os.makedirs') as makedirs:
+core.checkDir('foo', False)
+nt.eq_(makedirs.called, 1)
+
+
+@utils.not_raises(OSError)
+def test_check_dir_makedirs_pass():
+"""core.check_dir: If makedirs fails with EEXIST pass"""
+with mock.patch('framework.core.os.stat', mock.Mock()):
+with mock.patch('framework.core.os.makedirs',
+mock.Mock(side_effect=OSError(errno.EEXIST, 'foo'))):
+core.checkDir('foo', False)
+
+
+@nt.raises(OSError)
+def test_check_dir_makedirs_fail():
+"""core.check_dir: If makedirs fails with any other raise"""
+with mock.patch('framework.core.os.stat', mock.Mock()):
+with mock.patch('framework.core.os.makedirs',
+mock.Mock(side_effect=OSError)):
+core.checkDir('foo', False)
-- 
2.8.2

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


[Piglit] [PATCH 03/14] core.py: remove flag from checkDir, use exception block

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This simplifies the function.
---
 framework/core.py | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/framework/core.py b/framework/core.py
index 870a708..c339572 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -117,17 +117,13 @@ def get_config(arg=None):
 
 # Ensure the given directory exists
 def checkDir(dirname, failifexists):
-exists = True
 try:
 os.stat(dirname)
 except OSError as e:
-if e.errno in [errno.ENOENT, errno.ENOTDIR]:
-exists = False
-
-if exists and failifexists:
-print("%(dirname)s exists already.\nUse --overwrite if "
-  "you want to overwrite it.\n" % locals(), file=sys.stderr)
-exit(1)
+if e.errno not in [errno.ENOENT, errno.ENOTDIR] and failifexists:
+print("%(dirname)s exists already.\nUse --overwrite if "
+  "you want to overwrite it.\n" % locals(), file=sys.stderr)
+exit(1)
 
 try:
 os.makedirs(dirname)
-- 
2.8.2

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


[Piglit] [PATCH 05/14] core.py: use PiglitFatalError instead of sys.exit

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This is cleaner and more in keeping with the style of piglit.

Signed-off-by: Dylan Baker 
---
 framework/core.py   | 7 +++
 unittests/core_tests.py | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/framework/core.py b/framework/core.py
index aa21df5..a75e598 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -28,7 +28,6 @@ from __future__ import (
 import errno
 import os
 import subprocess
-import sys
 
 from six.moves import configparser
 
@@ -121,9 +120,9 @@ def checkDir(dirname, failifexists):
 os.stat(dirname)
 except OSError as e:
 if e.errno not in [errno.ENOENT, errno.ENOTDIR] and failifexists:
-print("%(dirname)s exists already.\nUse --overwrite if "
-  "you want to overwrite it.\n" % locals(), file=sys.stderr)
-exit(1)
+raise exceptions.PiglitFatalError(
+"%(dirname)s exists already.\nUse --overwrite if "
+"you want to overwrite it.\n" % locals())
 
 try:
 if not os.path.exists(dirname):
diff --git a/unittests/core_tests.py b/unittests/core_tests.py
index 4b05721..043440b 100644
--- a/unittests/core_tests.py
+++ b/unittests/core_tests.py
@@ -278,7 +278,7 @@ class TestPiglitConfig(object):
 
 
 @utils.capture_stderr
-@nt.raises(SystemExit)
+@nt.raises(exceptions.PiglitFatalError)
 def test_check_dir_exists_fail():
 """core.check_dir: if the directory exists and failifexsits is True fail"""
 with mock.patch('framework.core.os.stat', mock.Mock(side_effect=OSError)):
-- 
2.8.2

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


[Piglit] [PATCH 00/14] framework: Cleanup the core module

2016-05-06 Thread Dylan Baker
I'm trying to clear out my piglit repo of any branches that look like
they might be slightly useful, as I start to migrate my main focus away
from improving the framework and towards maintenance with a focus
elsewhere.

This little series fixes up some of the consistency issues with some of
the oldest code in the codebase. Ultimately this may be split up more
than is necessary and I can squash them if people prefer.

It also adds documentation to modules and functions that lacked it
before.

Note that most of the code additions are unittests rather than changes
in the framework itself.

Dylan Baker (14):
  unittests/core_tests.py: add tests for core.checkDir
  core.py: use 'in list' instead of a == b or a == c...
  core.py: remove flag from checkDir, use exception block
  core.py: only call makedirs if the directory doesn't exist
  core.py: use PiglitFatalError instead of sys.exit
  framework: core.checkDir raises PiglitException
  framework: rename core.checkDir to core.check_dir
  core.py: make second argument to check_dir a keyword argument
  core.py: add docstring to check_dir
  framework/core: extend check_dir to take a handler callable
  framework/core.py: Make copyright header match others in the framework
  framework/core.py: add module docstring
  framework/summary/html_.py: use core.check_dir
  unittests/core_tests.py: Add tests that only apply to python3

 framework/core.py | 91 +++
 framework/programs/run.py | 27 +++--
 framework/programs/summary.py |  8 +++-
 framework/summary/html_.py| 25 +---
 unittests/core_tests.py   | 74 +++
 unittests/utils.py| 25 
 6 files changed, 188 insertions(+), 62 deletions(-)

-- 
2.8.2

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


[Piglit] [PATCH 04/14] core.py: only call makedirs if the directory doesn't exist

2016-05-06 Thread Dylan Baker
From: Dylan Baker 

This isn't exactly idiomatic, but makedirs can be *very slow*,
especially if the path is long and exists completely. By checking before
creating, and recovering from expected errors this function can be fast
and safe.

Signed-off-by: Dylan Baker 
---
 framework/core.py   | 3 ++-
 unittests/core_tests.py | 8 +---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/framework/core.py b/framework/core.py
index c339572..aa21df5 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -126,7 +126,8 @@ def checkDir(dirname, failifexists):
 exit(1)
 
 try:
-os.makedirs(dirname)
+if not os.path.exists(dirname):
+os.makedirs(dirname)
 except OSError as e:
 if e.errno != errno.EEXIST:
 raise
diff --git a/unittests/core_tests.py b/unittests/core_tests.py
index 55cf7e1..4b05721 100644
--- a/unittests/core_tests.py
+++ b/unittests/core_tests.py
@@ -316,6 +316,8 @@ def test_check_dir_makedirs_pass():
 def test_check_dir_makedirs_fail():
 """core.check_dir: If makedirs fails with any other raise"""
 with mock.patch('framework.core.os.stat', mock.Mock()):
-with mock.patch('framework.core.os.makedirs',
-mock.Mock(side_effect=OSError)):
-core.checkDir('foo', False)
+with mock.patch('framework.core.os.path.exists',
+mock.Mock(return_value=False)):
+with mock.patch('framework.core.os.makedirs',
+mock.Mock(side_effect=OSError)):
+core.checkDir('foo', False)
-- 
2.8.2

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


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 v2] cl: Fix cl_khr_fp64 checks

2016-05-06 Thread Jan Vesely
On Fri, 2016-05-06 at 13:38 +0200, Serge Martin wrote:
> On Thursday 28 April 2016 22:22:06 Jan Vesely wrote:
> > 
> > v2: fix sizeof test
> > 
> > Signed-off-by: Jan Vesely 
> > ---
> >  tests/cl/program/build/scalar-data-types.cl | 5 +++--
> >  tests/cl/program/execute/sizeof.cl  | 4 +++-
> >  2 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/cl/program/build/scalar-data-types.cl
> > b/tests/cl/program/build/scalar-data-types.cl index
> > 00693f5..09fd15f 100755
> > --- a/tests/cl/program/build/scalar-data-types.cl
> > +++ b/tests/cl/program/build/scalar-data-types.cl
> > @@ -21,8 +21,9 @@ kernel void test(global int* out) {
> >     uintptr_t uit;
> >     //half h; // Can be only used as a pointer to a buffer
> > 
> > -// Needs cl_khr_fp64 or OpenCL C 1.2
> > -#if __OPENCL_C_VERSION__ >= 120
> > +// Needs cl_khr_fp64
> > +#ifdef cl_khr_fp64
> > +#pragma OPENCL EXTENSION cl_khr_fp64 : require
> pragma OPENCL EXTENSION cl_khr_fp64 : enable
> 
> > 
> >     double d;
> >     double* d1;
> >  #endif
> > diff --git a/tests/cl/program/execute/sizeof.cl
> > b/tests/cl/program/execute/sizeof.cl index 744a59c..aa78590 100644
> > --- a/tests/cl/program/execute/sizeof.cl
> > +++ b/tests/cl/program/execute/sizeof.cl
> > @@ -100,7 +100,9 @@ kernel void so(global int* out) {
> > 
> >     out[i++] = sizeof(half);
> > 
> > -#if __OPENCL_C_VERSION__ >= 120
> > +// Needs cl_khr_fp64
> > +#ifdef cl_khr_fp64
> > +#pragma OPENCL EXTENSION cl_khr_fp64 : require
> enable
> 
> with that fixed : 
> Reviewed-by Serge Martin 

thanks. I went for ocl1.0 compatibility, but I guess the online docs
are too old [0]. Fixed locally and pushed as 5ddb65.

Jan

[0] https://www.khronos.org/registry/cl/sdk/1.0/docs/man/xhtml/EXTENSIO
N.html

> 
>  - Serge
> 
> > 
> >     out[i++] = sizeof(double);
> >     out[i++] = sizeof(double2);
> >     out[i++] = sizeof(double3);
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
-- 
Jan Vesely 


signature.asc
Description: This is a digitally signed message part
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH v2] cl: Fix cl_khr_fp64 checks

2016-05-06 Thread Serge Martin
On Thursday 28 April 2016 22:22:06 Jan Vesely wrote:
> v2: fix sizeof test
> 
> Signed-off-by: Jan Vesely 
> ---
>  tests/cl/program/build/scalar-data-types.cl | 5 +++--
>  tests/cl/program/execute/sizeof.cl  | 4 +++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/cl/program/build/scalar-data-types.cl
> b/tests/cl/program/build/scalar-data-types.cl index 00693f5..09fd15f 100755
> --- a/tests/cl/program/build/scalar-data-types.cl
> +++ b/tests/cl/program/build/scalar-data-types.cl
> @@ -21,8 +21,9 @@ kernel void test(global int* out) {
>   uintptr_t uit;
>   //half h; // Can be only used as a pointer to a buffer
> 
> -// Needs cl_khr_fp64 or OpenCL C 1.2
> -#if __OPENCL_C_VERSION__ >= 120
> +// Needs cl_khr_fp64
> +#ifdef cl_khr_fp64
> +#pragma OPENCL EXTENSION cl_khr_fp64 : require

pragma OPENCL EXTENSION cl_khr_fp64 : enable

>   double d;
>   double* d1;
>  #endif
> diff --git a/tests/cl/program/execute/sizeof.cl
> b/tests/cl/program/execute/sizeof.cl index 744a59c..aa78590 100644
> --- a/tests/cl/program/execute/sizeof.cl
> +++ b/tests/cl/program/execute/sizeof.cl
> @@ -100,7 +100,9 @@ kernel void so(global int* out) {
> 
>   out[i++] = sizeof(half);
> 
> -#if __OPENCL_C_VERSION__ >= 120
> +// Needs cl_khr_fp64
> +#ifdef cl_khr_fp64
> +#pragma OPENCL EXTENSION cl_khr_fp64 : require

enable

with that fixed : 
Reviewed-by Serge Martin 

 - Serge

>   out[i++] = sizeof(double);
>   out[i++] = sizeof(double2);
>   out[i++] = sizeof(double3);

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


[Piglit] [PATCH 1/2] cl: add image support checking functions

2016-05-06 Thread Serge Martin
---
 tests/util/piglit-util-cl.c | 28 
 tests/util/piglit-util-cl.h | 18 ++
 2 files changed, 46 insertions(+)

diff --git a/tests/util/piglit-util-cl.c b/tests/util/piglit-util-cl.c
index efa289c..51d6808 100644
--- a/tests/util/piglit-util-cl.c
+++ b/tests/util/piglit-util-cl.c
@@ -1004,6 +1004,34 @@ piglit_cl_read_whole_buffer(cl_command_queue 
command_queue, cl_mem buffer,
return success;
 }
 
+bool
+piglit_cl_get_context_image_support(const piglit_cl_context context)
+{
+   bool ret = false;
+
+   unsigned i;
+   for(i = 0; i < context->num_devices; i++)
+   ret |= 
piglit_cl_get_device_image_support(context->device_ids[i]);
+
+   return ret;
+}
+
+bool
+piglit_cl_get_device_image_support(cl_device_id device)
+{
+   bool ret = false;
+
+   cl_bool *image_support =
+   piglit_cl_get_device_info(device, CL_DEVICE_IMAGE_SUPPORT);
+
+   if (image_support)
+   ret = *image_support;
+
+   free(image_support);
+
+   return ret;
+}
+
 cl_mem
 piglit_cl_create_image(piglit_cl_context context, cl_mem_flags flags,
const cl_image_format *format,
diff --git a/tests/util/piglit-util-cl.h b/tests/util/piglit-util-cl.h
index 0330740..e4730cc 100644
--- a/tests/util/piglit-util-cl.h
+++ b/tests/util/piglit-util-cl.h
@@ -545,6 +545,24 @@ typedef struct {
 } piglit_image_desc;
 #endif
 
+/**
+ * \brief Get context image support.
+ *
+ * @param context  Context on which to create image.
+ * @return True if there is one device with image support.
+ */
+bool
+piglit_cl_get_context_image_support(const piglit_cl_context context);
+
+/**
+ * \brief Get device image support.
+ *
+ * @param device   Context on which to create image.
+ * @return True if there the device has image support.
+ */
+bool
+piglit_cl_get_device_image_support(cl_device_id device);
+
 
 /**
  * \brief Create an image.
-- 
2.5.5

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


[Piglit] [PATCH 2/2] cl: check for image support using util/ functions

2016-05-06 Thread Serge Martin
---
 tests/cl/api/create-image.c   | 19 +--
 tests/cl/api/create-sampler.c | 18 +-
 tests/cl/api/enqueue-fill-image.c | 10 +-
 tests/cl/api/get-image-info.c | 19 +--
 tests/cl/api/set-kernel-arg.c | 18 +-
 5 files changed, 5 insertions(+), 79 deletions(-)

diff --git a/tests/cl/api/create-image.c b/tests/cl/api/create-image.c
index 1ee5f71..29e552c 100644
--- a/tests/cl/api/create-image.c
+++ b/tests/cl/api/create-image.c
@@ -38,23 +38,6 @@ PIGLIT_CL_API_TEST_CONFIG_BEGIN
 
 PIGLIT_CL_API_TEST_CONFIG_END
 
-static bool context_has_image_support(const piglit_cl_context ctx)
-{
-   unsigned i;
-   for(i = 0; i < ctx->num_devices; i++) {
-   int *image_support =
-   piglit_cl_get_device_info(ctx->device_ids[i],
-   CL_DEVICE_IMAGE_SUPPORT);
-   if (*image_support) {
-   free(image_support);
-   return true;
-   }
-
-   free(image_support);
-   }
-   return false;
-}
-
 static void
 no_image_check_invalid(
cl_int errcode_ret,
@@ -107,7 +90,7 @@ piglit_cl_test(const int argc,
const struct piglit_cl_api_test_config* config,
const struct piglit_cl_api_test_env* env)
 {
-   if (!context_has_image_support(env->context)) {
+   if (!piglit_cl_get_context_image_support(env->context)) {
return no_image_tests(env);
} else {
return PIGLIT_PASS;
diff --git a/tests/cl/api/create-sampler.c b/tests/cl/api/create-sampler.c
index dcdef05..d51fe47 100644
--- a/tests/cl/api/create-sampler.c
+++ b/tests/cl/api/create-sampler.c
@@ -36,22 +36,6 @@ PIGLIT_CL_API_TEST_CONFIG_BEGIN
 
 PIGLIT_CL_API_TEST_CONFIG_END
 
-static bool context_has_image_support(const piglit_cl_context ctx)
-{
-   unsigned i;
-   for(i = 0; i < ctx->num_devices; i++) {
-   int *image_support =
-   piglit_cl_get_device_info(ctx->device_ids[i],
-   CL_DEVICE_IMAGE_SUPPORT);
-   if (*image_support) {
-   free(image_support);
-   return true;
-   }
-   free(image_support);
-   }
-   return false;
-}
-
 static enum piglit_result
 no_image_tests(const struct piglit_cl_api_test_env* env)
 {
@@ -80,7 +64,7 @@ piglit_cl_test(const int argc,
const struct piglit_cl_api_test_config* config,
const struct piglit_cl_api_test_env* env)
 {
-   if (!context_has_image_support(env->context)) {
+   if (!piglit_cl_get_context_image_support(env->context)) {
return no_image_tests(env);
} else {
return PIGLIT_PASS;
diff --git a/tests/cl/api/enqueue-fill-image.c 
b/tests/cl/api/enqueue-fill-image.c
index 2839b67..4de5dca 100644
--- a/tests/cl/api/enqueue-fill-image.c
+++ b/tests/cl/api/enqueue-fill-image.c
@@ -104,19 +104,11 @@ piglit_cl_test(const int argc,
cl_command_queue queue = env->context->command_queues[0];
int i;
 
-   cl_bool *image_support =
-   piglit_cl_get_device_info(env->context->device_ids[0],
- CL_DEVICE_IMAGE_SUPPORT);
-
-   if (!*image_support) {
+   if (!piglit_cl_get_device_image_support(env->context->device_ids[0])) {
fprintf(stderr, "No image support\n");
-   free(image_support);
return PIGLIT_SKIP;
}
 
-   free(image_support);
-   image_support = NULL;
-
img_format.image_channel_order = CL_RGBA;
img_format.image_channel_data_type = CL_UNSIGNED_INT8;
img_desc.image_type = CL_MEM_OBJECT_IMAGE2D;
diff --git a/tests/cl/api/get-image-info.c b/tests/cl/api/get-image-info.c
index a8b5bec..d4dc842 100644
--- a/tests/cl/api/get-image-info.c
+++ b/tests/cl/api/get-image-info.c
@@ -46,23 +46,6 @@ PIGLIT_CL_API_TEST_CONFIG_BEGIN
 
 PIGLIT_CL_API_TEST_CONFIG_END
 
-static bool context_has_image_support(const piglit_cl_context ctx)
-{
-   int ret = 0;
-   unsigned i;
-   for(i = 0; i < ctx->num_devices; i++) {
-   int *image_support =
-   piglit_cl_get_device_info(ctx->device_ids[i],
-   CL_DEVICE_IMAGE_SUPPORT);
-   if (image_support)
-   ret |= *image_support;
-
-   free(image_support);
-   }
-   return ret;
-}
-
-
 enum piglit_result
 piglit_cl_test(const int argc,
const char** argv,
@@ -79,7 +62,7 @@ piglit_cl_test(const int argc,
.image_channel_data_type = CL_FLOAT,
};
 
-   if (!context_has_image_support(env->context)) {
+   if (!piglit_cl_get_context_image_support(env->context)) {
fprintf(stderr, "No device with image support