Simon Jakobi pushed to branch wip/sjakobi/T16720 at Glasgow Haskell Compiler / GHC Commits: 11001cf8 by Simon Jakobi at 2026-08-03T20:03:51+02:00 testsuite: Don't crash on non-UTF-8 test output read_stdout, read_stderr_for, read_comp_stderr and read_diff decoded strictly (the first three with UTF-8, read_diff with the locale encoding), so a test emitting invalid UTF-8 (binary output, or a crash truncating a multi-byte character) raised UnicodeDecodeError and was reported as a framework failure instead of its actual result. Decode with errors='replace', like read_no_crs and safe_print. Assisted-by: Claude Fable 5 - - - - - eeb76b33 by Simon Jakobi at 2026-08-03T20:03:51+02:00 testsuite: Colorize the test summary, also in CI The summary headings were plain, and SUMMARY was colored unconditionally, so the escapes also ended up in the file written by --summary-file. Color is now decided per output sink via term_color.colored_if; see the comments in term_color. CI logs are not a tty, but GitLab's log viewer renders ANSI colors, so add --force-colors and pass it in .gitlab/ci.sh. Assisted-by: Claude Opus 5 - - - - - 0c34ab3c by Simon Jakobi at 2026-08-03T20:03:51+02:00 testsuite: Repeat unexpected failure output in the summary Finding out why a test failed meant scrolling back through a possibly very long log to the point where the test ran. The summary now repeats the captured output of unexpected failures, before the statistics, so the most interesting part is at the end of the log (#16720). Output mismatches report their diff instead of the mismatching stream (see Note [Redundant output in test results]). The repeated output is bounded per stream, and skipped altogether beyond MAX_SUMMARY_OUTPUT_TESTS failure blocks. Tests failing identically in several ways share one block. Test results now report a source-relative directory, stable regardless of where the run was started from. Assisted-by: Claude Fable 5 - - - - - 4 changed files: - .gitlab/ci.sh - testsuite/driver/runtests.py - testsuite/driver/term_color.py - testsuite/driver/testlib.py Changes: ===================================== .gitlab/ci.sh ===================================== @@ -652,6 +652,10 @@ function test_hadrian() { check_msys2_deps _build/stage1/bin/ghc --version check_release_build + # GitLab's log viewer renders ANSI colors, but stdout here is not a tty, + # so the driver must be told to emit them. + RUNTEST_ARGS="${RUNTEST_ARGS:-} --force-colors" + # Ensure that statically-linked builds are actually static if [[ "${BUILD_FLAVOUR}" = *static* ]]; then bad_execs="" ===================================== testsuite/driver/runtests.py ===================================== @@ -94,6 +94,8 @@ parser.add_argument("--ignore-perf-failures", choices=['increases','decreases',' help="Do not fail due to out-of-tolerance perf tests") parser.add_argument("--only-report-hadrian-deps", type=Path, help="Dry run the testsuite and report all extra hadrian dependencies needed on the given file") +parser.add_argument("--force-colors", action="store_true", + help="emit ANSI colors even when stdout is not a tty (e.g. for CI logs)") args = parser.parse_args() @@ -259,7 +261,9 @@ def supports_colors(): return True config.supports_colors = supports_colors() -term_color.enable_color = config.supports_colors +# config.supports_colors deliberately stays tty-based: it also guards +# terminal-title updates, which must not end up in a CI log. +term_color.enable_color = config.supports_colors or args.force_colors # This has to come after arg parsing as the args can change the compiler get_compiler_info() @@ -587,7 +591,7 @@ else: print(Perf.allow_changes_string([(m.change, m.stat) for m in t.metrics])) print('-' * 25) - summary(t, sys.stdout, color=config.supports_colors) + summary(t, sys.stdout, color=term_color.enable_color, junit_path=args.junit) # Write perf stats if any exist or if a metrics file is specified. stats_metrics = [stat for (_, stat, __) in t.metrics] # type: List[PerfStat] ===================================== testsuite/driver/term_color.py ===================================== @@ -1,5 +1,6 @@ from enum import Enum +# Whether to emit color escapes; set in runtests.py. enable_color = True class Color(Enum): @@ -18,3 +19,7 @@ def colored(color: Color, s: str) -> str: else: return s +# For renderers that serve several sinks: `enabled` says whether *this* sink +# takes color (the summary is written both to stdout and to a plain-text file). +def colored_if(enabled: bool, color: Color, s: str) -> str: + return colored(color, s) if enabled else s ===================================== testsuite/driver/testlib.py ===================================== @@ -27,7 +27,7 @@ from testutil import strip_quotes, lndir, link_or_copy_file, passed, \ failBecause, testing_metrics, residency_testing_metrics, \ stable_perf_counters, \ PassFail, badResult, str_warn, str_removeprefix -from term_color import Color, colored +from term_color import Color, colored_if import testutil from cpu_features import have_cpu_feature import perf_notes as Perf @@ -1497,6 +1497,19 @@ def _newTestDir(name: TestName, opts: TestOptions, tempdir, dir): opts.testdir_raw = Path(os.path.join(tempdir, testdir, name + testdir_suffix)) opts.compiler_always_flags = config.compiler_always_flags +def _result_directory(opts: TestOptions) -> str: + # The test's source directory, relative to the GHC source root, so it reads + # the same regardless of which directory `make` was invoked from. + srcdir = opts.srcdir + if srcdir is None: + return '' + try: + return os.path.relpath(srcdir, config.top.parent) + except ValueError: + # No relative path exists (e.g. different Windows drives); the + # absolute path is still more useful than nothing. + return str(srcdir) + # ----------------------------------------------------------------------------- # Actually doing tests @@ -1821,7 +1834,7 @@ async def do_test(name: TestName, if opts.expect not in ['pass', 'fail', 'missing-lib']: framework_fail(name, way, 'bad expected ' + opts.expect) - directory = str_removeprefix(str_removeprefix(str(opts.testdir), './'), '.\\') + directory = _result_directory(opts) if way in opts.fragile_ways: if_verbose(1, '*** fragile test %s resulted in %s' % (full_name, 'pass' if result.passed else 'fail')) @@ -1875,7 +1888,7 @@ def framework_fail(name: Optional[TestName], way: Optional[WayName], reason: str # so we need to take care not to blow up with the wrong way # and report the actual reason for the failure. try: - directory = str_removeprefix(str_removeprefix(str(opts.testdir), './'), '.\\') + directory = _result_directory(opts) except: directory = '' full_name = '%s(%s)' % (name, way) @@ -1888,7 +1901,7 @@ def framework_fail(name: Optional[TestName], way: Optional[WayName], reason: str def framework_warn(name: TestName, way: WayName, reason: str) -> None: opts = getTestOpts() - directory = str_removeprefix(str_removeprefix(str(opts.testdir), './'), '.\\') + directory = _result_directory(opts) full_name = name + '(' + way + ')' if_verbose(1, '*** framework warning for %s %s ' % (full_name, reason)) t.framework_warnings.append(TestResult(directory, name, reason, way)) @@ -2443,19 +2456,23 @@ async def simple_run(name: TestName, way: WayName, prog: str, extra_run_opts: st dump_stdout(name) dump_stderr(name) message = format_bad_exit_code_message(exit_code) - return failBecause(message) + return failBecause(message, + stderr=read_stderr(name), + stdout=read_stdout(name)) stderr_match = CompareOutput(True) if (opts.ignore_stderr or opts.combined_output) else await stderr_ok(name, way) if not stderr_match: + # The diff already contains the mismatching stream; see Note [Redundant + # output in test results]. return failBecause('bad stderr', - stderr=read_stderr(name), + stderr=None if stderr_match.diff else read_stderr(name), stdout=read_stdout(name), diff=stderr_match.diff) stdout_match = CompareOutput(True) if opts.ignore_stdout else await stdout_ok(name, way) if not stdout_match: return failBecause('bad stdout', stderr=read_stderr(name), - stdout=read_stdout(name), + stdout=None if stdout_match.diff else read_stdout(name), diff=stdout_match.diff) check_hp = '-hT' in my_rts_flags and opts.check_hp @@ -2565,8 +2582,9 @@ async def interpreter_run(name: TestName, if not stderr_match: if _expect_pass(way): dump_stderr_for('comp', name) + # See Note [Redundant output in test results]. return failBecause('bad stderr', - stderr=read_stderr(name), + stderr=None if stderr_match.diff else read_stderr(name), stdout=read_stdout(name), diff=stderr_match.diff) stdout_match = CompareOutput(True) if opts.ignore_stdout else await stdout_ok(name, way) @@ -2575,7 +2593,7 @@ async def interpreter_run(name: TestName, dump_stderr_for('comp', name) return failBecause('bad stdout', stderr=read_stderr(name), - stdout=read_stdout(name), + stdout=None if stdout_match.diff else read_stdout(name), diff=stdout_match.diff) return passed() @@ -2633,13 +2651,13 @@ async def stdout_ok(name: TestName, way: WayName) -> CompareOutput: def read_stdout( name: TestName ) -> str: path = in_testdir(name, 'run.stdout') if path.exists(): - return path.read_text(encoding='UTF-8') + return path.read_text(encoding='UTF-8', errors='replace') else: return '' def read_diff( diff_file: Path ) -> Optional[str]: if diff_file.exists(): - diff = diff_file.read_text() + diff = diff_file.read_text(encoding='UTF-8', errors='replace') diff_file.unlink() return diff or None else: @@ -2663,14 +2681,14 @@ async def stderr_ok(name: TestName, way: WayName) -> CompareOutput: def read_comp_stderr( name: TestName ) -> str: path = in_testdir(name, 'comp.stderr') if path.exists(): - return path.read_text(encoding='UTF-8') + return path.read_text(encoding='UTF-8', errors='replace') else: return '' def read_stderr_for( phase: str, name: TestName ) -> str: path = in_testdir(name, phase + '.stderr') if path.exists(): - return path.read_text(encoding='UTF-8') + return path.read_text(encoding='UTF-8', errors='replace') else: return '' @@ -3569,12 +3587,50 @@ def findTFiles(roots: List[str]) -> Iterator[str]: # ----------------------------------------------------------------------------- # Output a test summary to the specified file object -def summary(t: TestRun, file: TextIO, color=False) -> None: +def summary(t: TestRun, file: TextIO, color=False, junit_path: Optional[Path]=None) -> None: file.write('\n') + + if t.unexpected_failures: + # Count output blocks rather than results: a test failing in many ways + # collapses to a single block. + groups = groupTestOutput(t.unexpected_failures) + if len(groups) <= MAX_SUMMARY_OUTPUT_TESTS: + printTestOutputSummary(file, groups, color, junit_path) + else: + where = '; see {}'.format(junit_path) if junit_path else '' + header = ('Unexpected failures (more than {}, output omitted{}):' + .format(MAX_SUMMARY_OUTPUT_TESTS, where)) + file.write(colored_if(color, Color.RED, header) + '\n') + printTestInfosSummary(file, t.unexpected_failures) + + if t.unexpected_passes: + header = 'Unexpected passes:' + file.write(colored_if(color, Color.RED, header) + '\n') + printTestInfosSummary(file, t.unexpected_passes) + + if t.unexpected_stat_failures: + header = 'Unexpected stat failures:' + file.write(colored_if(color, Color.RED, header) + '\n') + printTestInfosSummary(file, t.unexpected_stat_failures) + + if t.framework_failures: + header = 'Framework failures:' + file.write(colored_if(color, Color.RED, header) + '\n') + printTestInfosSummary(file, t.framework_failures) + + if t.framework_warnings: + header = 'Framework warnings:' + file.write(colored_if(color, Color.YELLOW, header) + '\n') + printTestInfosSummary(file, t.framework_warnings) + + if stopping(): + warning = 'WARNING: Testsuite run was terminated early' + file.write(colored_if(color, Color.YELLOW, warning) + '\n') + printUnexpectedTests(file, [t.unexpected_passes, t.unexpected_failures, - t.unexpected_stat_failures, t.framework_failures]) + t.unexpected_stat_failures, t.framework_failures], color) if len(t.unexpected_failures) > 0 or \ len(t.unexpected_stat_failures) > 0 or \ @@ -3585,7 +3641,8 @@ def summary(t: TestRun, file: TextIO, color=False) -> None: summary_color = Color.GREEN assert t.start_time is not None - file.write(colored(summary_color, 'SUMMARY') + ' for test run started at ' + summary_header = colored_if(color, summary_color, 'SUMMARY') + file.write(summary_header + ' for test run started at ' + t.start_time.strftime("%c %Z") + '\n' + str(datetime.datetime.now() - t.start_time).rjust(8) + ' spent to go through\n' @@ -3617,46 +3674,107 @@ def summary(t: TestRun, file: TextIO, color=False) -> None: + ' fragile tests\n' + '\n') - if t.unexpected_passes: - file.write('Unexpected passes:\n') - printTestInfosSummary(file, t.unexpected_passes) - - if t.unexpected_failures: - file.write('Unexpected failures:\n') - printTestInfosSummary(file, t.unexpected_failures) - - if t.unexpected_stat_failures: - file.write('Unexpected stat failures:\n') - printTestInfosSummary(file, t.unexpected_stat_failures) - - if t.framework_failures: - file.write('Framework failures:\n') - printTestInfosSummary(file, t.framework_failures) - - if t.framework_warnings: - file.write('Framework warnings:\n') - printTestInfosSummary(file, t.framework_warnings) - - if stopping(): - file.write('WARNING: Testsuite run was terminated early\n') - -def printUnexpectedTests(file: TextIO, testInfoss): +def printUnexpectedTests(file: TextIO, testInfoss, color=False): unexpected = set(result.testname for testInfos in testInfoss for result in testInfos if not result.testname.endswith('.T')) if unexpected: - file.write('Unexpected results from:\n') + header = 'Unexpected results from:' + file.write(colored_if(color, Color.RED, header) + '\n') file.write('TEST="' + ' '.join(sorted(unexpected)) + '"\n') file.write('\n') +# Per-stream cap on a failing test's output repeated in the final summary. +MAX_SUMMARY_OUTPUT_LINES = 100 + +# Above this many output blocks, skip repeating output entirely: the dump +# would drown out the summary. +MAX_SUMMARY_OUTPUT_TESTS = 20 + +# Note [Redundant output in test results] +# ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +# A failing test result carries up to three pieces of output: `diff`, `stdout` +# and `stderr`. For an output mismatch these overlap: the diff's `+` lines are +# the very stream that mismatched, normalised. Reporting both would print the +# same text twice, so the mismatching stream is dropped at the call sites in +# favour of the diff, which additionally shows what was expected. The *other* +# stream is kept: on a stdout mismatch, stderr is independent context. +# +# The drop is conditional on there being a diff at all: compare_outputs only +# runs `diff` when config.verbose >= 1, so under -v0 the stream is the only +# output there is. +# +# Note that, since the drop happens at result construction, it also affects the +# JUnit report (junit.py). + +def strip_diff_header(diff: Optional[str]) -> Optional[str]: + # Drop diff(1)'s ---/+++ lines: they name normalised files in the test + # directory and carry timestamps, which would also keep otherwise + # identical failures from being grouped. + if diff is None: + return None + lines = diff.split('\n') + if len(lines) >= 2 and lines[0].startswith('--- ') and lines[1].startswith('+++ '): + return '\n'.join(lines[2:]) + return diff + +def sorted_results(testInfos: List[TestResult]) -> List[TestResult]: + return sorted(testInfos, key=lambda r: (r.testname.lower(), r.directory, r.way)) + +# A failure-output block: a representative result, its header-stripped diff, +# and the ways that share it. +OutputGroup = Tuple[TestResult, Optional[str], List[WayName]] + +# Tests that fail identically in several ways (e.g. normal and g1) share one +# output block, with the ways collected in the header. +def groupTestOutput(testInfos: List[TestResult]) -> List[OutputGroup]: + # Relies on dicts preserving insertion order. + groups = {} # type: Dict[Tuple, OutputGroup] + for result in sorted_results(testInfos): + diff = strip_diff_header(result.diff) + key = (result.testname, result.directory, result.reason, + diff, result.stdout, result.stderr) + groups.setdefault(key, (result, diff, []))[2].append(result.way) + return list(groups.values()) + +def printTestOutputSummary(file: TextIO, + groups: List[OutputGroup], + color: bool=False, + junit_path: Optional[Path]=None) -> None: + # Repeat failing tests' captured output in the summary, so one needn't + # hunt for it earlier in a possibly very long log; see #16720. + header = '=====> Unexpected failures output summary' + file.write(colored_if(color, Color.RED, header) + '\n\n') + + where = ', see {}'.format(junit_path) if junit_path else '' + for result, diff, ways in groups: + header = '=====> {}({}) ({}) [{}]'.format( + result.testname, ', '.join(ways), result.directory + os.sep, result.reason) + file.write(colored_if(color, Color.RED, header) + '\n') + # See Note [Redundant output in test results] for why these don't overlap. + for label, contents in [('Output diff (expected vs actual):', diff), + ('Captured stdout:', result.stdout), + ('Captured stderr:', result.stderr)]: + if contents and contents.strip(): + lines = contents.rstrip('\n').split('\n') + if len(lines) > MAX_SUMMARY_OUTPUT_LINES: + omitted = len(lines) - MAX_SUMMARY_OUTPUT_LINES + lines = lines[:MAX_SUMMARY_OUTPUT_LINES] \ + + ['... ({} more lines omitted{})'.format(omitted, where)] + s = colored_if(color, Color.CYAN, label) + '\n' \ + + ''.join(l + '\n' for l in lines) + # Test output can contain characters that file's encoding + # cannot represent; replace rather than crash (cf safe_print). + enc = getattr(file, 'encoding', None) or 'utf-8' + file.write(s.encode(enc, errors='replace').decode(enc)) + footer = '<===== end of unexpected failures output summary' + file.write(colored_if(color, Color.RED, footer) + '\n\n') + def printTestInfosSummary(file: TextIO, testInfos): - maxDirLen = max(len(tr.directory) for tr in testInfos) - for result in sorted(testInfos, key=lambda r: (r.testname.lower(), r.way, r.directory)): - directory = result.directory.ljust(maxDirLen) - file.write(' {directory} {r.testname} [{r.reason}] ({r.way})\n'.format( - r = result, - directory = directory)) + for result in sorted_results(testInfos): + path = os.path.join(result.directory, result.testname) + file.write(' {path} [{r.reason}] ({r.way})\n'.format(r=result, path=path)) file.write('\n') def modify_lines(s: str, f: Callable[[str], str]) -> str: View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/997fbe2ea9795e69c31275dfdbf0701... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/997fbe2ea9795e69c31275dfdbf0701... You're receiving this email because of your account on gitlab.haskell.org. Manage all notifications: https://gitlab.haskell.org/-/profile/notifications | Help: https://gitlab.haskell.org/help