[Piglit] [PATCH 2/3] oclconform: Add test class for ocl conformance tests

[Dylan Baker]

On Monday, August 18, 2014 10:44:41 AM Tom Stellard wrote:
> ---
>  framework/oclconform.py | 91 +++++++++++++++++++++++++++++++++++++++++++++++++
>  piglit.conf.example     | 34 ++++++++++++++++++
>  2 files changed, 125 insertions(+)
>  create mode 100644 framework/oclconform.py
> 
> diff --git a/framework/oclconform.py b/framework/oclconform.py
> new file mode 100644
> index 0000000..a3e44ee
> --- /dev/null
> +++ b/framework/oclconform.py
> @@ -0,0 +1,91 @@
> +# 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>
> +#
> +
> +import re
> +import subprocess
> +from os import listdir
> +from os.path import isfile, join
> +
> +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)

You would probably be better served to do __init__(self, *args,
**kwargs) here, and pass those as Test.__init__(self, *args, **kwargs),
it will give you more flexibility and protect you from changes to Test.

> +
> +    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 = []
> +    for option in options:
> +        if PIGLIT_CONFIG.get(section_name, option) == None:

Dont use == with None, this should be 'if <thing> is None', although, if
you dont specifically need to know that it is None and not another falsy
value it is more pythonic to say 'if not <condition>'

> +            tests.append(option)

You should refactor this to:
tests = (o for o in options if not PIGLIT_CONFIg.get(section_name, o))

or to this if you really need to test for None
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: not section defined for ", test
no section

Please use print_function from the __future__ module? I've been trying
to get all of piglit using it. You might also want to redirect this to
stderr instead stdout, but that's just a suggestion. If you wanted to
use stderr (with the print_function):

print("Warning: no section defined for {}".foramt(test), file=sys.stderr)

Obviously you'd need sys for printing to stderr, be aware that the print
function unlike the print statement does not print all arguments, it
prints only the first one, so format is necessary.

> +            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')

I would refactor your re like this
subtest_regex = PIGLIT_CONFIG.get(test_section_name, 'subtest_regex')
subtest_regex.encode('string_escape')  # This converts to a raw string

> +            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(r'{}'.format(subtest_regex), subtest)
m = re.match(subest_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
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- 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/20140818/fdcc24cc/attachment.sig>