[Piglit] [PATCH] oclconform: Add test class for ocl conformance tests v2

Tue Aug 19 09:26:27 PDT 2014

Hi Tom, I have a couple of comments, but this looks pretty good

On Tuesday, August 19, 2014 10:33:42 AM Tom Stellard wrote:
> v2:
>   - Code cleanups
> ---
> 
> Hi Dylan,
> 
> Here is an updated patch addressing your comments.  However, I was not able
> to get the constructor to work using args, kwargs.
> 
> class OCLConform(Test):
>     def __init__(self, *args, **kwargs):
>         Test.__init__(self, *args, **kwargs)
> 
> subtest_command='run_test'
> 
> # When I call the constructor this way and then dump the value of command from
> # The Test construction, I get:
> # ('run_test',)
> OCLConform(subtest_command)
> 
> # When I call the constructor this way, the value of command is ()
> OCLConform(command=subtest_command)
> 
> # The expected value of command is 'run_test'

I feel silly now, I wasn't reading your code close enough. Just delete
your __init__ method completely and everything should work correctly.

> 
> -Tom
> 
>  framework/oclconform.py | 92 +++++++++++++++++++++++++++++++++++++++++++++++++
>  piglit.conf.example     | 34 ++++++++++++++++++
>  2 files changed, 126 insertions(+)
>  create mode 100644 framework/oclconform.py
> 
> diff --git a/framework/oclconform.py b/framework/oclconform.py
> new file mode 100644
> index 0000000..cd8e2a7
> --- /dev/null
> +++ b/framework/oclconform.py
> @@ -0,0 +1,92 @@
> +# Copyright 2014 Advanced Micro Devices, Inc.
> +#
> +# 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 (including the next
> +# paragraph) 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.
> +#
> +# Authors: Tom Stellard <thomas.stellard at amd.com>
> +#
> +
> +from __future__ import print_function
> +
> +import re
> +import subprocess
> +from os import listdir
> +from os.path import isfile, join

It doesn't look like you're using listdir or isfile, so you could drop
them

> +from sys import stderr

You either need to change this to just 'import sys' or see my next
comment

> +
> +from framework.core import PIGLIT_CONFIG
> +from framework.exectest import Test
> +
> +def get_test_section_name(test):
> +    return 'oclconform-{}'.format(test)
> +
> +class OCLConform(Test):
> +    def __init__(self, commands, run_concurrent=False):
> +        Test.__init__(self, commands, run_concurrent)
> +
> +    def interpret_result(self):
> +        if self.result['returncode'] != 0 or 'FAIL' in self.result['out']:
> +            self.result['result'] = 'fail'
> +        else:
> +            self.result['result'] = 'pass'
> +
> +def add_sub_test(profile, test_name, subtest_name, subtest):
> +    profile.tests['oclconform/{}/{}'.format(test_name, subtest_name)] = subtest
> +
> +def add_test(profile, test_name, test):
> +    profile.tests['oclconform/{}'.format(test_name)] = test
> +
> +def add_oclconform_tests(profile):
> +    section_name = 'oclconform'
> +    if not PIGLIT_CONFIG.has_section(section_name):
> +        return
> +
> +    bindir = PIGLIT_CONFIG.get(section_name, 'bindir')
> +    options = PIGLIT_CONFIG.options(section_name)
> +
> +    tests = (o for o in options if PIGLIT_CONFIG.get(section_name, o) is None)
> +
> +    for test in tests:
> +        test_section_name = get_test_section_name(test)
> +        if not PIGLIT_CONFIG.has_section(test_section_name):
> +            print("Warning: no section defined for {}".format(test), file=sys.stderr)

This is the second comment, if you decide to use 'import sys' ignore
this. Otherwise you need to change this to file=stderr, since you didn't
import the module, just the stderr instance into this namespace

This really needs to be fixed or when you hit this warning you'll
get NameError and the program will die.

> +            continue
> +
> +        test_name = PIGLIT_CONFIG.get(test_section_name, 'test_name')
> +        should_run_concurrent = PIGLIT_CONFIG.has_option(test_section_name, 'concurrent')
> +        if PIGLIT_CONFIG.has_option(test_section_name, 'list_subtests'):
> +            # Test with subtests
> +            list_tests = PIGLIT_CONFIG.get(test_section_name, 'list_subtests')
> +            subtest_regex = PIGLIT_CONFIG.get(test_section_name, 'subtest_regex')
> +            subtest_regex.encode('string_escape')
> +            run_subtests = PIGLIT_CONFIG.get(test_section_name, 'run_subtest')
> +            list_tests =list_tests.split()
> +
> +            subtests = subprocess.check_output(args=list_tests, cwd=bindir).split('\n')
> +            for subtest in subtests:
> +                m = re.match(subtest_regex, subtest)
> +                if not m:
> +                    continue
> +                subtest = m.group(1)
> +                subtest_command = join(bindir, run_subtests.replace('<subtest>', subtest))
> +                add_sub_test(profile, test_name, subtest, OCLConform(subtest_command, should_run_concurrent))
> +        else:
> +            run_test = PIGLIT_CONFIG.get(test_section_name, 'run_test')
> +            add_test(profile, test_name, OCLConform(run_test, should_run_concurrent))
> +
> diff --git a/piglit.conf.example b/piglit.conf.example
> index 61a28cf..bdf27aa 100644
> --- a/piglit.conf.example
> +++ b/piglit.conf.example
> @@ -14,3 +14,37 @@
>  [oglconform]
>  ; Set bindir equal to the absolute root of the oglconform directory
>  ;path=/home/usr/src/oglconform
> +
> +[oclconform]
> +; bindir is the directory that the commands to run tests and list subtests
> +; will be executed in.
> +bindir=/home/usr/oclconform
> +; List the tests you want to run
> +testA
> +testB
> +
> +; Section for specific oclconform test.  One of these sections is required for
> +; each test list in the oclconform section and must be called:
> +; oclconform-$testname
> +[oclconform-testA]
> +test_name=testA
> +; Add concurrent to this section if the test can be run concurrently
> +; concurrent
> +
> +; For tests with subtests:
> +
> +; The value of list_subtests is a command that will list all the subtest for
> +; this test
> +; list_subtest=./%(test_name)s --list-tests
> +
> +; The value of subtest_regex should be a regular expression used to select
> +; which subtests to run.
> +; subtest_regex=fast.+
> +
> +; run_subtest is a command to execute a subtest.  Anywhere <subtest> is found
> +; in the command, it will be replaced with the name of the subtest.
> +; run_subtest=./%(test_name)s --test=<subtest>
> +
> +; For regular tests:
> +; run_test is the command used for running the test
> +run_test=./%(test_name)s
> -- 
> 1.8.1.5
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140819/7d08874a/attachment-0001.sig>