On 8 September 2011 23:07, Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">--- tests/all.tests | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/all.tests b/tests/all.tests index 03dcd59..fc66ff6 100644 --- a/tests/all.tests +++ b/tests/all.tests @@ -25,6 +25,12 @@ generatedTestDir = os.path.join( os.path.join(os.path.dirname(__file__), '..')), 'generated_tests') +# Quick wrapper for PlainExecTest for our usual concurrent args. +def concurrent_test(args): + test = PlainExecTest(args + " -auto -fbo") + test.runConcurrent = True + return test + ###### # Collecting all tests profile = TestProfile() @@ -293,12 +299,6 @@ add_fbo_depthstencil_tests(general, 'default_fb') shaders = Group() -def add_shader_test(group, filepath, test_name): - """Add a shader test to the given group.""" - group[test_name] = PlainExecTest(['shader_runner', '-auto', '-fbo', - filepath]) - group[test_name].runConcurrent = True - def add_shader_test_dir(group, dirpath, recursive=False): """Add all shader tests in a directory to the given group.""" for filename in os.listdir(dirpath): @@ -314,7 +314,7 @@ def add_shader_test_dir(group, dirpath, recursive=False): if ext != 'shader_test': continue testname = filename[0:-(len(ext) + 1)] # +1 for '.' - add_shader_test(group, filepath, testname) + group[testname] = concurrent_test('shader_runner ' + filepath) def add_getactiveuniform_count(group, name, expected): path = 'shaders/' -- 1.7.5.4 _______________________________________________ Piglit mailing list <a href="mailto:Piglit@lists.freedesktop.org" target="_blank">Piglit@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote></div> This patch introduces a limitation to add_shader_test_dir(): it no longer works properly when a test name or directory contains spaces. Although spaces in file/directory names are generally frowned upon in the UNIX world (and Piglit currently doesn't contain any such files or directories), I don't think that's any excuse to make the code fragile. In addition, the new function concurrent_test() has the limitation that it can *only* be called with a string, not a list of strings. Since the intended purpose of concurrent_test() is to take the place of ExecTest() for tests that should be executed concurrently, it would avoid confusion if it accepted inputs in the same form that ExecTest() accepts. In my experience, the best way to avoid issues like these is to convert from a space-separated string to a list as early as possible (at the top of concurrent_test()), and then manipulate everything as lists of strings thereafter.