[Piglit] [PATCH 24/24] Test: don't accept strings for command arguments.
Dylan Baker
baker.dylan.c at gmail.com
Mon Jan 5 13:50:45 PST 2015
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 <dylanx.c.baker at intel.com>
---
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
More information about the Piglit
mailing list