One of the oddities of piglit's Test derived classes has always been the ability to use either a string or a list for arguments. This makes sense and is nice when the command is a single value (say, 'fbo-blit-test'), but it quickly becomes difficult to work with when multiple space separated arguments need to be passed, requiring the use of shlex.split() to be handled correctly. shlex.split is an OS dependent module, and doesn't have the same behavior on windows and Unices (even Unices are guaranteed to be exactly the same). The best solution is to only use lists, period.
This fixes bugs seen on cygwin. Also fixes unit tests, and adds an additional unit test that checks that strings are rejected. Signed-off-by: Dylan Baker <[email protected]> --- framework/test/base.py | 11 ++--------- framework/tests/base_tests.py | 8 ++++---- framework/tests/gtest_tests.py | 2 +- framework/tests/piglit_test_tests.py | 22 +++++++++++----------- tests/all.py | 1 + 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/framework/test/base.py b/framework/test/base.py index d92d4c8..efc20cb 100644 --- a/framework/test/base.py +++ b/framework/test/base.py @@ -26,7 +26,6 @@ from __future__ import print_function, absolute_import import errno import os import subprocess -import shlex import time import sys import traceback @@ -123,7 +122,8 @@ class Test(object): timeout = 0 def __init__(self, command, run_concurrent=False): - self._command = None + assert isinstance(command, list), command + self.run_concurrent = run_concurrent self._command = copy.copy(command) self.env = {} @@ -178,13 +178,6 @@ class Test(object): '--tool=memcheck'] + self._command return self._command - @command.setter - def command(self, value): - if isinstance(value, basestring): - self._command = shlex.split(str(value)) - return - self._command = value - @abc.abstractmethod def interpret_result(self): """ Convert the raw output of the test into a form piglit understands diff --git a/framework/tests/base_tests.py b/framework/tests/base_tests.py index c800634..f4b4ad6 100644 --- a/framework/tests/base_tests.py +++ b/framework/tests/base_tests.py @@ -63,7 +63,7 @@ def test_timeout(): if (test.result['returncode'] == 0): test.result['result'] = "pass" - test = TestTest("sleep 60") + test = TestTest(['sleep', '60']) test.test_interpret_result = helper test.timeout = 1 test.run() @@ -78,7 +78,7 @@ def test_timeout_pass(): if (test.result['returncode'] == 0): test.result['result'] = "pass" - test = TestTest("true") + test = TestTest(['true']) test.test_interpret_result = helper test.timeout = 1 test.run() @@ -108,7 +108,7 @@ def test_WindowResizeMixin_rerun(): def interpret_result(self): pass - test = Test_('foo') + test = Test_(['foo']) test.run() nt.assert_equal(test.result['out'], 'all good') @@ -123,7 +123,7 @@ def test_run_command_early(): def _run_command(self): return True - test = Test_('foo') + test = Test_(['foo']) test.run() diff --git a/framework/tests/gtest_tests.py b/framework/tests/gtest_tests.py index d8c8b64..2a90cf9 100644 --- a/framework/tests/gtest_tests.py +++ b/framework/tests/gtest_tests.py @@ -27,5 +27,5 @@ from framework.test import GTest def test_initialize_gtest(): """ Test that GTest successfully initializes correctly """ - test = GTest('/bin/true') + test = GTest(['/bin/true']) assert test diff --git a/framework/tests/piglit_test_tests.py b/framework/tests/piglit_test_tests.py index 9834a1b..764a9c6 100644 --- a/framework/tests/piglit_test_tests.py +++ b/framework/tests/piglit_test_tests.py @@ -31,7 +31,7 @@ from framework.test.piglit_test import (PiglitBaseTest, PiglitGLTest, def test_initialize_piglitgltest(): """ Test that PiglitGLTest initializes correctly """ try: - PiglitGLTest('/bin/true') + PiglitGLTest(['/bin/true']) except Exception as e: raise AssertionError(e) @@ -39,14 +39,14 @@ def test_initialize_piglitgltest(): def test_initialize_piglitcltest(): """ Test that PiglitCLTest initializes correctly """ try: - PiglitCLTest('/bin/true') + PiglitCLTest(['/bin/true']) except Exception as e: raise AssertionError(e) def test_piglittest_interpret_result(): """ PiglitBaseTest.interpret_result() works no subtests """ - test = PiglitBaseTest('foo') + test = PiglitBaseTest(['foo']) test.result['out'] = 'PIGLIT: {"result": "pass"}\n' test.interpret_result() assert test.result['result'] == 'pass' @@ -54,7 +54,7 @@ def test_piglittest_interpret_result(): def test_piglittest_interpret_result_subtest(): """ PiglitBaseTest.interpret_result() works with subtests """ - test = PiglitBaseTest('foo') + test = PiglitBaseTest(['foo']) test.result['out'] = ('PIGLIT: {"result": "pass"}\n' 'PIGLIT: {"subtest": {"subtest": "pass"}}\n') test.interpret_result() @@ -77,13 +77,13 @@ def test_piglitest_no_clobber(): def test_piglittest_command_getter_serial(): """ PiglitGLTest.command adds -auto to serial tests """ - test = PiglitGLTest('foo') + test = PiglitGLTest(['foo']) nt.assert_in('-auto', test.command) def test_piglittest_command_getter_concurrent(): """ PiglitGLTest.command adds -fbo and -auto to concurrent tests """ - test = PiglitGLTest('foo', run_concurrent=True) + test = PiglitGLTest(['foo'], run_concurrent=True) nt.assert_in('-auto', test.command) nt.assert_in('-fbo', test.command) @@ -91,7 +91,7 @@ def test_piglittest_command_getter_concurrent(): def test_PiglitGLTest_include_and_exclude(): """PiglitGLTest.is_skip() raises if include and exclude are given.""" with nt.assert_raises(AssertionError): - PiglitGLTest('foo', + PiglitGLTest(['foo'], require_platforms=['glx'], exclude_platforms=['gbm']) @@ -99,26 +99,26 @@ def test_PiglitGLTest_include_and_exclude(): def test_PiglitGLTest_platform_in_require(): """PiglitGLTest.is_skip() does not skip if platform is in require_platforms.""" PiglitGLTest.OPTS.env['PIGLIT_PLATFORM'] = 'glx' - test = PiglitGLTest('foo', require_platforms=['glx']) + test = PiglitGLTest(['foo'], require_platforms=['glx']) nt.assert_false(test.is_skip()) def test_PiglitGLTest_platform_not_in_require(): """PiglitGLTest.is_skip() skips if platform is not in require_platforms.""" PiglitGLTest.OPTS.env['PIGLIT_PLATFORM'] = 'gbm' - test = PiglitGLTest('foo', require_platforms=['glx']) + test = PiglitGLTest(['foo'], require_platforms=['glx']) nt.assert_true(test.is_skip()) def test_PiglitGLTest_platform_in_exclude(): """PiglitGLTest.is_skip() skips if platform is in exclude_platforms..""" PiglitGLTest.OPTS.env['PIGLIT_PLATFORM'] = 'glx' - test = PiglitGLTest('foo', exclude_platforms=['glx']) + test = PiglitGLTest(['foo'], exclude_platforms=['glx']) nt.assert_true(test.is_skip()) def test_PiglitGLTest_platform_not_in_exclude(): """PiglitGLTest.is_skip() does not skip if platform is in exclude_platforms.""" PiglitGLTest.OPTS.env['PIGLIT_PLATFORM'] = 'gbm' - test = PiglitGLTest('foo', exclude_platforms=['glx']) + test = PiglitGLTest(['foo'], exclude_platforms=['glx']) nt.assert_false(test.is_skip()) diff --git a/tests/all.py b/tests/all.py index 7319705..6efa827 100644 --- a/tests/all.py +++ b/tests/all.py @@ -1678,6 +1678,7 @@ arb_texture_multisample = {} spec['ARB_texture_multisample'] = arb_texture_multisample add_concurrent_test(arb_texture_multisample, ['arb_texture_multisample-minmax']) for sample_count in MSAA_SAMPLE_COUNTS: + sample_count = str(sample_count) # fb-completeness spec[grouptools.join('ARB_texture_multisample', 'fb-completeness', sample_count)] = \ PiglitGLTest(['arb_texture_multisample-fb-completeness', sample_count], run_concurrent=True) -- 2.2.1 _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
