From d1b9a2969ff526223febc4913471874a9d5ca156 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 6 Nov 2023 01:25:27 -0600 Subject: [PATCH] Added -F/--failures to test.py/bench.py to limit failures when -k/--keep-going The -k/--keep-going option has been more or less useless before this since it would completely flood the screen/logs when a bug triggers multiple test failures, which is common. Some things to note: - RAM management is tricky with -k/--keep-going, if we try to save logs and filter after running everything we quickly fill up memory. - Failing test cases are a much slower path than successes since we need to kill and restart the underlying test_runner, its state can't be trusted anymore. This is a-ok since hopefully you usually hope for many more successes than failures. Unfortunately it can make -k/--keep-going quite slow. --- ALSO -- warning this is a tangent rant-into-the-void -- I have discovered that Ubuntu has a "helpful" subsystem named Apport that tries to record/log/report any process crash in the system. It is "disabled" by default, but the way it's disabled requires LAUNCHING A PYTHON INTERPRETER to check a flag on every segfault/assert failure. This is what it does when it's "disabled"! This subsystem is fundamentally incompatible with any program that intentionally crashes subprocesses, such as our test runner. The sheer amount of python interpreters being launched quickly eats through all available RAM and starts OOM killing half the processes on the system. If anyone else runs into this, a shallow bit of googling suggests the best solution is to just disable Apport. It is not a developer friendly subsystem: $ sudo systemctl disable apport.service Removing Apport brings RAM usage back down to a constant level, even with absurd numbers of test failures. And here I thought I had memory leak somewhere. --- scripts/bench.py | 41 +++++++++++++++++++++++++++++++---------- scripts/test.py | 43 ++++++++++++++++++++++++++++++++----------- 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/scripts/bench.py b/scripts/bench.py index 65c6a67b..f797e198 100755 --- a/scripts/bench.py +++ b/scripts/bench.py @@ -965,6 +965,7 @@ def run_stage(name, runner, bench_ids, stdout_, trace_, output_, **args): passed_suite_perms = co.defaultdict(lambda: 0) passed_case_perms = co.defaultdict(lambda: 0) passed_perms = 0 + failed_perms = 0 readed = 0 proged = 0 erased = 0 @@ -1111,6 +1112,7 @@ def run_stage(name, runner, bench_ids, stdout_, trace_, output_, **args): last_assert) def run_job(start=None, step=None): + nonlocal failed_perms nonlocal failures nonlocal killed nonlocal locals @@ -1138,7 +1140,13 @@ def run_stage(name, runner, bench_ids, stdout_, trace_, output_, **args): if failures and not args.get('keep_going'): break - failures.append(failure) + # keep track of how many failed + failed_perms += 1 + + # do not store more failures than we need to, otherwise we + # quickly explode RAM when a common bug fails a bunch of cases + if len(failures) < args.get('failures', 3): + failures.append(failure) if args.get('keep_going') and not killed: # resume after failed bench @@ -1170,7 +1178,7 @@ def run_stage(name, runner, bench_ids, stdout_, trace_, output_, **args): sys.stdout.write('%s%srunning %s%s:%s %s%s' % ( '\r\x1b[K' if args['color'] else '', '\x1b[?7l' if not done else '', - ('\x1b[34m' if not failures else '\x1b[31m') + ('\x1b[34m' if not failed_perms else '\x1b[31m') if args['color'] else '', name, '\x1b[m' if args['color'] else '', @@ -1189,10 +1197,10 @@ def run_stage(name, runner, bench_ids, stdout_, trace_, output_, **args): '%d/%d perms' % (passed_perms, expected_perms), '%s%d/%d failures%s' % ( '\x1b[31m' if args['color'] else '', - len(failures), + failed_perms, expected_perms, '\x1b[m' if args['color'] else '') - if failures else None])), + if failed_perms else None])), '\x1b[?7h' if not done else '\n')) sys.stdout.flush() @@ -1216,6 +1224,7 @@ def run_stage(name, runner, bench_ids, stdout_, trace_, output_, **args): return ( expected_perms, passed_perms, + failed_perms, readed, proged, erased, @@ -1266,6 +1275,7 @@ def run(runner, bench_ids=[], **args): # spawn runners expected = 0 passed = 0 + failed = 0 readed = 0 proged = 0 erased = 0 @@ -1274,6 +1284,7 @@ def run(runner, bench_ids=[], **args): # spawn jobs for stage (expected_, passed_, + failed_, readed_, proged_, erased_, @@ -1289,11 +1300,16 @@ def run(runner, bench_ids=[], **args): # collect passes/failures expected += expected_ passed += passed_ + failed += failed_ readed += readed_ proged += proged_ erased += erased_ - failures.extend(failures_) - if (failures and not args.get('keep_going')) or killed: + # do not store more failures than we need to, otherwise we + # quickly explode RAM when a common bug fails a bunch of cases + failures.extend(failures_[:max( + args.get('failures', 3) - len(failures), + 0)]) + if (failed and not args.get('keep_going')) or killed: break stop = time.time() @@ -1314,7 +1330,7 @@ def run(runner, bench_ids=[], **args): # show summary print() print('%sdone:%s %s' % ( - ('\x1b[34m' if not failures else '\x1b[31m') + ('\x1b[34m' if not failed else '\x1b[31m') if args['color'] else '', '\x1b[m' if args['color'] else '', ', '.join(filter(None, [ @@ -1325,7 +1341,7 @@ def run(runner, bench_ids=[], **args): print() # print each failure - for failure in failures: + for failure in failures[:args.get('failures', 3)]: assert failure.id is not None, '%s broken? %r' % ( ' '.join(shlex.quote(c) for c in find_runner(runner, **args)), failure) @@ -1348,7 +1364,7 @@ def run(runner, bench_ids=[], **args): stdout = failure.stdout if failure.assert_ is not None: stdout = stdout[:-1] - for line in stdout[-args.get('context', 5):]: + for line in stdout[len(stdout)-args.get('context', 5):]: sys.stdout.write(line) if failure.assert_ is not None: @@ -1402,7 +1418,7 @@ def run(runner, bench_ids=[], **args): print(' '.join(shlex.quote(c) for c in cmd)) os.execvp(cmd[0], cmd) - return 1 if failures else 0 + return 1 if failed else 0 def main(**args): @@ -1556,6 +1572,11 @@ if __name__ == "__main__": '-B', '--by-cases', action='store_true', help="Step through benches by case.") + bench_parser.add_argument( + '-F', '--failures', + type=lambda x: int(x, 0), + default=3, + help="Show this many test failures. Defaults to 3.") bench_parser.add_argument( '--context', type=lambda x: int(x, 0), diff --git a/scripts/test.py b/scripts/test.py index b6b5d7c6..492dcaf8 100755 --- a/scripts/test.py +++ b/scripts/test.py @@ -974,6 +974,7 @@ def run_stage(name, runner, test_ids, stdout_, trace_, output_, **args): passed_suite_perms = co.defaultdict(lambda: 0) passed_case_perms = co.defaultdict(lambda: 0) passed_perms = 0 + failed_perms = 0 powerlosses = 0 failures = [] killed = False @@ -1078,6 +1079,7 @@ def run_stage(name, runner, test_ids, stdout_, trace_, output_, **args): last_assert) def run_job(start=None, step=None): + nonlocal failed_perms nonlocal failures nonlocal killed nonlocal locals @@ -1117,7 +1119,13 @@ def run_stage(name, runner, test_ids, stdout_, trace_, output_, **args): if failures and not args.get('keep_going'): break - failures.append(failure) + # keep track of how many failed + failed_perms += 1 + + # do not store more failures than we need to, otherwise we + # quickly explode RAM when a common bug fails a bunch of cases + if len(failures) < args.get('failures', 3): + failures.append(failure) if args.get('keep_going') and not killed: # resume after failed test @@ -1149,7 +1157,7 @@ def run_stage(name, runner, test_ids, stdout_, trace_, output_, **args): sys.stdout.write('%s%srunning %s%s:%s %s%s' % ( '\r\x1b[K' if args['color'] else '', '\x1b[?7l' if not done else '', - ('\x1b[32m' if not failures else '\x1b[31m') + ('\x1b[32m' if not failed_perms else '\x1b[31m') if args['color'] else '', name, '\x1b[m' if args['color'] else '', @@ -1170,10 +1178,10 @@ def run_stage(name, runner, test_ids, stdout_, trace_, output_, **args): if powerlosses else None, '%s%d/%d failures%s' % ( '\x1b[31m' if args['color'] else '', - len(failures), + failed_perms, expected_perms, '\x1b[m' if args['color'] else '') - if failures else None])), + if failed_perms else None])), '\x1b[?7h' if not done else '\n')) sys.stdout.flush() @@ -1197,6 +1205,7 @@ def run_stage(name, runner, test_ids, stdout_, trace_, output_, **args): return ( expected_perms, passed_perms, + failed_perms, powerlosses, failures, killed) @@ -1244,12 +1253,14 @@ def run(runner, test_ids=[], **args): # spawn runners expected = 0 passed = 0 + failed = 0 powerlosses = 0 failures = [] for by in (test_ids if test_ids else [None]): # spawn jobs for stage (expected_, passed_, + failed_, powerlosses_, failures_, killed) = run_stage( @@ -1263,9 +1274,14 @@ def run(runner, test_ids=[], **args): # collect passes/failures expected += expected_ passed += passed_ + failed += failed_ powerlosses += powerlosses_ - failures.extend(failures_) - if (failures and not args.get('keep_going')) or killed: + # do not store more failures than we need to, otherwise we + # quickly explode RAM when a common bug fails a bunch of cases + failures.extend(failures_[:max( + args.get('failures', 3) - len(failures), + 0)]) + if (failed and not args.get('keep_going')) or killed: break stop = time.time() @@ -1286,18 +1302,18 @@ def run(runner, test_ids=[], **args): # show summary print() print('%sdone:%s %s' % ( - ('\x1b[32m' if not failures else '\x1b[31m') + ('\x1b[32m' if not failed else '\x1b[31m') if args['color'] else '', '\x1b[m' if args['color'] else '', ', '.join(filter(None, [ '%d/%d passed' % (passed, expected), - '%d/%d failed' % (len(failures), expected), + '%d/%d failed' % (failed, expected), '%dpls!' % powerlosses if powerlosses else None, 'in %.2fs' % (stop-start)])))) print() # print each failure - for failure in failures: + for failure in failures[:args.get('failures', 3)]: assert failure.id is not None, '%s broken? %r' % ( ' '.join(shlex.quote(c) for c in find_runner(runner, **args)), failure) @@ -1320,7 +1336,7 @@ def run(runner, test_ids=[], **args): stdout = failure.stdout if failure.assert_ is not None: stdout = stdout[:-1] - for line in stdout[-args.get('context', 5):]: + for line in stdout[len(stdout)-args.get('context', 5):]: sys.stdout.write(line) if failure.assert_ is not None: @@ -1408,7 +1424,7 @@ def run(runner, test_ids=[], **args): print(' '.join(shlex.quote(c) for c in cmd)) os.execvp(cmd[0], cmd) - return 1 if failures else 0 + return 1 if failed else 0 def main(**args): @@ -1570,6 +1586,11 @@ if __name__ == "__main__": '-B', '--by-cases', action='store_true', help="Step through tests by case.") + test_parser.add_argument( + '-F', '--failures', + type=lambda x: int(x, 0), + default=3, + help="Show this many test failures. Defaults to 3.") test_parser.add_argument( '--context', type=lambda x: int(x, 0),