<div dir="ltr">I think the default mode we want here is more of a "continue after crash" mode where, when it crashes, we don't rerun anything but instead just continue starting from the next test. This is what Google does with their "cherry" tool and it would prevent ensure that random crashes don't get hidden by the rerun. </div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Mar 24, 2016 at 11:43 AM, Dylan Baker <<a href="mailto:baker.dylan.c@gmail.com" target="_blank">baker.dylan.c@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This adds a new feature to the deqp integration group at a time mode, which allows tests to be rerun when they meet a set of bad status in test at a time mode. This helps to eliminate tests where the crash caused tests to not be run, and helps clear up issues where an error would change test statuses. In my runs this is still significantly faster than test at a time mode, unless a huge number of tests are crashing. In that case it is really up to the developer to know whether they need test at a time mode, or whether group at a time mode is sufficient. A new option, --deqp-no-group-rerun, is added which disables this feature. This also addresses the vulkan suite issue, by rerunning any tests where the problematic statuses are detected in test at a time mode. Signed-off-by: Dylan Baker <<a href="mailto:dylanx.c.baker@intel.com">dylanx.c.baker@intel.com</a>> --- framework/backends/abstract.py | 6 ++ framework/options.py | 3 + framework/profile.py | 80 ++++++++++-------- framework/programs/run.py | 9 ++ framework/test/deqp.py | 182 +++++++++++++++++++++++++++-------------- tests/deqp_vk.py | 11 ++- unittests/deqp_tests.py | 50 +++++++---- 7 files changed, 228 insertions(+), 113 deletions(-) diff --git a/framework/backends/abstract.py b/framework/backends/abstract.py index b5b4858..4ebc8ae 100644 --- a/framework/backends/abstract.py +++ b/framework/backends/abstract.py @@ -207,6 +207,12 @@ class FileBackend(Backend): """ def finish(val): + # If finish is passed None delete the file. This is used in the + # dEQP backend + if val is None: + os.unlink(file_) + return + tfile = file_ + '.tmp' with open(tfile, 'w') as f: self._write(f, name, val) diff --git a/framework/options.py b/framework/options.py index 52d90be..b39cf60 100644 --- a/framework/options.py +++ b/framework/options.py @@ -181,6 +181,8 @@ class _Options(object): # pylint: disable=too-many-instance-attributes env -- environment variables set for each test before run deqp_mode -- either 'test' or 'group. Controls deqp integration mode, to either run test at a time or group at a time. + deqp_group_rerun -- If True any tests that fail in group mode will be rerun + in individual mode. """ include_filter = _ReListDescriptor('_include_filter', type_=_FilterReList) @@ -196,6 +198,7 @@ class _Options(object): # pylint: disable=too-many-instance-attributes self.dmesg = False self.sync = False self.deqp_mode = 'test' + self.deqp_group_rerun = False # env is used to set some base environment variables that are not going # to change across runs, without sending them to os.environ which is diff --git a/framework/profile.py b/framework/profile.py index fc38c56..5971dcb 100644 --- a/framework/profile.py +++ b/framework/profile.py @@ -232,6 +232,50 @@ class TestProfile(object): """ pass + def _test(self, pair, log, backend): + """Function to call test.execute from map""" + name, test = pair + with backend.write_test(name) as w: + test.execute(name, log.get(), self.dmesg) + w(test.result) + + def _run_threads(self, pool, testlist, log, backend): + """ Open a pool, close it, and join it """ + pool.imap(lambda pair: self._test(pair, log, backend), testlist) + pool.close() + pool.join() + + def _run(self, log, backend, test_list=None): + """Run all the tests using requested threading.""" + test_list = test_list or self.test_list + + # Multiprocessing.dummy is a wrapper around Threading that provides a + # multiprocessing compatible API + # + # The default value of pool is the number of virtual processor cores + single = multiprocessing.dummy.Pool(1) + multi = multiprocessing.dummy.Pool() + + if options.OPTIONS.concurrent == "all": + self._run_threads(multi, six.iteritems(test_list), log, + backend) + elif options.OPTIONS.concurrent == "none": + self._run_threads(single, six.iteritems(test_list), log, + backend) + else: + # Filter and return only thread safe tests to the threaded pool + self._run_threads( + multi, + (x for x in six.iteritems(test_list) if x[1].run_concurrent), + log, + backend) + # Filter and return the non thread safe tests to the single pool + self._run_threads( + single, + (x for x in six.iteritems(test_list) if not x[1].run_concurrent), + log, + backend) + def run(self, logger, backend): """ Runs all tests using Thread pool @@ -252,43 +296,9 @@ class TestProfile(object): self._pre_run_hook() - chunksize = 1 - self._prepare_test_list() log = LogManager(logger, len(self.test_list)) - - def test(pair): - """Function to call test.execute from map""" - name, test = pair - with backend.write_test(name) as w: - test.execute(name, log.get(), self.dmesg) - w(test.result) - - def run_threads(pool, testlist): - """ Open a pool, close it, and join it """ - pool.imap(test, testlist, chunksize) - pool.close() - pool.join() - - # Multiprocessing.dummy is a wrapper around Threading that provides a - # multiprocessing compatible API - # - # The default value of pool is the number of virtual processor cores - single = multiprocessing.dummy.Pool(1) - multi = multiprocessing.dummy.Pool() - - if options.OPTIONS.concurrent == "all": - run_threads(multi, six.iteritems(self.test_list)) - elif options.OPTIONS.concurrent == "none": - run_threads(single, six.iteritems(self.test_list)) - else: - # Filter and return only thread safe tests to the threaded pool - run_threads(multi, (x for x in six.iteritems(self.test_list) - if x[1].run_concurrent)) - # Filter and return the non thread safe tests to the single pool - run_threads(single, (x for x in six.iteritems(self.test_list) - if not x[1].run_concurrent)) - + self._run(log, backend) log.get().summary() self._post_run_hook() diff --git a/framework/programs/run.py b/framework/programs/run.py index 5d171c1..9142d2e 100644 --- a/framework/programs/run.py +++ b/framework/programs/run.py @@ -171,6 +171,13 @@ def _run_parser(input_): dest='deqp_mode', help='Run DEQP integration in either "group-at-a-time"' ' or "test-at-a-time" mode.') + parser.add_argument('--deqp-no-group-rerun', + action='store_true', + dest='deqp_no_group_rerun', + help='With --deqp-test-mode=group, this option will ' + 'prevent all tests with a status not in ' + '[pass, skip] from being rerun in test at a ' + 'time mode') parser.add_argument("test_profile", metavar="<Profile path(s)>", nargs='+', @@ -249,6 +256,7 @@ def run(input_): options.OPTIONS.dmesg = args.dmesg options.OPTIONS.sync = args.sync options.OPTIONS.deqp_mode = args.deqp_mode + options.OPTIONS.deqp_group_rerun = not args.deqp_no_group_rerun # Set the platform to pass to waffle options.OPTIONS.env['PIGLIT_PLATFORM'] = args.platform @@ -329,6 +337,7 @@ def resume(input_): options.OPTIONS.dmesg = results.options['dmesg'] options.OPTIONS.sync = results.options['sync'] options.OPTIONS.deqp_mode = results.options['deqp_mode'] + options.OPTIONS.deqp_group_rerun = results.options['deqp_group_rerun'] core.get_config(args.config_file) diff --git a/framework/test/deqp.py b/framework/test/deqp.py index de6aab4..e7b52a4 100644 --- a/framework/test/deqp.py +++ b/framework/test/deqp.py @@ -31,6 +31,7 @@ import six from framework import core, grouptools, exceptions, status from framework.profile import TestProfile +from framework.log import LogManager from framework.test.base import Test, is_crash_returncode from framework.options import OPTIONS @@ -61,6 +62,8 @@ _EXTRA_ARGS = get_option('PIGLIT_DEQP_EXTRA_ARGS', ('deqp', 'extra_args'), default='').split() +_RERUN = ['crash', 'incomplete', 'timeout', 'notrun', 'skip'] + def _gen_caselist_txt(bin_, caselist, extra_args): """Generate a caselist.txt and return its path. @@ -108,19 +111,29 @@ def _iter_test_groups(case_file): contains only tests.) """ - slice_ = slice(len('GROUP: '), None) + slice_group = slice(len('GROUP: '), None) + slice_test = slice(len('TEST: '), None) + group = '' + tests = [] with open(case_file, 'r') as caselist_file: for i, line in enumerate(_iterate_file(caselist_file)): if line.startswith('GROUP:'): - group = line[slice_] + new = line[slice_group].strip() + + # This needs to handle the name of the new group being a + # superset of the old group (ex: items to items_max) + if new != group and tests: + yield group, tests + tests = [] + group = new elif line.startswith('TEST:'): - if group != '': - yield group.rstrip() - group = '' + tests.append(line[slice_test].strip()) else: raise exceptions.PiglitFatalError( 'deqp: {}:{}: ill-formed line'.format(case_file, i)) + # Yield the final set of tests. + yield group.strip(), tests def _iter_test_single(case_file): @@ -188,18 +201,61 @@ class DEQPProfile(TestProfile): filter_ = pop('filter_', default=lambda x: x) super(DEQPProfile, self).__init__(*args, **kwargs) + self._rerun = multi_class.rerun + self._rerun_class = single_class iter_ = _iter_test_cases(filter_( _gen_caselist_txt(bin_, filename, extra_args))) if OPTIONS.deqp_mode == 'group': - class_ = multi_class + for testname, rerun in iter_: + # deqp uses '.' as the testgroup separator. + piglit_name = testname.replace('.', grouptools.SEPARATOR) + self.test_list[piglit_name] = multi_class(testname, rerun) elif OPTIONS.deqp_mode == 'test': - class_ = single_class + for testname in iter_: + # deqp uses '.' as the testgroup separator. + piglit_name = testname.replace('.', grouptools.SEPARATOR) + self.test_list[piglit_name] = single_class(testname) + + def run(self, logger, backend): + """Run all tests. + + Adds the option to rerun tests if requested. + + """ + super(DEQPProfile, self).run(logger, backend) + + if OPTIONS.deqp_group_rerun and self._rerun: + print('\nRerunning failed tests in single mode. ' + '(run with --deqp-no-group-rerun to disable)\n') + + log = LogManager(logger, len(self._rerun)) + self._run(log, backend, + # TODO: replace this dict with a profile.TestDict + test_list={k.replace('.', grouptools.SEPARATOR).lower(): + self._rerun_class(k) for k in self._rerun}) - for testname in iter_: - piglit_name = testname.replace('.', grouptools.SEPARATOR) - self.test_list[piglit_name] = class_(testname) + log.get().summary() + + def _test(self, pair, log, backend): + """Function to call test.execute from map. + + if in Group mode and group-rerun is enabled,then pass None to write if + the test isn't pass or skip, this will cause it to delete the result + file rather than loading it, which will be replaced when the re-run + happens. + + """ + name, test = pair + if (OPTIONS.deqp_mode == 'group' and + OPTIONS.deqp_group_rerun and + isinstance(test, DEQPGroupTest)): + with backend.write_test(name) as w: + test.execute(name, log.get(), self.dmesg) + w(None if test.result.result in _RERUN else test.result) + else: + super(DEQPProfile, self)._test(pair, log, backend) @six.add_metaclass(abc.ABCMeta) @@ -273,11 +329,13 @@ class DEQPBaseTest(Test): class DEQPGroupTest(DEQPBaseTest): timeout = 300 # 5 minutes + rerun = [] __name_slicer = slice(len("Test case '"), -len("'..")) __finder = re.compile(r'^ (Warnings|Not supported|Failed|Passed):\s+\d/(?P<total>\d+).*') - def __init__(self, case_name, **kwargs): + def __init__(self, case_name, individual_cases, **kwargs): super(DEQPGroupTest, self).__init__(case_name + '*', **kwargs) + self._individual_cases = individual_cases def interpret_result(self): """Group based result interpretation. @@ -301,55 +359,59 @@ class DEQPGroupTest(DEQPBaseTest): # since there will almost certinaly be an exception raised. if self.result.returncode != 0: self.result.result = 'crash' - return - - # Strip the first 3 lines, and the last 8 lines, which aren't useful - # for this pass - lines = self.result.out.rstrip().split('\n')[3:] - cur = '' - total = None - for each in reversed(lines): - m = self.__finder.match(each) - if m: - total = int(m.group('total')) - break - assert total is not None, 'Could not calculate total test count' - - lines = (l for l in lines[:-8]) - - # Walk over standard out line by line, looking for 'Test case' (to get - # the name of the test) and then for a result. Track each line, which - # is used to both know when to stop walking and for error reporting. - while len(self.result.subtests) < total: - for l in lines: - if l.startswith('Test case'): - name = l[self.__name_slicer].rsplit('.', 1)[1].lower() - break - else: - raise exceptions.PiglitInternalError( - 'Expected "Test case", but didn\'t find it in:\n' - '{}\ncurrent line: {}'.format(self.result.out, l)) - - for l in lines: - # If there is an info block fast forward through it by calling - # next on the generator until it is passed. - if l.startswith('INFO'): - cur = '' - while not (cur.startswith('INFO') and cur.endswith('----')): - cur = next(lines) - - elif l.startswith(' '): - try: - self.result.subtests[name] = self._RESULT_MAP[l[2]] - except KeyError: - raise exceptions.PiglitInternalError( - 'Unknown status {}'.format(l[2:].split()[0])) + else: + # Strip the first 3 lines, and the last 8 lines, which aren't + # useful for this pass + lines = self.result.out.rstrip().split('\n')[3:] + cur = '' + total = None + for each in reversed(lines): + m = self.__finder.match(each) + if m: + total = int(m.group('total')) break - else: - raise exceptions.PiglitInternalError( - 'Expected " (Pass,Fail,...)", but didn\'t find it in:\n' - '{}\ncurrent line: {}'.format(self.result.out, l)) - + assert total is not None, 'Could not calculate total test count' + + lines = (l for l in lines[:-8]) + + # Walk over standard out line by line, looking for 'Test case' (to + # get the name of the test) and then for a result. Track each line, + # which is used to both know when to stop walking and for error + # reporting. + while len(self.result.subtests) < total: + for l in lines: + if l.startswith('Test case'): + name = l[self.__name_slicer].rsplit('.', 1)[1].lower() + break + else: + raise exceptions.PiglitInternalError( + 'Expected "Test case", but didn\'t find it in:\n' + '{}\ncurrent line: {}'.format(self.result.out, l)) + + for l in lines: + # If there is an info block fast forward through it by + # calling next on the generator until it is passed. + if l.startswith('INFO'): + cur = '' + while not (cur.startswith('INFO') and cur.endswith('----')): + cur = next(lines) + + elif l.startswith(' '): + try: + self.result.subtests[name] = self._RESULT_MAP[l[2]] + except KeyError: + raise exceptions.PiglitInternalError( + 'Unknown status {}'.format(l[2:].split()[0])) + break + else: + raise exceptions.PiglitInternalError( + 'Expected " (Pass,Fail,...)", ' + 'but didn\'t find it in:\n' + '{}\ncurrent line: {}'.format(self.result.out, l)) + + # If group_rerun (the default) and the status is crash rerun + if OPTIONS.deqp_group_rerun and self.result.result == 'crash': + self.rerun.extend(self._individual_cases) # We failed to parse the test output. Fallback to 'fail'. - if self.result.result == 'notrun': + elif self.result.result == 'notrun': self.result.result = 'fail' diff --git a/tests/deqp_vk.py b/tests/deqp_vk.py index 1cfce6e..1a8a911 100644 --- a/tests/deqp_vk.py +++ b/tests/deqp_vk.py @@ -30,7 +30,6 @@ from __future__ import ( import re from framework.test import deqp -from framework import exceptions __all__ = ['profile'] @@ -74,7 +73,15 @@ class DEQPVKTest(_Mixin, deqp.DEQPBaseTest): class DEQPVKGroupTest(_Mixin, deqp.DEQPGroupTest): """Test representation for Khronos Vulkacn CTS in group mode.""" - pass + def interpret_result(self): + # If there are either of the know result problems add all subcases to + # the rerun list. + if 'Failed to compile shader at vkGlslToSpirV' in self.result.out: + self.rerun.extend(self._individual_cases) + elif _DEQP_ASSERT.search(self.result.err): + self.rerun.extend(self._individual_cases) + else: + super(DEQPVKGroupTest, self).interpret_result() profile = deqp.DEQPProfile( # pylint: disable=invalid-name diff --git a/unittests/deqp_tests.py b/unittests/deqp_tests.py index 2d347a8..f3e62b0 100644 --- a/unittests/deqp_tests.py +++ b/unittests/deqp_tests.py @@ -45,7 +45,7 @@ else: except ImportError: from unittest import mock -from framework import profile, grouptools, exceptions +from framework import grouptools, exceptions from framework.test import deqp from . import utils @@ -107,23 +107,23 @@ def test_get_option_conf_no_option(): None) -def test_iter_deqp_test_cases_test(): - """deqp.iter_deqp_test_cases: correctly detects a TEST: line""" +def test_iter_test_cases_test(): + """deqp.iter_test_cases: correctly detects a TEST: line""" with utils.tempfile('TEST: a.deqp.test') as tfile: gen = deqp._iter_test_cases(tfile) nt.eq_('a.deqp.test', next(gen)) -def test_iter_deqp_test_cases_group(): - """deqp.iter_deqp_test_casesgen_caselist_txt: correctly detects a GROUP: line""" +def test_iter_test_cases_group(): + """deqp.iter_test_casesgen_caselist_txt: correctly detects a GROUP: line""" with utils.tempfile('GROUP: a group\nTEST: a.deqp.test') as tfile: gen = deqp._iter_test_cases(tfile) nt.eq_('a.deqp.test', next(gen)) @nt.raises(exceptions.PiglitFatalError) -def test_iter_deqp_test_cases_bad(): - """deqp.iter_deqp_test_casesgen_caselist_txt: PiglitFatalException is raised if line is not TEST: or GROUP: +def test_iter_test_cases_bad(): + """deqp.iter_test_casesgen_caselist_txt: PiglitFatalException is raised if line is not TEST: or GROUP: """ with utils.tempfile('this will fail') as tfile: gen = deqp._iter_test_cases(tfile) @@ -363,7 +363,7 @@ class TestDEQPGroupTest_interpret_result(object): @classmethod def setup_class(cls): - cls.test = _DEQPGroupTest('foo') + cls.test = _DEQPGroupTest('foo', []) cls.test.result.returncode = 0 cls.test.result.out = cls.__out cls.test.interpret_result() @@ -480,7 +480,7 @@ def test_DEQPGroupTest_interpret_result_cts(): Warnings: 0/2 (0.00%) """) - test = _DEQPGroupTest('foo') + test = _DEQPGroupTest('foo', []) test.result.returncode = 0 test.result.out = out test.interpret_result() @@ -490,7 +490,7 @@ def test_DEQPGroupTest_interpret_result_cts(): def test_DEQPGroupTest_interpret_result_nonzero(): """test.deqp.DEQPGroupTest.interpret_results: if returncode is nonzero test is crash""" - test = _DEQPGroupTest('foo') + test = _DEQPGroupTest('foo', []) test.result.returncode = -6 test.interpret_result() nt.eq_(test.result.result, 'crash') @@ -499,8 +499,8 @@ def test_DEQPGroupTest_interpret_result_nonzero(): @utils.skip(not (sys.version_info[0:2] >= (3, 4) or float(mock.__version__[:3]) >= 1.2), 'Test requires that mock.mock_open provides readline method.') -def test_iter_deqp_test_groups(): - """deqp._test_deqp_test_groups: Returns expected values""" +def test_iter_test_groups(): + """deqp._test_test_groups: Returns expected values""" text = textwrap.dedent("""\ GROUP: dEQP-GLES2.info TEST: dEQP-GLES2.info.vendor @@ -529,10 +529,28 @@ def test_iter_deqp_test_groups(): """) expected = [ - 'dEQP-GLES2.info', - 'dEQP-GLES2.capability.limits', - 'dEQP-GLES2.capability.limits_lower', - 'dEQP-GLES2.capability.extensions.uncompressed_texture_formats', + ('dEQP-GLES2.info', + ['dEQP-GLES2.info.vendor', + 'dEQP-GLES2.info.renderer', + 'dEQP-GLES2.info.version', + 'dEQP-GLES2.info.shading_language_version', + 'dEQP-GLES2.info.extensions', + 'dEQP-GLES2.info.render_target',]), + ('dEQP-GLES2.capability.limits', + ['dEQP-GLES2.capability.limits.vertex_attribs', + 'dEQP-GLES2.capability.limits.varying_vectors', + 'dEQP-GLES2.capability.limits.vertex_uniform_vectors', + 'dEQP-GLES2.capability.limits.fragment_uniform_vectors', + 'dEQP-GLES2.capability.limits.texture_image_units', + 'dEQP-GLES2.capability.limits.vertex_texture_image_units', + 'dEQP-GLES2.capability.limits.combined_texture_image_units', + 'dEQP-GLES2.capability.limits.texture_2d_size', + 'dEQP-GLES2.capability.limits.texture_cube_size', + 'dEQP-GLES2.capability.limits.renderbuffer_size']), + ('dEQP-GLES2.capability.limits_lower', + ['dEQP-GLES2.capability.limits_lower.minimum_size']), + ('dEQP-GLES2.capability.extensions.uncompressed_texture_formats', + ['dEQP-GLES2.capability.extensions.uncompressed_texture_formats.foo']), ] with mock.patch('framework.test.deqp.open', create=True, -- 2.7.4 _______________________________________________ Piglit mailing list <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/piglit" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/piglit</a> </blockquote></div> </div>