yln added a comment.
Very happy with the patch! Thanks for all your hard work! :)
I have a few small nits and one ask: can we model "test failed" explicitly
instead of making their execution time really large?
================
Comment at: llvm/utils/lit/lit/discovery.py:141
+ test_times = {}
+ test_times_file = ts.exec_root + os.sep + '.lit_test_times.txt'
+ if not os.path.exists(test_times_file):
----------------
Use `os.path.join(...)`
================
Comment at: llvm/utils/lit/lit/discovery.py:145-149
+ file = open(test_times_file, 'r')
+ for line in file.readlines():
+ time, path = line.split(' ', 1)
+ test_times[path.strip('\n')] = float(time)
+ file.close()
----------------
Use Python's `with` statement
================
Comment at: llvm/utils/lit/lit/main.py:108
+ record_test_times(discovered_tests, lit_config)
+
----------------
I think it would be sufficient here to pass in the tests that we attempted to
run.
================
Comment at: llvm/utils/lit/lit/main.py:166
def determine_order(tests, order):
from lit.cl_arguments import TestOrder
----------------
Really like all the simplifications this enabled! :)
================
Comment at: llvm/utils/lit/lit/main.py:213
run = lit.run.Run(tests, lit_config, workers, progress_callback,
opts.max_failures, opts.timeout)
----------------
Is it possible to directly pass the display callback here now (and remove the
local `progress_callback()`)?
================
Comment at: llvm/utils/lit/lit/main.py:270-271
+ time = t.result.elapsed
+ if t.isFailure():
+ time += 1.0e10
+
times_by_suite[t.suite.exec_root].append((os.sep.join(t.path_in_suite),
t.result.elapsed))
----------------
Can we explicitly model this part in code instead of increasing the execution
time of failed tests? We can use the `test.result.code` property.
================
Comment at: llvm/utils/lit/lit/main.py:274-275
+
+ if not times_by_suite:
+ return
+
----------------
Not really needed and can be removed, right?
================
Comment at: llvm/utils/lit/lit/main.py:277-286
+ for s, value in times_by_suite.items():
+ try:
+ path = s + os.sep + '.lit_test_times.txt'
+ file = open(path, 'w')
+ for name, time in value:
+ file.write(("%e" % time) + ' ' + name + '\n')
+ file.close()
----------------
Using Python's `with` and `os.path` and string interpolation
================
Comment at: llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt:3
+2.0 bbb.txt
+0.1 aaa.txt
----------------
Clarifying my above comment:
```
PASS 3.0 subdir/ccc.txt
FAIL 2.0 bbb.txt
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98179/new/
https://reviews.llvm.org/D98179
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
