From f951b0f82dff97f91d9b89c67886740acf3373cd Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Tue, 17 Mar 2020 15:27:59 -0400 Subject: [PATCH] [lit] Add builtin support for flaky tests in lit This commit adds a new keyword in lit called ALLOW_RETRIES. This keyword takes a single integer as an argument, and it allows the test to fail that number of times before it first succeeds. This work attempts to make the existing test_retry_attempts more flexible by allowing by-test customization, as well as eliminate libc++'s FLAKY_TEST custom logic. Differential Revision: https://reviews.llvm.org/D76288 --- llvm/utils/lit/lit/Test.py | 4 ++ llvm/utils/lit/lit/TestRunner.py | 39 ++++++++++++++---- .../does-not-succeed-within-limit.py | 3 ++ .../lit/tests/Inputs/allow-retries/lit.cfg | 9 ++++ .../more-than-one-allow-retries-lines.py | 4 ++ .../allow-retries/not-a-valid-integer.py | 3 ++ .../allow-retries/succeeds-within-limit.py | 24 +++++++++++ .../tests/Inputs/test_retry_attempts/lit.cfg | 10 +++++ .../tests/Inputs/test_retry_attempts/test.py | 22 ++++++++++ .../Inputs/testrunner-custom-parsers/test.txt | 3 ++ llvm/utils/lit/tests/allow-retries.py | 41 +++++++++++++++++++ llvm/utils/lit/tests/unit/TestRunner.py | 12 ++++++ llvm/utils/vim/syntax/llvm.vim | 1 + .../vscode/llvm/syntaxes/ll.tmLanguage.yaml | 2 + 14 files changed, 170 insertions(+), 7 deletions(-) create mode 100644 llvm/utils/lit/tests/Inputs/allow-retries/does-not-succeed-within-limit.py create mode 100644 llvm/utils/lit/tests/Inputs/allow-retries/lit.cfg create mode 100644 llvm/utils/lit/tests/Inputs/allow-retries/more-than-one-allow-retries-lines.py create mode 100644 llvm/utils/lit/tests/Inputs/allow-retries/not-a-valid-integer.py create mode 100644 llvm/utils/lit/tests/Inputs/allow-retries/succeeds-within-limit.py create mode 100644 llvm/utils/lit/tests/Inputs/test_retry_attempts/lit.cfg create mode 100644 llvm/utils/lit/tests/Inputs/test_retry_attempts/test.py create mode 100644 llvm/utils/lit/tests/allow-retries.py diff --git a/llvm/utils/lit/lit/Test.py b/llvm/utils/lit/lit/Test.py index 62f1bbf1f03a..000bcf8fc38f 100644 --- a/llvm/utils/lit/lit/Test.py +++ b/llvm/utils/lit/lit/Test.py @@ -220,6 +220,10 @@ class Test: # triple parts. All of them must be False for the test to run. self.unsupported = [] + # An optional number of retries allowed before the test finally succeeds. + # The test is run at most once plus the number of retries specified here. + self.allowed_retries = getattr(config, 'test_retry_attempts', 0) + # The test result, once complete. self.result = None diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py index 96411c98ee33..a9518b2b5a0b 100644 --- a/llvm/utils/lit/lit/TestRunner.py +++ b/llvm/utils/lit/lit/TestRunner.py @@ -1182,13 +1182,15 @@ class ParserKind(object): LIST: A keyword taking a comma-separated list of values. BOOLEAN_EXPR: A keyword taking a comma-separated list of boolean expressions. Ex 'XFAIL:' + INTEGER: A keyword taking a single integer. Ex 'ALLOW_RETRIES:' CUSTOM: A keyword with custom parsing semantics. """ TAG = 0 COMMAND = 1 LIST = 2 BOOLEAN_EXPR = 3 - CUSTOM = 4 + INTEGER = 4 + CUSTOM = 5 @staticmethod def allowedKeywordSuffixes(value): @@ -1196,6 +1198,7 @@ class ParserKind(object): ParserKind.COMMAND: [':'], ParserKind.LIST: [':'], ParserKind.BOOLEAN_EXPR: [':'], + ParserKind.INTEGER: [':'], ParserKind.CUSTOM: [':', '.'] } [value] @@ -1205,6 +1208,7 @@ class ParserKind(object): ParserKind.COMMAND: 'COMMAND', ParserKind.LIST: 'LIST', ParserKind.BOOLEAN_EXPR: 'BOOLEAN_EXPR', + ParserKind.INTEGER: 'INTEGER', ParserKind.CUSTOM: 'CUSTOM' } [value] @@ -1247,6 +1251,8 @@ class IntegratedTestKeywordParser(object): self.parser = self._handleList elif kind == ParserKind.BOOLEAN_EXPR: self.parser = self._handleBooleanExpr + elif kind == ParserKind.INTEGER: + self.parser = self._handleSingleInteger elif kind == ParserKind.TAG: self.parser = self._handleTag elif kind == ParserKind.CUSTOM: @@ -1311,6 +1317,18 @@ class IntegratedTestKeywordParser(object): output.extend([s.strip() for s in line.split(',')]) return output + @staticmethod + def _handleSingleInteger(line_number, line, output): + """A parser for INTEGER type keywords""" + if output is None: + output = [] + try: + n = int(line) + except ValueError: + raise ValueError("INTEGER parser requires the input to be an integer (got {})".format(line)) + output.append(n) + return output + @staticmethod def _handleBooleanExpr(line_number, line, output): """A parser for BOOLEAN_EXPR type keywords""" @@ -1331,8 +1349,8 @@ class IntegratedTestKeywordParser(object): def parseIntegratedTestScript(test, additional_parsers=[], require_script=True): """parseIntegratedTestScript - Scan an LLVM/Clang style integrated test - script and extract the lines to 'RUN' as well as 'XFAIL' and 'REQUIRES' - and 'UNSUPPORTED' information. + script and extract the lines to 'RUN' as well as 'XFAIL', 'REQUIRES', + 'UNSUPPORTED' and 'ALLOW_RETRIES' information. If additional parsers are specified then the test is also scanned for the keywords they specify and all matches are passed to the custom parser. @@ -1353,6 +1371,7 @@ def parseIntegratedTestScript(test, additional_parsers=[], initial_value=test.requires), IntegratedTestKeywordParser('UNSUPPORTED:', ParserKind.BOOLEAN_EXPR, initial_value=test.unsupported), + IntegratedTestKeywordParser('ALLOW_RETRIES:', ParserKind.INTEGER), IntegratedTestKeywordParser('END.', ParserKind.TAG) ] keyword_parsers = {p.keyword: p for p in builtin_parsers} @@ -1412,6 +1431,14 @@ def parseIntegratedTestScript(test, additional_parsers=[], "Test does not support the following features " "and/or targets: %s" % msg) + # Handle ALLOW_RETRIES: + allowed_retries = keyword_parsers['ALLOW_RETRIES:'].getValue() + if allowed_retries: + if len(allowed_retries) > 1: + return lit.Test.Result(Test.UNRESOLVED, + "Test has more than one ALLOW_RETRIES lines") + test.allowed_retries = allowed_retries[0] + # Enforce limit_to_features. if not test.isWithinFeatureLimits(): msg = ', '.join(test.config.limit_to_features) @@ -1477,10 +1504,8 @@ def executeShTest(test, litConfig, useExternalSh, normalize_slashes=useExternalSh) script = applySubstitutions(script, substitutions) - # Re-run failed tests up to test_retry_attempts times. - attempts = 1 - if hasattr(test.config, 'test_retry_attempts'): - attempts += test.config.test_retry_attempts + # Re-run failed tests up to test.allowed_retries times. + attempts = test.allowed_retries + 1 for i in range(attempts): res = _runShTest(test, litConfig, useExternalSh, script, tmpBase) if res.code != Test.FAIL: diff --git a/llvm/utils/lit/tests/Inputs/allow-retries/does-not-succeed-within-limit.py b/llvm/utils/lit/tests/Inputs/allow-retries/does-not-succeed-within-limit.py new file mode 100644 index 000000000000..05e3f35b6f81 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/allow-retries/does-not-succeed-within-limit.py @@ -0,0 +1,3 @@ +# ALLOW_RETRIES: 3 + +# RUN: false diff --git a/llvm/utils/lit/tests/Inputs/allow-retries/lit.cfg b/llvm/utils/lit/tests/Inputs/allow-retries/lit.cfg new file mode 100644 index 000000000000..eed69f389ed0 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/allow-retries/lit.cfg @@ -0,0 +1,9 @@ +import lit.formats +config.name = 'allow-retries' +config.suffixes = ['.py'] +config.test_format = lit.formats.ShTest() +config.test_source_root = None +config.test_exec_root = None + +config.substitutions.append(('%python', lit_config.params.get('python', ''))) +config.substitutions.append(('%counter', lit_config.params.get('counter', ''))) diff --git a/llvm/utils/lit/tests/Inputs/allow-retries/more-than-one-allow-retries-lines.py b/llvm/utils/lit/tests/Inputs/allow-retries/more-than-one-allow-retries-lines.py new file mode 100644 index 000000000000..14fb6b26661a --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/allow-retries/more-than-one-allow-retries-lines.py @@ -0,0 +1,4 @@ +# ALLOW_RETRIES: 3 +# ALLOW_RETRIES: 5 + +# RUN: true diff --git a/llvm/utils/lit/tests/Inputs/allow-retries/not-a-valid-integer.py b/llvm/utils/lit/tests/Inputs/allow-retries/not-a-valid-integer.py new file mode 100644 index 000000000000..d624de900b7f --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/allow-retries/not-a-valid-integer.py @@ -0,0 +1,3 @@ +# ALLOW_RETRIES: not-an-integer + +# RUN: true diff --git a/llvm/utils/lit/tests/Inputs/allow-retries/succeeds-within-limit.py b/llvm/utils/lit/tests/Inputs/allow-retries/succeeds-within-limit.py new file mode 100644 index 000000000000..45ac9433fc7e --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/allow-retries/succeeds-within-limit.py @@ -0,0 +1,24 @@ +# ALLOW_RETRIES: 5 + +# RUN: "%python" "%s" "%counter" + +import sys +import os + +counter_file = sys.argv[1] + +# The first time the test is run, initialize the counter to 1. +if not os.path.exists(counter_file): + with open(counter_file, 'w') as counter: + counter.write("1") + +# Succeed if this is the fourth time we're being run. +with open(counter_file, 'r') as counter: + num = int(counter.read()) + if num == 4: + sys.exit(0) + +# Otherwise, increment the counter and fail +with open(counter_file, 'w') as counter: + counter.write(str(num + 1)) + sys.exit(1) diff --git a/llvm/utils/lit/tests/Inputs/test_retry_attempts/lit.cfg b/llvm/utils/lit/tests/Inputs/test_retry_attempts/lit.cfg new file mode 100644 index 000000000000..a3b660fbaef3 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/test_retry_attempts/lit.cfg @@ -0,0 +1,10 @@ +import lit.formats +config.name = 'test_retry_attempts' +config.suffixes = ['.py'] +config.test_format = lit.formats.ShTest() +config.test_source_root = None +config.test_exec_root = None + +config.test_retry_attempts = 5 +config.substitutions.append(('%python', lit_config.params.get('python', ''))) +config.substitutions.append(('%counter', lit_config.params.get('counter', ''))) diff --git a/llvm/utils/lit/tests/Inputs/test_retry_attempts/test.py b/llvm/utils/lit/tests/Inputs/test_retry_attempts/test.py new file mode 100644 index 000000000000..ee8a92cc5d8f --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/test_retry_attempts/test.py @@ -0,0 +1,22 @@ +# RUN: "%python" "%s" "%counter" + +import sys +import os + +counter_file = sys.argv[1] + +# The first time the test is run, initialize the counter to 1. +if not os.path.exists(counter_file): + with open(counter_file, 'w') as counter: + counter.write("1") + +# Succeed if this is the fourth time we're being run. +with open(counter_file, 'r') as counter: + num = int(counter.read()) + if num == 4: + sys.exit(0) + +# Otherwise, increment the counter and fail +with open(counter_file, 'w') as counter: + counter.write(str(num + 1)) + sys.exit(1) diff --git a/llvm/utils/lit/tests/Inputs/testrunner-custom-parsers/test.txt b/llvm/utils/lit/tests/Inputs/testrunner-custom-parsers/test.txt index e28060320a2e..5809af5477ce 100644 --- a/llvm/utils/lit/tests/Inputs/testrunner-custom-parsers/test.txt +++ b/llvm/utils/lit/tests/Inputs/testrunner-custom-parsers/test.txt @@ -13,6 +13,9 @@ // MY_BOOL: b) // MY_BOOL: d // +// MY_INT: 4 +// MY_INT: 6 +// // MY_BOOL_UNTERMINATED: a \ // // END. diff --git a/llvm/utils/lit/tests/allow-retries.py b/llvm/utils/lit/tests/allow-retries.py new file mode 100644 index 000000000000..3f6cf8f1faa5 --- /dev/null +++ b/llvm/utils/lit/tests/allow-retries.py @@ -0,0 +1,41 @@ +# Check the behavior of the ALLOW_RETRIES keyword. + +# This test uses a file that's stable across retries of the test to fail and +# only succeed the fourth time it is retried. +# +# RUN: rm -f %t.counter +# RUN: %{lit} -j 1 %{inputs}/allow-retries/succeeds-within-limit.py -Dcounter=%t.counter -Dpython=%{python} | FileCheck --check-prefix=CHECK-TEST1 %s +# CHECK-TEST1: Passes With Retry : 1 + +# Test that a per-file ALLOW_RETRIES overwrites the config-wide test_retry_attempts property, if any. +# +# RUN: rm -f %t.counter +# RUN: %{lit} -j 1 %{inputs}/allow-retries/succeeds-within-limit.py -Dtest_retry_attempts=2 -Dcounter=%t.counter -Dpython=%{python} | FileCheck --check-prefix=CHECK-TEST2 %s +# CHECK-TEST2: Passes With Retry : 1 + +# This test does not succeed within the allowed retry limit +# +# RUN: not %{lit} -j 1 %{inputs}/allow-retries/does-not-succeed-within-limit.py | FileCheck --check-prefix=CHECK-TEST3 %s +# CHECK-TEST3: Failing Tests (1): +# CHECK-TEST3: allow-retries :: does-not-succeed-within-limit.py + +# This test should be UNRESOLVED since it has more than one ALLOW_RETRIES +# lines, and that is not allowed. +# +# RUN: not %{lit} -j 1 %{inputs}/allow-retries/more-than-one-allow-retries-lines.py | FileCheck --check-prefix=CHECK-TEST4 %s +# CHECK-TEST4: Unresolved Tests (1): +# CHECK-TEST4: allow-retries :: more-than-one-allow-retries-lines.py + +# This test does not provide a valid integer to the ALLOW_RETRIES keyword. +# It should be unresolved. +# +# RUN: not %{lit} -j 1 %{inputs}/allow-retries/not-a-valid-integer.py | FileCheck --check-prefix=CHECK-TEST5 %s +# CHECK-TEST5: Unresolved Tests (1): +# CHECK-TEST5: allow-retries :: not-a-valid-integer.py + +# This test checks that the config-wide test_retry_attempts property is used +# when no ALLOW_RETRIES keyword is present. +# +# RUN: rm -f %t.counter +# RUN: %{lit} -j 1 %{inputs}/test_retry_attempts/test.py -Dcounter=%t.counter -Dpython=%{python} | FileCheck --check-prefix=CHECK-TEST6 %s +# CHECK-TEST6: Passes With Retry : 1 diff --git a/llvm/utils/lit/tests/unit/TestRunner.py b/llvm/utils/lit/tests/unit/TestRunner.py index ceb7bef34f6a..4f33fce64885 100644 --- a/llvm/utils/lit/tests/unit/TestRunner.py +++ b/llvm/utils/lit/tests/unit/TestRunner.py @@ -57,6 +57,7 @@ class TestIntegratedTestKeywordParser(unittest.TestCase): IntegratedTestKeywordParser("MY_DNE_TAG.", ParserKind.TAG), IntegratedTestKeywordParser("MY_LIST:", ParserKind.LIST), IntegratedTestKeywordParser("MY_BOOL:", ParserKind.BOOLEAN_EXPR), + IntegratedTestKeywordParser("MY_INT:", ParserKind.INTEGER), IntegratedTestKeywordParser("MY_RUN:", ParserKind.COMMAND), IntegratedTestKeywordParser("MY_CUSTOM:", ParserKind.CUSTOM, custom_parse), @@ -112,6 +113,17 @@ class TestIntegratedTestKeywordParser(unittest.TestCase): self.assertEqual(value[0].strip(), "a && (b)") self.assertEqual(value[1].strip(), "d") + def test_integer(self): + parsers = self.make_parsers() + self.parse_test(parsers) + int_parser = self.get_parser(parsers, 'MY_INT:') + value = int_parser.getValue() + self.assertEqual(len(value), 2) # there are only two MY_INT: lines + self.assertEqual(type(value[0]), int) + self.assertEqual(value[0], 4) + self.assertEqual(type(value[1]), int) + self.assertEqual(value[1], 6) + def test_boolean_unterminated(self): parsers = self.make_parsers() + \ [IntegratedTestKeywordParser("MY_BOOL_UNTERMINATED:", ParserKind.BOOLEAN_EXPR)] diff --git a/llvm/utils/vim/syntax/llvm.vim b/llvm/utils/vim/syntax/llvm.vim index 487a37b4b86b..0a661e82d86a 100644 --- a/llvm/utils/vim/syntax/llvm.vim +++ b/llvm/utils/vim/syntax/llvm.vim @@ -203,6 +203,7 @@ syn match llvmConstant /\/ syn match llvmSpecialComment /;\s*PR\d*\s*$/ syn match llvmSpecialComment /;\s*REQUIRES:.*$/ syn match llvmSpecialComment /;\s*RUN:.*$/ +syn match llvmSpecialComment /;\s*ALLOW_RETRIES:.*$/ syn match llvmSpecialComment /;\s*CHECK:.*$/ syn match llvmSpecialComment "\v;\s*CHECK-(NEXT|NOT|DAG|SAME|LABEL):.*$" syn match llvmSpecialComment /;\s*XFAIL:.*$/ diff --git a/llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml b/llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml index 9765cee98df8..117ec134d573 100644 --- a/llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml +++ b/llvm/utils/vscode/llvm/syntaxes/ll.tmLanguage.yaml @@ -319,6 +319,8 @@ patterns: name: string.regexp - match: ";\\s*RUN:.*$" name: string.regexp + - match: ";\\s*ALLOW_RETRIES:.*$" + name: string.regexp - match: ";\\s*CHECK:.*$" name: string.regexp - match: ";\\s*CHECK-(NEXT|NOT|DAG|SAME|LABEL):.*$"