davezarzycki updated this revision to Diff 329613.
davezarzycki added a comment.
I believe I have addressed all of the feedback to date.
For people that care about easily identifying the failures from the previous
testing run, that is now encoded via the sign of the test time (please remember
that negative test times are impossible). For example:
3.14159 a-successful-test.txt
-2.71828 a-failed-test.txt
I've also implemented but commented out how one might use this to implement
`--only-failures`. Feedback would be appreciated but I don't want to derail
this change proposal on what should be an independent change.
Finally, python is not my native programming language, so thank you for helping
me with idiomatic changes. What I'd really like help with though is figuring
out a way to not pass the prior test timing data down as a new parameter to
various methods. I tried to hang the data off the test suite data structure but
that had brutal performance side effects. It seemed like python was hammering
on the global interpreter lock despite the prior timing data being unused after
the sorting phase. Weird. Help would be greatly appreciated. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98179/new/
https://reviews.llvm.org/D98179
Files:
libcxx/utils/libcxx/test/format.py
libcxx/utils/libcxx/test/googlebenchmark.py
lldb/test/API/lldbtest.py
llvm/docs/CommandGuide/lit.rst
llvm/test/Unit/lit.cfg.py
llvm/utils/lit/lit/Test.py
llvm/utils/lit/lit/TestingConfig.py
llvm/utils/lit/lit/cl_arguments.py
llvm/utils/lit/lit/discovery.py
llvm/utils/lit/lit/formats/base.py
llvm/utils/lit/lit/formats/googletest.py
llvm/utils/lit/lit/main.py
llvm/utils/lit/tests/Inputs/early-tests/aaa.txt
llvm/utils/lit/tests/Inputs/early-tests/bbb.txt
llvm/utils/lit/tests/Inputs/early-tests/lit.cfg
llvm/utils/lit/tests/Inputs/early-tests/subdir/ccc.txt
llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
llvm/utils/lit/tests/Inputs/reorder/aaa.txt
llvm/utils/lit/tests/Inputs/reorder/bbb.txt
llvm/utils/lit/tests/Inputs/reorder/lit.cfg
llvm/utils/lit/tests/Inputs/reorder/subdir/ccc.txt
llvm/utils/lit/tests/early-tests.py
llvm/utils/lit/tests/ignore-fail.py
llvm/utils/lit/tests/reorder.py
llvm/utils/lit/tests/shtest-shell.py
mlir/test/Unit/lit.cfg.py
Index: mlir/test/Unit/lit.cfg.py
===================================================================
--- mlir/test/Unit/lit.cfg.py
+++ mlir/test/Unit/lit.cfg.py
@@ -13,9 +13,6 @@
# suffixes: A list of file extensions to treat as test files.
config.suffixes = []
-# is_early; Request to run this suite early.
-config.is_early = True
-
# test_source_root: The root path where tests are located.
# test_exec_root: The root path where tests should be run.
config.test_exec_root = os.path.join(config.mlir_obj_root, 'unittests')
Index: llvm/utils/lit/tests/shtest-shell.py
===================================================================
--- llvm/utils/lit/tests/shtest-shell.py
+++ llvm/utils/lit/tests/shtest-shell.py
@@ -8,6 +8,8 @@
#
# Test again in non-UTF shell to catch potential errors with python 2 seen
# on stdout-encoding.txt
+# FIXME: lit's testing sets source_root == exec_root which complicates running lit more than once per test
+# RUN: rm -f %{inputs}/shtest-shell/.lit_test_times.txt
# RUN: env PYTHONIOENCODING=ascii not %{lit} -j 1 -a %{inputs}/shtest-shell > %t.ascii.out
# FIXME: Temporarily dump test output so we can debug failing tests on
# buildbots.
Index: llvm/utils/lit/tests/reorder.py
===================================================================
--- /dev/null
+++ llvm/utils/lit/tests/reorder.py
@@ -0,0 +1,11 @@
+## Check that we can reorder test starts
+
+# RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# RUN: %{lit} -j1 %{inputs}/reorder | FileCheck %s
+# RUN: not diff %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+
+# CHECK: -- Testing: 3 tests, 1 workers --
+# CHECK-NEXT: PASS: reorder :: subdir/ccc.txt
+# CHECK-NEXT: PASS: reorder :: bbb.txt
+# CHECK-NEXT: PASS: reorder :: aaa.txt
+# CHECK: Passed: 3
Index: llvm/utils/lit/tests/ignore-fail.py
===================================================================
--- llvm/utils/lit/tests/ignore-fail.py
+++ llvm/utils/lit/tests/ignore-fail.py
@@ -6,10 +6,10 @@
# END.
-# CHECK: FAIL: ignore-fail :: fail.txt
-# CHECK: UNRESOLVED: ignore-fail :: unresolved.txt
-# CHECK: XFAIL: ignore-fail :: xfail.txt
-# CHECK: XPASS: ignore-fail :: xpass.txt
+# CHECK-DAG: FAIL: ignore-fail :: fail.txt
+# CHECK-DAG: UNRESOLVED: ignore-fail :: unresolved.txt
+# CHECK-DAG: XFAIL: ignore-fail :: xfail.txt
+# CHECK-DAG: XPASS: ignore-fail :: xpass.txt
# CHECK: Testing Time:
# CHECK-NEXT: Expectedly Failed : 1
Index: llvm/utils/lit/tests/early-tests.py
===================================================================
--- llvm/utils/lit/tests/early-tests.py
+++ /dev/null
@@ -1,9 +0,0 @@
-## Check that we can run tests early.
-
-# RUN: %{lit} -j1 %{inputs}/early-tests | FileCheck %s
-
-# CHECK: -- Testing: 3 tests, 1 workers --
-# CHECK-NEXT: PASS: early-tests :: subdir/ccc.txt
-# CHECK-NEXT: PASS: early-tests :: aaa.txt
-# CHECK-NEXT: PASS: early-tests :: bbb.txt
-# CHECK: Passed: 3
Index: llvm/utils/lit/tests/Inputs/reorder/lit.cfg
===================================================================
--- llvm/utils/lit/tests/Inputs/reorder/lit.cfg
+++ llvm/utils/lit/tests/Inputs/reorder/lit.cfg
@@ -1,7 +1,6 @@
import lit.formats
-config.name = 'early-tests'
+config.name = 'reorder'
config.suffixes = ['.txt']
config.test_format = lit.formats.ShTest()
config.test_source_root = None
config.test_exec_root = None
-config.early_tests = { "subdir/ccc.txt" }
Index: llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
===================================================================
--- /dev/null
+++ llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
@@ -0,0 +1,3 @@
+3.0 subdir/ccc.txt
+2.0 bbb.txt
+0.1 aaa.txt
Index: llvm/utils/lit/lit/main.py
===================================================================
--- llvm/utils/lit/lit/main.py
+++ llvm/utils/lit/lit/main.py
@@ -71,6 +71,9 @@
opts.filter.search(t.getFullName()) and not
opts.filter_out.search(t.getFullName())]
+ #if opts.only_failures:
+ # selected_tests = [t for t in selected_tests if t.previous_failure]
+
if not selected_tests:
sys.stderr.write('error: filter did not match any tests '
'(of %d discovered). ' % len(discovered_tests))
@@ -105,6 +108,8 @@
run_tests(selected_tests, lit_config, opts, len(discovered_tests))
elapsed = time.time() - start
+ record_test_times(selected_tests, lit_config)
+
if opts.time_tests:
print_histogram(discovered_tests)
@@ -163,20 +168,12 @@
def determine_order(tests, order):
from lit.cl_arguments import TestOrder
- if order == TestOrder.EARLY_TESTS_THEN_BY_NAME:
- tests.sort(key=lambda t: (not t.isEarlyTest(), t.getFullName()))
- elif order == TestOrder.FAILING_FIRST:
- def by_mtime(test):
- return os.path.getmtime(test.getFilePath())
- tests.sort(key=by_mtime, reverse=True)
- elif order == TestOrder.RANDOM:
+ if order == TestOrder.RANDOM:
import random
random.shuffle(tests)
-
-
-def touch_file(test):
- if test.isFailure():
- os.utime(test.getFilePath(), None)
+ else:
+ assert order == TestOrder.DEFAULT
+ tests.sort(key=lambda t: (not t.previous_failure, -t.previous_elapsed, t.getFullName()))
def filter_by_shard(tests, run, shards, lit_config):
@@ -213,12 +210,7 @@
display = lit.display.create_display(opts, len(tests), discovered_tests,
workers)
- def progress_callback(test):
- display.update(test)
- if opts.order == lit.cl_arguments.TestOrder.FAILING_FIRST:
- touch_file(test)
-
- run = lit.run.Run(tests, lit_config, workers, progress_callback,
+ run = lit.run.Run(tests, lit_config, workers, display.update,
opts.max_failures, opts.timeout)
display.print_header()
@@ -267,6 +259,27 @@
lit_config.warning("Failed to delete temp directory '%s', try upgrading your version of Python to fix this" % tmp_dir)
+def record_test_times(tests, lit_config):
+ times_by_suite = {}
+ for t in tests:
+ if not t.result.elapsed:
+ continue
+ if not t.suite.exec_root in times_by_suite:
+ times_by_suite[t.suite.exec_root] = []
+ time = -t.result.elapsed if t.isFailure() else t.result.elapsed
+ times_by_suite[t.suite.exec_root].append((os.sep.join(t.path_in_suite), t.result.elapsed))
+
+ for s, value in times_by_suite.items():
+ try:
+ path = os.path.join(s, '.lit_test_times.txt')
+ with open(path, 'w') as file:
+ for name, time in value:
+ file.write(("%e" % time) + ' ' + name + '\n')
+ except:
+ lit_config.warning('Could not save test time: ' + path)
+ continue
+
+
def print_histogram(tests):
test_times = [(t.getFullName(), t.result.elapsed)
for t in tests if t.result.elapsed]
Index: llvm/utils/lit/lit/formats/googletest.py
===================================================================
--- llvm/utils/lit/lit/formats/googletest.py
+++ llvm/utils/lit/lit/formats/googletest.py
@@ -88,7 +88,7 @@
yield ''.join(nested_tests) + ln
def getTestsInDirectory(self, testSuite, path_in_suite,
- litConfig, localConfig):
+ litConfig, localConfig, test_times):
source_path = testSuite.getSourcePath(path_in_suite)
for subdir in self.test_sub_dirs:
dir_path = os.path.join(source_path, subdir)
Index: llvm/utils/lit/lit/formats/base.py
===================================================================
--- llvm/utils/lit/lit/formats/base.py
+++ llvm/utils/lit/lit/formats/base.py
@@ -11,7 +11,7 @@
class FileBasedTest(TestFormat):
def getTestsInDirectory(self, testSuite, path_in_suite,
- litConfig, localConfig):
+ litConfig, localConfig, test_times):
source_path = testSuite.getSourcePath(path_in_suite)
for filename in os.listdir(source_path):
# Ignore dot files and excluded tests.
@@ -23,8 +23,14 @@
if not os.path.isdir(filepath):
base,ext = os.path.splitext(filename)
if ext in localConfig.suffixes:
- yield lit.Test.Test(testSuite, path_in_suite + (filename,),
- localConfig)
+ full_path_in_suite = path_in_suite + (filename,)
+ test = lit.Test.Test(testSuite, full_path_in_suite, localConfig)
+ if os.sep.join(full_path_in_suite) in test_times:
+ time = test_times[os.sep.join(full_path_in_suite)]
+ test.previous_elapsed = abs(time)
+ test.previous_failure = time < 0
+ yield test
+
###
@@ -49,7 +55,7 @@
self.useTempInput = useTempInput
def getTestsInDirectory(self, testSuite, path_in_suite,
- litConfig, localConfig):
+ litConfig, localConfig, test_times):
dir = self.dir
if dir is None:
dir = testSuite.getSourcePath(path_in_suite)
Index: llvm/utils/lit/lit/discovery.py
===================================================================
--- llvm/utils/lit/lit/discovery.py
+++ llvm/utils/lit/lit/discovery.py
@@ -137,11 +137,24 @@
litConfig.note('resolved input %r to %r::%r' % (path, ts.name,
path_in_suite))
+ test_times = {}
+ test_times_file = os.path.join(ts.exec_root, '.lit_test_times.txt')
+ if not os.path.exists(test_times_file):
+ test_times_file = os.path.join(ts.source_root, '.lit_test_times.txt')
+ if litConfig.debug:
+ litConfig.note('using previous test time data: ' + test_times_file)
+ if os.path.exists(test_times_file):
+ with open(test_times_file, 'r') as file:
+ for line in file.readlines():
+ time, path = line.split(' ', 1)
+ test_times[path.strip('\n')] = float(time)
+
return ts, getTestsInSuite(ts, path_in_suite, litConfig,
- testSuiteCache, localConfigCache, indirectlyRunCheck)
+ testSuiteCache, localConfigCache,
+ indirectlyRunCheck, test_times)
-def getTestsInSuite(ts, path_in_suite, litConfig,
- testSuiteCache, localConfigCache, indirectlyRunCheck):
+def getTestsInSuite(ts, path_in_suite, litConfig, testSuiteCache,
+ localConfigCache, indirectlyRunCheck, test_times):
# Check that the source path exists (errors here are reported by the
# caller).
source_path = ts.getSourcePath(path_in_suite)
@@ -169,7 +182,8 @@
):
found = False
for res in lc.test_format.getTestsInDirectory(ts, test_dir_in_suite,
- litConfig, lc):
+ litConfig, lc,
+ test_times):
if test.getFullName() == res.getFullName():
found = True
break
@@ -198,7 +212,7 @@
# Search for tests.
if lc.test_format is not None:
for res in lc.test_format.getTestsInDirectory(ts, path_in_suite,
- litConfig, lc):
+ litConfig, lc, test_times):
yield res
# Search subdirectories.
@@ -235,10 +249,11 @@
if sub_ts is not None:
subiter = getTestsInSuite(sub_ts, subpath_in_suite, litConfig,
testSuiteCache, localConfigCache,
- indirectlyRunCheck)
+ indirectlyRunCheck, test_times)
else:
subiter = getTestsInSuite(ts, subpath, litConfig, testSuiteCache,
- localConfigCache, indirectlyRunCheck)
+ localConfigCache, indirectlyRunCheck,
+ test_times)
N = 0
for res in subiter:
Index: llvm/utils/lit/lit/cl_arguments.py
===================================================================
--- llvm/utils/lit/lit/cl_arguments.py
+++ llvm/utils/lit/lit/cl_arguments.py
@@ -9,8 +9,7 @@
class TestOrder(enum.Enum):
- EARLY_TESTS_THEN_BY_NAME = enum.auto()
- FAILING_FIRST = enum.auto()
+ DEFAULT = enum.auto()
RANDOM = enum.auto()
@@ -152,11 +151,14 @@
help="Maximum time to spend testing (in seconds)",
type=_positive_int)
selection_group.add_argument("--shuffle",
- help="Run tests in random order",
+ help="Start tests in random order",
action="store_true")
selection_group.add_argument("-i", "--incremental",
- help="Run modified and failing tests first (updates mtimes)",
+ help="Start failed tests first (DEPRECATED: now always enabled)",
action="store_true")
+ #selection_group.add_argument("--only-failures",
+ # help="Only run tests that failed the previous testing cycle",
+ # action="store_true")
selection_group.add_argument("--filter",
metavar="REGEX",
type=_case_insensitive_regex,
@@ -208,12 +210,13 @@
if opts.echoAllCommands:
opts.showOutput = True
+ if opts.incremental:
+ print('WARNING: --incremental was ignored. Failing tests now always start first.')
+
if opts.shuffle:
opts.order = TestOrder.RANDOM
- elif opts.incremental:
- opts.order = TestOrder.FAILING_FIRST
else:
- opts.order = TestOrder.EARLY_TESTS_THEN_BY_NAME
+ opts.order = TestOrder.DEFAULT
if opts.numShards or opts.runShard:
if not opts.numShards or not opts.runShard:
Index: llvm/utils/lit/lit/TestingConfig.py
===================================================================
--- llvm/utils/lit/lit/TestingConfig.py
+++ llvm/utils/lit/lit/TestingConfig.py
@@ -125,10 +125,6 @@
# require one of the features in this list if this list is non-empty.
# Configurations can set this list to restrict the set of tests to run.
self.limit_to_features = set(limit_to_features)
- # Whether the suite should be tested early in a given run.
- self.is_early = bool(is_early)
- # List of tests to run early.
- self.early_tests = {}
self.parallelism_group = parallelism_group
self._recursiveExpansionLimit = None
Index: llvm/utils/lit/lit/Test.py
===================================================================
--- llvm/utils/lit/lit/Test.py
+++ llvm/utils/lit/lit/Test.py
@@ -246,6 +246,12 @@
# The test result, once complete.
self.result = None
+ # The previous test failure state, if applicable.
+ self.previous_failure = False
+
+ # The previous test elapsed time, if applicable.
+ self.previous_elapsed = 0.0
+
def setResult(self, result):
assert self.result is None, "result already set"
assert isinstance(result, Result), "unexpected result type"
@@ -395,15 +401,3 @@
)
identifiers = set(filter(BooleanExpression.isIdentifier, tokens))
return identifiers
-
- def isEarlyTest(self):
- """
- isEarlyTest() -> bool
-
- Check whether this test should be executed early in a particular run.
- This can be used for test suites with long running tests to maximize
- parallelism or where it is desirable to surface their failures early.
- """
- if '/'.join(self.path_in_suite) in self.suite.config.early_tests:
- return True
- return self.suite.config.is_early
Index: llvm/test/Unit/lit.cfg.py
===================================================================
--- llvm/test/Unit/lit.cfg.py
+++ llvm/test/Unit/lit.cfg.py
@@ -13,9 +13,6 @@
# suffixes: A list of file extensions to treat as test files.
config.suffixes = []
-# is_early; Request to run this suite early.
-config.is_early = True
-
# test_source_root: The root path where tests are located.
# test_exec_root: The root path where tests should be run.
config.test_exec_root = os.path.join(config.llvm_obj_root, 'unittests')
Index: llvm/docs/CommandGuide/lit.rst
===================================================================
--- llvm/docs/CommandGuide/lit.rst
+++ llvm/docs/CommandGuide/lit.rst
@@ -20,7 +20,7 @@
command line. Tests can be either individual test files or directories to
search for tests (see :ref:`test-discovery`).
-Each specified test will be executed (potentially in parallel) and once all
+Each specified test will be executed (potentially concurrently) and once all
tests have been run :program:`lit` will print summary information on the number
of tests which passed or failed (see :ref:`test-status-results`). The
:program:`lit` program will execute with a non-zero exit code if any tests
@@ -151,8 +151,7 @@
Track the wall time individual tests take to execute and includes the results
in the summary output. This is useful for determining which tests in a test
- suite take the most time to execute. Note that this option is most useful
- with ``-j 1``.
+ suite take the most time to execute.
.. option:: --ignore-fail
@@ -168,6 +167,17 @@
SELECTION OPTIONS
-----------------
+By default, `lit` will start failing tests first, then start tests in descending
+execution time order to optimize concurrency.
+
+The timing data is stored in the `test_exec_root` in a file named
+`.lit_test_times.txt`. If this file does not exist, then `lit` checks the
+`test_source_root` for the file to optionally accelerate clean builds.
+
+.. option:: --shuffle
+
+ Start the tests in a random order, not failing/slowest first.
+
.. option:: --max-failures N
Stop execution after the given number ``N`` of failures.
@@ -201,10 +211,6 @@
must be in the range ``1..M``. The environment variable
``LIT_RUN_SHARD`` can also be used in place of this option.
-.. option:: --shuffle
-
- Run the tests in a random order.
-
.. option:: --timeout=N
Spend at most ``N`` seconds (approximately) running each individual test.
@@ -416,13 +422,6 @@
**root** The root configuration. This is the top-most :program:`lit` configuration in
the project.
- **is_early** Whether the test suite as a whole should be given a head start
- before other test suites run.
-
- **early_tests** An explicit set of '/' separated test paths that should be
- given a head start before other tests run. For example, the top five or so
- slowest tests. See also: `--time-tests`
-
**pipefail** Normally a test using a shell pipe fails if any of the commands
on the pipe fail. If this is not desired, setting this variable to false
makes the test fail only if the last command in the pipe fails.
Index: lldb/test/API/lldbtest.py
===================================================================
--- lldb/test/API/lldbtest.py
+++ lldb/test/API/lldbtest.py
@@ -16,7 +16,7 @@
self.dotest_cmd = dotest_cmd
def getTestsInDirectory(self, testSuite, path_in_suite, litConfig,
- localConfig):
+ localConfig, test_times):
source_path = testSuite.getSourcePath(path_in_suite)
for filename in os.listdir(source_path):
# Ignore dot files and excluded tests.
Index: libcxx/utils/libcxx/test/googlebenchmark.py
===================================================================
--- libcxx/utils/libcxx/test/googlebenchmark.py
+++ libcxx/utils/libcxx/test/googlebenchmark.py
@@ -70,7 +70,7 @@
yield ''.join(nested_tests) + ln
def getTestsInDirectory(self, testSuite, path_in_suite,
- litConfig, localConfig):
+ litConfig, localConfig, test_times):
source_path = testSuite.getSourcePath(path_in_suite)
for subdir in self.test_sub_dirs:
dir_path = os.path.join(source_path, subdir)
Index: libcxx/utils/libcxx/test/format.py
===================================================================
--- libcxx/utils/libcxx/test/format.py
+++ libcxx/utils/libcxx/test/format.py
@@ -194,7 +194,8 @@
Equivalent to `%{exec} %t.exe`. This is intended to be used
in conjunction with the %{build} substitution.
"""
- def getTestsInDirectory(self, testSuite, pathInSuite, litConfig, localConfig):
+ def getTestsInDirectory(self, testSuite, pathInSuite, litConfig,
+ localConfig, test_times):
SUPPORTED_SUFFIXES = ['[.]pass[.]cpp$', '[.]pass[.]mm$',
'[.]compile[.]pass[.]cpp$', '[.]compile[.]fail[.]cpp$',
'[.]link[.]pass[.]cpp$', '[.]link[.]fail[.]cpp$',
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
