Re: [Piglit] [PATCH 4/4] piglit-print-commands: Add --format option
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
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
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
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
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
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
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
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
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...
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
--- 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
--- 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