[Piglit] [Patch v2 04/13] framework: put GleanTest class in exectest module
Ilia Mirkin
imirkin at alum.mit.edu
Tue May 13 12:14:28 PDT 2014
On Tue, May 13, 2014 at 3:06 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Tue, May 13, 2014 at 2:38 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
>> This is the first step to merging the builtin test classes into a single
>> module. It also fixes some problems with GleanTest.interpret_result()
Oh, also, something that occurred to me when looking at the GLSL
parser test, but applies just as much here... why are you doing this
in the first place? It seems like this breaks the abstraction between
"test framework" and "tests" which is otherwise maintained. IMHO the
glean/glsl test stuff belongs in tests/.
>>
>> This also merges the unit tests for glean into the exectest_tests.py
>> module.
>>
>> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
>> ---
>> framework/exectest.py | 21 +++++++++++
>> framework/gleantest.py | 46 ------------------------
>> framework/tests/dmesg_tests.py | 3 +-
>> framework/tests/exectest_test.py | 50 ++++++++++++++++++++++++++-
>> framework/tests/gleantest_tests.py | 71 --------------------------------------
>> tests/all.py | 3 +-
>> tests/quick.py | 2 +-
>> tests/sanity.py | 2 +-
>> 8 files changed, 74 insertions(+), 124 deletions(-)
>> delete mode 100644 framework/gleantest.py
>> delete mode 100644 framework/tests/gleantest_tests.py
>>
>> diff --git a/framework/exectest.py b/framework/exectest.py
>> index c863d20..9131d08 100644
>> --- a/framework/exectest.py
>> +++ b/framework/exectest.py
>> @@ -33,6 +33,7 @@ from .core import TestResult, Environment
>>
>> __all__ = ['Test',
>> 'PiglitTest',
>> + 'GleanTest',
>> 'TEST_BIN_DIR']
>>
>> # Platform global variables
>> @@ -47,6 +48,8 @@ else:
>> TEST_BIN_DIR = os.path.normpath(os.path.join(os.path.dirname(__file__),
>> '../bin'))
>>
>> +glean_executable = os.path.join(TEST_BIN_DIR, "glean")
>
> Minor comment, but I'd have stuck this into the GleanTest class as a
> class-level var.
>
>> +
>>
>> class Test(object):
>> ENV = Environment()
>> @@ -288,3 +291,21 @@ class PiglitTest(Test):
>> self.result.update(eval(piglit))
>> self.result['out'] = '\n'.join(
>> s for s in outlines if not s.startswith('PIGLIT:'))
>> +
>> +
>> +class GleanTest(Test):
>> + globalParams = []
>> +
>> + def __init__(self, name, **kwargs):
>> + super(GleanTest, self).__init__([glean_executable, "-o", "-v", "-v",
>> + "-v", "-t", "+" + name])
>> +
>> + @Test.command.getter
>> + def command(self):
>> + return super(GleanTest, self).command + self.globalParams
>> +
>> + def interpret_result(self):
>> + if 'FAIL' in self.result['out'] or self.result['returncode'] != 0:
>
> You swapped the order of these from the original version... not sure
> why. Doesn't really matter either way, but thought I'd point it out.
> (The common case is 'pass' so you'll incur the cost of both of the
> tests most of the time anyways.)
>
>> + self.result['result'] = 'fail'
>> + else:
>> + self.result['result'] = 'pass'
>> diff --git a/framework/gleantest.py b/framework/gleantest.py
>> deleted file mode 100644
>> index 5229cf5..0000000
>> --- a/framework/gleantest.py
>> +++ /dev/null
>> @@ -1,46 +0,0 @@
>> -#
>> -# 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.
>> -
>> -import os
>> -
>> -from .exectest import Test, TEST_BIN_DIR
>> -
>> -glean_executable = os.path.join(TEST_BIN_DIR, "glean")
>> -
>> -# GleanTest: Execute a sub-test of Glean
>> -class GleanTest(Test):
>> - globalParams = []
>> -
>> - def __init__(self, name, **kwargs):
>> - super(GleanTest, self).__init__([glean_executable, "-o", "-v", "-v",
>> - "-v", "-t", "+" + name])
>> -
>> - @Test.command.getter
>> - def command(self):
>> - return super(GleanTest, self).command + self.globalParams
>> -
>> - def interpret_result(self):
>> - if self.result['returncode'] != 0 or 'FAIL' in self.result['out']:
>> - self.result['result'] = 'fail'
>> - else:
>> - self.result['result'] = 'pass'
>> diff --git a/framework/tests/dmesg_tests.py b/framework/tests/dmesg_tests.py
>> index ccc3144..8c2b20b 100644
>> --- a/framework/tests/dmesg_tests.py
>> +++ b/framework/tests/dmesg_tests.py
>> @@ -28,8 +28,7 @@ import nose.tools as nt
>> from nose.plugins.skip import SkipTest
>> from framework.dmesg import DummyDmesg, LinuxDmesg, get_dmesg
>> from framework.core import TestResult, PiglitJSONEncoder
>> -from framework.exectest import PiglitTest
>> -from framework.gleantest import GleanTest
>> +from framework.exectest import PiglitTest, GleanTest
>> from framework.shader_test import ShaderTest
>> from framework.glsl_parser_test import GLSLParserTest
>>
>> diff --git a/framework/tests/exectest_test.py b/framework/tests/exectest_test.py
>> index 2f0569f..5875b00 100644
>> --- a/framework/tests/exectest_test.py
>> +++ b/framework/tests/exectest_test.py
>> @@ -20,7 +20,10 @@
>>
>> """ Tests for the exectest module """
>>
>> -from framework.exectest import PiglitTest, Test
>> +from __future__ import print_function
>> +import os
>> +from nose.plugins.skip import SkipTest
>> +from framework.exectest import PiglitTest, Test, GleanTest
>>
>>
>> def test_initialize_test():
>> @@ -31,3 +34,48 @@ def test_initialize_test():
>> def test_initialize_piglittest():
>> """ Test that PiglitTest initializes correctly """
>> PiglitTest('/bin/true')
>> +
>> +
>> +def test_initialize_gleantest():
>> + """ Test that GleanTest initilizes """
>> + test = GleanTest('name')
>> + assert test
>> +
>> +
>> +def test_globalParams_assignment():
>> + """ Test to ensure that GleanTest.globalParams are correctly assigned
>> +
>> + Specifically this tests for a bug where globalParams only affected
>> + instances of GleanTest created after globalParams were set, so changing the
>> + globalParams value had unexpected results.
>> +
>> + If this test passes the GleanTest.command attributes will be the same in
>> + the instance created before the globalParams assignment and the one created
>> + after. A failure means the that globalParams are not being added to tests
>> + initialized before it is set.
>> +
>> + """
>> + test1 = GleanTest('basic')
>> + GleanTest.globalParams = ['--quick']
>> + test2 = GleanTest('basic')
>> + assert test1.command == test2.command
>> +
>> +
>> +def test_bad_returncode():
>> + """ If glean doesn't return zero it should be a fail
>> +
>> + Currently clean returns 127 if piglit can't find it's libs (LD_LIBRARY_PATH
>> + isn't set properly), and then marks such tests as pass, when they obviously
>> + are not.
>> +
>> + """
>> + if not os.path.exists('bin'):
>> + raise SkipTest("This test requires a working, built version of piglit")
>> +
>> + # Clearing the environment should remove the piglit/lib from
>> + # LD_LIBRARY_PATH
>> + os.environ = {}
>> +
>> + test = GleanTest('basic')
>> + test.run()
>> + assert test.result['result'] == 'fail', "Result should have been fail"
>> diff --git a/framework/tests/gleantest_tests.py b/framework/tests/gleantest_tests.py
>> deleted file mode 100644
>> index 20aa647..0000000
>> --- a/framework/tests/gleantest_tests.py
>> +++ /dev/null
>> @@ -1,71 +0,0 @@
>> -# Copyright (c) 2014 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.
>> -
>> -""" Tests for the glean class. Requires Nose """
>> -
>> -from __future__ import print_function
>> -import os
>> -from nose.plugins.skip import SkipTest
>> -from framework.gleantest import GleanTest
>> -
>> -
>> -def test_initialize_gleantest():
>> - """ Test that GleanTest initilizes """
>> - test = GleanTest('name')
>> - assert test
>> -
>> -
>> -def test_globalParams_assignment():
>> - """ Test to ensure that GleanTest.globalParams are correctly assigned
>> -
>> - Specifically this tests for a bug where globalParams only affected
>> - instances of GleanTest created after globalParams were set, so changing the
>> - globalParams value had unexpected results.
>> -
>> - If this test passes the GleanTest.command attributes will be the same in
>> - the instance created before the globalParams assignment and the one created
>> - after. A failure means the that globalParams are not being added to tests
>> - initialized before it is set.
>> -
>> - """
>> - test1 = GleanTest('basic')
>> - GleanTest.globalParams = ['--quick']
>> - test2 = GleanTest('basic')
>> - assert test1.command == test2.command
>> -
>> -
>> -def test_bad_returncode():
>> - """ If glean doesn't return zero it should be a fail
>> -
>> - Currently clean returns 127 if piglit can't find it's libs (LD_LIBRARY_PATH
>> - isn't set properly), and then marks such tests as pass, when they obviously
>> - are not.
>> -
>> - """
>> - if not os.path.exists('bin'):
>> - raise SkipTest("This test requires a working, built version of piglit")
>> -
>> - # Clearing the environment should remove the piglit/lib from
>> - # LD_LIBRARY_PATH
>> - os.environ = {}
>> -
>> - test = GleanTest('basic')
>> - test.run()
>> - assert test.result['result'] == 'fail', "Result should have been fail"
>> diff --git a/tests/all.py b/tests/all.py
>> index a67e725..23bf702 100644
>> --- a/tests/all.py
>> +++ b/tests/all.py
>> @@ -10,8 +10,7 @@ import platform
>> import shlex
>>
>> from framework.profile import TestProfile
>> -from framework.exectest import PiglitTest
>> -from framework.gleantest import GleanTest
>> +from framework.exectest import PiglitTest, GleanTest
>> from framework.glsl_parser_test import GLSLParserTest, add_glsl_parser_test, import_glsl_parser_tests
>> from framework.shader_test import add_shader_test_dir
>>
>> diff --git a/tests/quick.py b/tests/quick.py
>> index 939aded..53b4eb3 100644
>> --- a/tests/quick.py
>> +++ b/tests/quick.py
>> @@ -1,6 +1,6 @@
>> # -*- coding: utf-8 -*-
>>
>> -from framework.gleantest import GleanTest
>> +from framework.exectest import GleanTest
>> from tests.all import profile
>>
>> __all__ = ['profile']
>> diff --git a/tests/sanity.py b/tests/sanity.py
>> index 0e0e038..8e5c81f 100644
>> --- a/tests/sanity.py
>> +++ b/tests/sanity.py
>> @@ -3,7 +3,7 @@
>> #
>>
>> from framework.profile import TestProfile
>> -from framework.gleantest import GleanTest
>> +from framework.exectest import GleanTest
>>
>> __all__ = ['profile']
>>
>> --
>> 2.0.0.rc2
>>
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list