GitLab

Marge Bot pushed to branch master at Glasgow Haskell Compiler / GHC

Commits:

29032f17

by Zubin Duggal at 2026-07-09T09:24:58-04:00

testsuite: Keep real reason for fragile test failures

c34e03a7

by Zubin Duggal at 2026-07-09T09:24:58-04:00

testsuite: Fall back to the failure reason for empty JUnit bodies

409d40f0

by Zubin Duggal at 2026-07-09T09:24:58-04:00

testsuite: Show output diffs in JUnit output

Also refactor compare_outputs to return essentially a `Maybe Diff`
(`CompareOutput`) instead of a bool, but more pythonic. This
allows us to pass the diff through nice.

06fee1ab

by Zubin Duggal at 2026-07-09T09:24:58-04:00

perf notes: include stat deviation and acceptance window in notes so they show up in gitlab

Changes:

testsuite/driver/junit.py

@@ -26,10 +26,14 @@ def junit(t: TestRun) -> ET.ElementTree:
                                       classname = tr.way,
                                       name = '%s(%s)' % (tr.testname, tr.way))
              message = [] # type: List[str]
 +            if tr.diff:
 +                message += ['diff:', '==========', tr.diff]
              if tr.stdout:
                  message += ['', 'stdout:', '==========', tr.stdout]
              if tr.stderr:
                  message += ['', 'stderr:', '==========', tr.stderr]
 +            if not message:
 +                message = [tr.reason]
              result = ET.SubElement(testcase, kind,
                                     type = res_type,

testsuite/driver/perf_notes.py

@@ -653,7 +653,11 @@ def check_stats_change(actual: PerfStat,
          error = str(change) + ' from ' + baseline.perfStat.test_env + \
                  ' baseline @ %s' % baseline.commit
          print(actual.metric, error + ':')
 -        result = failBecause('stat ' + error, tag='stat')
 +        dev = 100.0 if expected_val == 0 else round(((float(actual.value) * 100) / int(expected_val)) - 100, 1)
 +        change_line = (f'{actual.metric} {change.value} from {baseline.perfStat.test_env} '
 +                       f'baseline @ {baseline.commit[:7]}: {expected_val} -> {actual.value} '
 +                       f'({dev:+g}%, allowed {acceptance_window.describe()})')
 +        result = failBecause('stat ' + change_line, tag='stat')
      if not change_allowed or force_print:
          length = max(len(str(x)) for x in [expected_val, lowerBound, upperBound, actual.value])

testsuite/driver/testglobals.py

@@ -291,20 +291,22 @@ class TestResult:
      framework_failures, framework_warnings, unexpected_passes,
      unexpected_failures, unexpected_stat_failures lists of TestRun.
      """
 -    __slots__ = 'directory', 'testname', 'reason', 'way', 'stdout', 'stderr'
 +    __slots__ = 'directory', 'testname', 'reason', 'way', 'stdout', 'stderr', 'diff'
      def __init__(self,
                   directory: str,
                   testname: TestName,
                   reason: str,
                   way: WayName,
                   stdout: Optional[str]=None,
 -                 stderr: Optional[str]=None) -> None:
 +                 stderr: Optional[str]=None,
 +                 diff: Optional[str]=None) -> None:
          self.directory = directory
          self.testname = testname
          self.reason = reason
          self.way = way
          self.stdout = stdout
          self.stderr = stderr
 +        self.diff = diff
  # A performance metric measured in this test run.
  PerfMetric = NamedTuple('PerfMetric',

testsuite/driver/testlib.py

@@ -1828,9 +1828,11 @@ async def do_test(name: TestName,
          if result.passed:
              t.fragile_passes.append(TestResult(directory, name, 'fragile', way))
          else:
 -            t.fragile_failures.append(TestResult(directory, name, 'fragile', way,
 +            reason = '%s (fragile)' % result.reason if result.reason else 'fragile'
 +            t.fragile_failures.append(TestResult(directory, name, reason, way,
                                                   stdout=result.stdout,
 -                                                 stderr=result.stderr))
 +                                                 stderr=result.stderr,
 +                                                 diff=result.diff))
      elif result.passed:
          if _expect_pass(way):
              t.expected_passes.append(TestResult(directory, name, "", way))
@@ -1849,7 +1851,8 @@ async def do_test(name: TestName,
                  if_verbose(1, '*** unexpected failure for %s' % full_name)
                  tr = TestResult(directory, name, reason, way,
                                  stdout=result.stdout,
 -                                stderr=result.stderr)
 +                                stderr=result.stderr,
 +                                diff=result.diff)
                  t.unexpected_failures.append(tr)
          else:
              t.n_expected_failures += 1
@@ -2021,19 +2024,16 @@ async def do_compile(name: TestName,
      expected_stderr_file = find_expected_file(name, 'stderr', way)
      actual_stderr_file = add_suffix(name, 'comp.stderr')
 -    diff_file_name = in_testdir(add_suffix(name, 'comp.diff'))
+-
 -    if compare_stderr and not await compare_outputs(way, 'stderr',
 +    if compare_stderr:
 +        stderr_match = await compare_outputs(way, 'stderr',
                             join_normalisers(getTestOpts().extra_errmsg_normaliser,
                                              normalise_errmsg),
                             expected_stderr_file, actual_stderr_file,
 -                           diff_file=diff_file_name,
                             whitespace_normaliser=getattr(getTestOpts(),
                                                           "whitespace_normaliser",
 -                                                         normalise_whitespace)):
 -        stderr = diff_file_name.read_text()
 -        diff_file_name.unlink()
 -        return failBecause('stderr mismatch', stderr=stderr)
 +                                                         normalise_whitespace))
 +        if not stderr_match:
 +            return failBecause('stderr mismatch', diff=stderr_match.diff)
      opts = getTestOpts()
      if isGenericStatsTest():
@@ -2063,10 +2063,11 @@ async def compile_cmp_asm(name: TestName,
      expected_asm_file = find_expected_file(name, 'asm', way)
      actual_asm_file = add_suffix(name, 's')
 -    if not await compare_outputs(way, 'asm',
 +    asm_match = await compare_outputs(way, 'asm',
                             join_normalisers(normalise_errmsg, normalise_asm),
 -                           expected_asm_file, actual_asm_file):
 -        return failBecause('asm mismatch')
 +                           expected_asm_file, actual_asm_file)
 +    if not asm_match:
 +        return failBecause('asm mismatch', diff=asm_match.diff)
      # no problems found, this test passed
      return passed()
@@ -2147,19 +2148,15 @@ async def compile_and_run__(name: TestName,
          if compile_stderr:
              expected_stderr_file = find_expected_file(name, 'ghc.stderr', way)
              actual_stderr_file = add_suffix(name, 'comp.stderr')
 -            diff_file_name = in_testdir(add_suffix(name, 'comp.diff'))
+-
 -            if not await compare_outputs(way, 'stderr',
 +            ghc_stderr_match = await compare_outputs(way, 'stderr',
                             join_normalisers(getTestOpts().extra_errmsg_normaliser,
                                              normalise_errmsg),
                             expected_stderr_file, actual_stderr_file,
 -                           diff_file=diff_file_name,
                             whitespace_normaliser=getattr(getTestOpts(),
                                                           "whitespace_normaliser",
 -                                                         normalise_whitespace)):
 -             stderr = diff_file_name.read_text()
 -             diff_file_name.unlink()
 -             return failBecause('ghc.stderr mismatch', stderr=stderr)
 +                                                         normalise_whitespace))
 +            if not ghc_stderr_match:
 +                return failBecause('ghc.stderr mismatch', diff=ghc_stderr_match.diff)
          opts = getTestOpts()
          extension = exe_extension() if not opts.ignore_extension else ""
@@ -2448,14 +2445,18 @@ async def simple_run(name: TestName, way: WayName, prog: str, extra_run_opts: st
          message = format_bad_exit_code_message(exit_code)
          return failBecause(message)
 -    if not (opts.ignore_stderr or await stderr_ok(name, way) or opts.combined_output):
 +    stderr_match = CompareOutput(True) if (opts.ignore_stderr or opts.combined_output) else await stderr_ok(name, way)
 +    if not stderr_match:
          return failBecause('bad stderr',
                             stderr=read_stderr(name),
 -                           stdout=read_stdout(name))
 -    if not (opts.ignore_stdout or await stdout_ok(name, way)):
 +                           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=read_stdout(name),
 +                           diff=stdout_match.diff)
      check_hp = '-hT' in my_rts_flags and opts.check_hp
      check_prof = '-p' in my_rts_flags
@@ -2463,8 +2464,10 @@ async def simple_run(name: TestName, way: WayName, prog: str, extra_run_opts: st
      # exit_code > 127 probably indicates a crash, so don't try to run hp2ps.
      if check_hp and (exit_code <= 127 or exit_code == 251) and not await check_hp_ok(name):
          return failBecause('bad heap profile')
 -    if check_prof and not await check_prof_ok(name, way):
 -        return failBecause('bad profile')
 +    if check_prof:
 +        prof_match = await check_prof_ok(name, way)
 +        if not prof_match:
 +            return failBecause('bad profile', diff=prof_match.diff)
      # Check the results of stats tests
      if isGenericStatsTest():
@@ -2558,20 +2561,23 @@ async def interpreter_run(name: TestName,
      # ToDo: if the sub-shell was killed by ^C, then exit
 -    if not (opts.ignore_stderr or await stderr_ok(name, way)):
 +    stderr_match = CompareOutput(True) if opts.ignore_stderr else await stderr_ok(name, way)
 +    if not stderr_match:
          if _expect_pass(way):
              dump_stderr_for('comp', name)
          return failBecause('bad stderr',
                             stderr=read_stderr(name),
 -                           stdout=read_stdout(name))
 -    elif not (opts.ignore_stdout or await stdout_ok(name, way)):
 +                           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:
          if _expect_pass(way):
              dump_stderr_for('comp', name)
          return failBecause('bad stdout',
                             stderr=read_stderr(name),
 -                           stdout=read_stdout(name))
 -    else:
 -        return passed()
 +                           stdout=read_stdout(name),
 +                           diff=stdout_match.diff)
 +    return passed()
  def split_file(in_fn: Path, delimiter: str, out1_fn: Path, out2_fn: Path):
      # See Note [Universal newlines].
@@ -2602,7 +2608,15 @@ def get_compiler_flags() -> List[str]:
      return flags
 -async def stdout_ok(name: TestName, way: WayName) -> bool:
 +class CompareOutput:
 +    __slots__ = 'ok', 'diff'
 +    def __init__(self, ok: bool, diff: Optional[str]=None) -> None:
 +        self.ok = ok
 +        self.diff = diff
 +    def __bool__(self) -> bool:
 +        return self.ok
++
 +async def stdout_ok(name: TestName, way: WayName) -> CompareOutput:
     actual_stdout_file = add_suffix(name, 'run.stdout')
     expected_stdout_file = find_expected_file(name, 'stdout', way)
@@ -2611,7 +2625,7 @@ async def stdout_ok(name: TestName, way: WayName) -> bool:
     check_stdout = getTestOpts().check_stdout
     if check_stdout is not None:
        actual_stdout_path = in_testdir(actual_stdout_file)
 -      return check_stdout(actual_stdout_path, extra_norm)
 +      return CompareOutput(check_stdout(actual_stdout_path, extra_norm))
     return await compare_outputs(way, 'stdout', extra_norm,
                            expected_stdout_file, actual_stdout_file)
@@ -2623,13 +2637,21 @@ def read_stdout( name: TestName ) -> str:
      else:
          return ''
 +def read_diff( diff_file: Path ) -> Optional[str]:
 +    if diff_file.exists():
 +        diff = diff_file.read_text()
 +        diff_file.unlink()
 +        return diff or None
 +    else:
 +        return None
++
  def dump_stdout( name: TestName ) -> None:
      s = read_stdout(name).strip()
      if s:
          print("Stdout (", name, "):")
          safe_print(s)
 -async def stderr_ok(name: TestName, way: WayName) -> bool:
 +async def stderr_ok(name: TestName, way: WayName) -> CompareOutput:
     actual_stderr_file = add_suffix(name, 'run.stderr')
     expected_stderr_file = find_expected_file(name, 'stderr', way)
@@ -2735,48 +2757,48 @@ async def check_hp_ok(name: TestName) -> bool:
          print("hp2ps error when processing heap profile for " + name)
          return False
 -async def check_prof_ok(name: TestName, way: WayName) -> bool:
 +async def check_prof_ok(name: TestName, way: WayName) -> CompareOutput:
      expected_prof_file = find_expected_file(name, 'prof.sample', way)
      expected_prof_path = in_testdir(expected_prof_file)
      # Check actual prof file only if we have an expected prof file to
      # compare it with.
      if not expected_prof_path.exists():
 -        return True
 +        return CompareOutput(True)
      actual_prof_file = add_suffix(name, 'prof')
      actual_prof_path = in_testdir(actual_prof_file)
      if not actual_prof_path.exists():
          print("%s does not exist" % actual_prof_path)
 -        return(False)
 +        return CompareOutput(False)
      if actual_prof_path.stat().st_size == 0:
          print("%s is empty" % actual_prof_path)
 -        return(False)
 +        return CompareOutput(False)
      return await compare_outputs(way, 'prof', normalise_prof,
                              expected_prof_file, actual_prof_file,
                              whitespace_normaliser=normalise_whitespace)
  # Compare expected output to actual output, and optionally accept the
 -# new output. Returns true if output matched or was accepted, false
 -# otherwise. See Note [Output comparison] for the meaning of the
 -# normaliser and whitespace_normaliser parameters.
 +# new output. Returns a truthy CompareOutput if output matched or was
 +# accepted, a falsy one (with the diff) otherwise. See Note [Output
 +# comparison] for the meaning of the normaliser and whitespace_normaliser
 +# parameters.
  async def compare_outputs(
          way: WayName,
          kind: str,
          normaliser: OutputNormalizer,
          expected_file: Path,
          actual_file: Path,
 -        diff_file: Optional[Path]=None,
 -        whitespace_normaliser: OutputNormalizer=lambda x:x) -> bool:
 +        whitespace_normaliser: OutputNormalizer=lambda x:x) -> CompareOutput:
      # Respect ignore_stdout and ignore_stderr options
      if kind == 'stderr' and getTestOpts().ignore_stderr:
 -        return True
 +        return CompareOutput(True)
      if kind == 'stdout' and getTestOpts().ignore_stdout:
 -        return True
 +        return CompareOutput(True)
      expected_path = in_srcdir(expected_file)
      actual_path = in_testdir(actual_file)
@@ -2796,7 +2818,7 @@ async def compare_outputs(
      # See Note [Output comparison].
      if whitespace_normaliser(expected_str) == whitespace_normaliser(actual_str):
 -        return True
 +        return CompareOutput(True)
      else:
          if config.verbose >= 1 and _expect_pass(way):
              print('Actual ' + kind + ' output differs from expected:')
@@ -2808,6 +2830,7 @@ async def compare_outputs(
          actual_normalised_path = add_suffix(actual_path, 'normalised')
          write_file(actual_normalised_path, actual_str)
 +        diff_file = add_suffix(actual_path, 'diff')
          if config.verbose >= 1 and _expect_pass(way):
              # See Note [Output comparison].
              r = await runCmd('diff -uw "{0}" "{1}"'.format(null2unix_null(expected_normalised_path),
@@ -2822,13 +2845,14 @@ async def compare_outputs(
                                                             actual_normalised_path),
                             stdout=diff_file,
                             print_output=True)
 -        elif diff_file: diff_file.open('ab').close() # Make sure the file exists still as
 -                                                     # we will try to read it later
 +        else:
 +            diff_file.open('ab').close() # Make sure the file exists still as
 +                                         # we will try to read it later
          if config.accept and (getTestOpts().expect == 'fail' or
                                way in getTestOpts().expect_fail_for):
              if_verbose(1, 'Test is expected to fail. Not accepting new output.')
 -            return False
 +            return CompareOutput(False, read_diff(diff_file))
          elif config.accept and actual_raw:
              if config.accept_platform:
                  if_verbose(1, 'Accepting new output for platform "'
@@ -2842,11 +2866,11 @@ async def compare_outputs(
                  if_verbose(1, 'Accepting new output.')
              write_file(expected_path, actual_raw)
 -            return True
 +            return CompareOutput(True)
          elif config.accept:
              if_verbose(1, 'No output. Deleting "{0}".'.format(expected_path))
              expected_path.unlink()
 -            return True
 +            return CompareOutput(True)
          else:
              if config.unexpected_output_dir is not None:
                  ghc_root = expected_path.relative_to(config.top.parent)
@@ -2854,7 +2878,7 @@ async def compare_outputs(
                  out.parent.mkdir(exist_ok=True, parents=True)
                  write_file(out, actual_raw)
 -            return False
 +            return CompareOutput(False, read_diff(diff_file))
  # Checks that each line from pattern_file is present in actual_file as
  # a substring or regex pattern depending on is_substring.

testsuite/driver/testutil.py

@@ -14,6 +14,7 @@ PassFail = NamedTuple('PassFail',
                         ('tag', Optional[str]),
                         ('stderr', Optional[str]),
                         ('stdout', Optional[str]),
 +                       ('diff', Optional[str]),
                         ('hc_opts', Optional[str]),
                         ])
@@ -26,15 +27,17 @@ def passed(hc_opts=None) -> PassFail:
                      tag=None,
                      stderr=None,
                      stdout=None,
 +                    diff=None,
                      hc_opts=hc_opts)
  def failBecause(reason: str,
                  tag: Optional[str]=None,
                  stderr: Optional[str]=None,
 -                stdout: Optional[str]=None
 +                stdout: Optional[str]=None,
 +                diff: Optional[str]=None
                  ) -> PassFail:
      return PassFail(passed=False, reason=reason, tag=tag,
 -                    stderr=stderr, stdout=stdout, hc_opts=None)
 +                    stderr=stderr, stdout=stdout, diff=diff, hc_opts=None)
  def strip_quotes(s: str) -> str:
      # Don't wrap commands to subprocess.call/Popen in quotes.