From c1e9bc02f86d584a4036375ac25e918f2f28e649 Mon Sep 17 00:00:00 2001 From: Michael Achenbach Date: Thu, 4 Jan 2018 07:39:09 +0000 Subject: [PATCH] Revert "Revert "[test] Move has unexpected output to outproc."" This reverts commit 1685b5d27ad371dc3ac79ad4a47042a11b5ac814. Reason for revert: Was probably caused by infra change: https://crrev.com/c/845781 Original change's description: > Revert "[test] Move has unexpected output to outproc." > > This reverts commit 71605b3ea4ddb700e9b59f49d16181a8736787b1. > > Reason for revert: Seems to break static-initializers step: > https://build.chromium.org/p/client.v8/builders/V8%20Linux64/builds/22156 > > Original change's description: > > [test] Move has unexpected output to outproc. > > > > Expected outcomes optimized to serialize [PASS] as None. > > > > Keeping expected outcomes inside output processors should be > > optimized in the future. Few possible optimizations: > > - separate classes for tests that are expected to PASS - done as > > an example in mozilla test suite. > > - cache output processors inside testcase. > > - share output processors between copies of the same test - needs > > some updates to the create_variant to update outproc only if > > expected outcomes changed. > > > > Bug: v8:6917 > > Change-Id: Ie73f1dcdf17fdfc65bce27228f818b1dd1e420c9 > > Reviewed-on: https://chromium-review.googlesource.com/843025 > > Commit-Queue: Michael Achenbach > > Reviewed-by: Michael Achenbach > > Cr-Commit-Position: refs/heads/master@{#50347} > > TBR=machenbach@chromium.org,sergiyb@chromium.org,majeski@google.com > > Change-Id: Ice1f3aee0a26f7f38996459d38fd6e0bd964113d > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:6917 > Reviewed-on: https://chromium-review.googlesource.com/849572 > Reviewed-by: Bill Budge > Commit-Queue: Bill Budge > Cr-Commit-Position: refs/heads/master@{#50348} TBR=bbudge@chromium.org,machenbach@chromium.org,sergiyb@chromium.org,majeski@google.com Change-Id: I7a522b6487de6e96985d223524533493eb9171f0 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: v8:6917 Reviewed-on: https://chromium-review.googlesource.com/848975 Reviewed-by: Michael Achenbach Commit-Queue: Michael Achenbach Cr-Commit-Position: refs/heads/master@{#50350} --- test/inspector/testcfg.py | 6 +-- test/message/testcfg.py | 9 ++-- test/mkgrokdump/testcfg.py | 8 ++-- test/mozilla/testcfg.py | 37 ++++++++++------ test/test262/testcfg.py | 44 ++++++++++++++----- test/webkit/testcfg.py | 6 +-- tools/testrunner/local/testsuite.py | 2 +- tools/testrunner/objects/outproc.py | 66 ++++++++++++++++++++++++++-- tools/testrunner/objects/testcase.py | 20 ++++++--- 9 files changed, 150 insertions(+), 48 deletions(-) diff --git a/test/inspector/testcfg.py b/test/inspector/testcfg.py index b1ff5ecd0f..bd82396bf0 100644 --- a/test/inspector/testcfg.py +++ b/test/inspector/testcfg.py @@ -42,8 +42,6 @@ class TestCase(testcase.TestCase): super(TestCase, self).__init__(*args, **kwargs) self._source_flags = self._parse_source_flags() - self._outproc = outproc.ExpectedOutProc( - os.path.join(self.suite.root, self.path) + EXPECTED_SUFFIX) def _get_files_params(self, ctx): return [ @@ -62,7 +60,9 @@ class TestCase(testcase.TestCase): @property def output_proc(self): - return self._outproc + return outproc.ExpectedOutProc( + self.expected_outcomes, + os.path.join(self.suite.root, self.path) + EXPECTED_SUFFIX) def GetSuite(name, root): diff --git a/test/message/testcfg.py b/test/message/testcfg.py index 87113a8f06..82ace455e1 100644 --- a/test/message/testcfg.py +++ b/test/message/testcfg.py @@ -71,8 +71,6 @@ class TestCase(testcase.TestCase): source = self.get_source() self._source_files = self._parse_source_files(source) self._source_flags = self._parse_source_flags(source) - self._outproc = OutProc(os.path.join(self.suite.root, self.path), - self._expected_fail()) def _parse_source_files(self, source): files = [] @@ -105,11 +103,14 @@ class TestCase(testcase.TestCase): @property def output_proc(self): - return self._outproc + return OutProc(self.expected_outcomes, + os.path.join(self.suite.root, self.path), + self._expected_fail()) class OutProc(outproc.OutProc): - def __init__(self, basepath, expected_fail): + def __init__(self, expected_outcomes, basepath, expected_fail): + super(OutProc, self).__init__(expected_outcomes) self._basepath = basepath self._expected_fail = expected_fail diff --git a/test/mkgrokdump/testcfg.py b/test/mkgrokdump/testcfg.py index f6b552a2e2..83033fc17d 100644 --- a/test/mkgrokdump/testcfg.py +++ b/test/mkgrokdump/testcfg.py @@ -17,8 +17,7 @@ class TestSuite(testsuite.TestSuite): super(TestSuite, self).__init__(*args, **kwargs) v8_path = os.path.dirname(os.path.dirname(os.path.abspath(self.root))) - expected_path = os.path.join(v8_path, 'tools', 'v8heapconst.py') - self.out_proc = OutProc(expected_path) + self.expected_path = os.path.join(v8_path, 'tools', 'v8heapconst.py') def ListTests(self, context): test = self._create_test(SHELL) @@ -43,11 +42,12 @@ class TestCase(testcase.TestCase): @property def output_proc(self): - return self.suite.out_proc + return OutProc(self.expected_outcomes, self.suite.expected_path) class OutProc(outproc.OutProc): - def __init__(self, expected_path): + def __init__(self, expected_outcomes, expected_path): + super(OutProc, self).__init__(expected_outcomes) self._expected_path = expected_path def _is_failure_output(self, output): diff --git a/test/mozilla/testcfg.py b/test/mozilla/testcfg.py index 0d0d0a7f6a..ed2fe3c7da 100644 --- a/test/mozilla/testcfg.py +++ b/test/mozilla/testcfg.py @@ -108,27 +108,38 @@ class TestCase(testcase.TestCase): @property def output_proc(self): + if not self.expected_outcomes: + if self.path.endswith('-n'): + return MOZILLA_PASS_NEGATIVE + return MOZILLA_PASS_DEFAULT if self.path.endswith('-n'): - return MOZILLA_NEGATIVE - return MOZILLA_DEFAULT + return NegOutProc(self.expected_outcomes) + return OutProc(self.expected_outcomes) + + +def _is_failure_output(self, output): + return ( + output.exit_code != 0 or + 'FAILED!' in output.stdout + ) class OutProc(outproc.OutProc): - def _is_failure_output(self, output): - return ( - output.exit_code != 0 or - 'FAILED!' in output.stdout - ) + """Optimized for positive tests.""" +OutProc._is_failure_output = _is_failure_output -class NegativeOutProc(OutProc): - @property - def negative(self): - return True +class PassOutProc(outproc.PassOutProc): + """Optimized for positive tests expected to PASS.""" +PassOutProc._is_failure_output = _is_failure_output -MOZILLA_DEFAULT = OutProc() -MOZILLA_NEGATIVE = NegativeOutProc() +NegOutProc = outproc.negative(OutProc) +NegPassOutProc = outproc.negative(PassOutProc) + + +MOZILLA_PASS_DEFAULT = PassOutProc() +MOZILLA_PASS_NEGATIVE = NegPassOutProc() def GetSuite(name, root): diff --git a/test/test262/testcfg.py b/test/test262/testcfg.py index e1571f98e6..0f5acb5c11 100644 --- a/test/test262/testcfg.py +++ b/test/test262/testcfg.py @@ -200,16 +200,11 @@ class TestCase(testcase.TestCase): source = self.get_source() self.test_record = self.suite.parse_test_record(source, self.path) - - expected_exception = ( + self._expected_exception = ( self.test_record .get('negative', {}) .get('type', None) ) - if expected_exception is None: - self._outproc = NO_EXCEPTION - else: - self._outproc = OutProc(expected_exception) def _get_files_params(self, ctx): return ( @@ -250,18 +245,23 @@ class TestCase(testcase.TestCase): @property def output_proc(self): - return self._outproc + if self._expected_exception is not None: + return ExceptionOutProc(self.expected_outcomes, self._expected_exception) + if self.expected_outcomes == outproc.OUTCOMES_PASS: + return PASS_NO_EXCEPTION + return NoExceptionOutProc(self.expected_outcomes) -class OutProc(outproc.OutProc): - def __init__(self, expected_exception=None): +class ExceptionOutProc(outproc.OutProc): + """Output processor for tests with expected exception.""" + def __init__(self, expected_outcomes, expected_exception=None): + super(ExceptionOutProc, self).__init__(expected_outcomes) self._expected_exception = expected_exception def _is_failure_output(self, output): if output.exit_code != 0: return True - if (self._expected_exception and - self._expected_exception != self._parse_exception(output.stdout)): + if self._expected_exception != self._parse_exception(output.stdout): return True return 'FAILED!' in output.stdout @@ -276,7 +276,27 @@ class OutProc(outproc.OutProc): return None -NO_EXCEPTION = OutProc() +def _is_failure_output(self, output): + return ( + output.exit_code != 0 or + 'FAILED!' in output.stdout + ) + + +class NoExceptionOutProc(outproc.OutProc): + """Output processor optimized for tests without expected exception.""" +NoExceptionOutProc._is_failure_output = _is_failure_output + + +class PassNoExceptionOutProc(outproc.PassOutProc): + """ + Output processor optimized for tests expected to PASS without expected + exception. + """ +PassNoExceptionOutProc._is_failure_output = _is_failure_output + + +PASS_NO_EXCEPTION = PassNoExceptionOutProc() def GetSuite(name, root): diff --git a/test/webkit/testcfg.py b/test/webkit/testcfg.py index 2e8b6304dd..2f32fa2ef1 100644 --- a/test/webkit/testcfg.py +++ b/test/webkit/testcfg.py @@ -68,8 +68,6 @@ class TestCase(testcase.TestCase): source = self.get_source() self._source_files = self._parse_source_files(source) self._source_flags = self._parse_source_flags(source) - self._outproc = OutProc( - os.path.join(self.suite.root, self.path) + '-expected.txt') def _parse_source_files(self, source): files_list = [] # List of file names to append to command arguments. @@ -106,7 +104,9 @@ class TestCase(testcase.TestCase): @property def output_proc(self): - return self._outproc + return OutProc( + self.expected_outcomes, + os.path.join(self.suite.root, self.path) + '-expected.txt') class OutProc(outproc.ExpectedOutProc): diff --git a/tools/testrunner/local/testsuite.py b/tools/testrunner/local/testsuite.py index 5ecb1ea1ba..bcc65267a4 100644 --- a/tools/testrunner/local/testsuite.py +++ b/tools/testrunner/local/testsuite.py @@ -183,7 +183,7 @@ class TestSuite(object): not test.output_proc.negative and statusfile.FAIL not in test.expected_outcomes ) - return test.output_proc.get_outcome(output) not in test.expected_outcomes + return test.output_proc.has_unexpected_output(output) def _create_test(self, path, **kwargs): test = self._test_class()(self, path, self._path_to_name(path), **kwargs) diff --git a/tools/testrunner/objects/outproc.py b/tools/testrunner/objects/outproc.py index a99fb1f050..557dfd349c 100644 --- a/tools/testrunner/objects/outproc.py +++ b/tools/testrunner/objects/outproc.py @@ -7,7 +7,14 @@ import itertools from ..local import statusfile -class OutProc(object): +OUTCOMES_PASS = [statusfile.PASS] +OUTCOMES_FAIL = [statusfile.FAIL] + + +class BaseOutProc(object): + def has_unexpected_output(self, output): + return self.get_outcome(output) not in self.expected_outcomes + def get_outcome(self, output): if output.HasCrashed(): return statusfile.CRASH @@ -31,12 +38,65 @@ class OutProc(object): def negative(self): return False + @property + def expected_outcomes(self): + raise NotImplementedError() -DEFAULT = OutProc() + +def negative(cls): + class Neg(cls): + @property + def negative(self): + return True + return Neg + + +class PassOutProc(BaseOutProc): + """Output processor optimized for positive tests expected to PASS.""" + def has_unexpected_output(self, output): + return self.get_outcome(output) != statusfile.PASS + + @property + def expected_outcomes(self): + return OUTCOMES_PASS + + +class OutProc(BaseOutProc): + """Output processor optimized for positive tests with expected outcomes + different than a single PASS. + """ + def __init__(self, expected_outcomes): + self._expected_outcomes = expected_outcomes + + @property + def expected_outcomes(self): + return self._expected_outcomes + + # TODO(majeski): Inherit from PassOutProc in case of OUTCOMES_PASS and remove + # custom get/set state. + def __getstate__(self): + d = self.__dict__ + if self._expected_outcomes is OUTCOMES_PASS: + d = d.copy() + del d['_expected_outcomes'] + return d + + def __setstate__(self, d): + if '_expected_outcomes' not in d: + d['_expected_outcomes'] = OUTCOMES_PASS + self.__dict__.update(d) + + +# TODO(majeski): Override __reduce__ to make it deserialize as one instance. +DEFAULT = PassOutProc() class ExpectedOutProc(OutProc): - def __init__(self, expected_filename): + """Output processor that has is_failure_output depending on comparing the + output with the expected output. + """ + def __init__(self, expected_outcomes, expected_filename): + super(ExpectedOutProc, self).__init__(expected_outcomes) self._expected_filename = expected_filename def _is_failure_output(self, output): diff --git a/tools/testrunner/objects/testcase.py b/tools/testrunner/objects/testcase.py index c285a6a58b..8f9c73fef7 100644 --- a/tools/testrunner/objects/testcase.py +++ b/tools/testrunner/objects/testcase.py @@ -38,6 +38,7 @@ from ..local import utils FLAGS_PATTERN = re.compile(r"//\s+Flags:(.*)") + class TestCase(object): def __init__(self, suite, path, name): self.suite = suite # TestSuite object @@ -53,7 +54,7 @@ class TestCase(object): self.cmd = None self._statusfile_outcomes = None - self.expected_outcomes = None + self._expected_outcomes = None # optimization: None == [statusfile.PASS] self._statusfile_flags = None self._prepare_outcomes() @@ -88,7 +89,7 @@ class TestCase(object): def _parse_status_file_outcomes(self, outcomes): if (statusfile.FAIL_SLOPPY in outcomes and '--use-strict' not in self.variant_flags): - return [statusfile.FAIL] + return outproc.OUTCOMES_FAIL expected_outcomes = [] if (statusfile.FAIL in outcomes or @@ -96,9 +97,16 @@ class TestCase(object): expected_outcomes.append(statusfile.FAIL) if statusfile.CRASH in outcomes: expected_outcomes.append(statusfile.CRASH) - if statusfile.PASS in outcomes: + + # Do not add PASS if there is nothing else. Empty outcomes are converted to + # the global [PASS]. + if expected_outcomes and statusfile.PASS in outcomes: expected_outcomes.append(statusfile.PASS) - return expected_outcomes or [statusfile.PASS] + + # Avoid creating multiple instances of a list with a single FAIL. + if expected_outcomes == outproc.OUTCOMES_FAIL: + return outproc.OUTCOMES_FAIL + return expected_outcomes or outproc.OUTCOMES_PASS @property def do_skip(self): @@ -239,7 +247,9 @@ class TestCase(object): @property def output_proc(self): - return outproc.DEFAULT + if self.expected_outcomes is outproc.OUTCOMES_PASS: + return outproc.DEFAULT + return outproc.OutProc(self.expected_outcomes) def __cmp__(self, other): # Make sure that test cases are sorted correctly if sorted without