Testsuite: diff non-whitespace normalised output (#10152)

On a test failure, we show a diff between the expected and the actual
output. The method of how we do this has changed a couple of times:

* In 2007: 9951189ccf90b709436fa55ee49eeef031f79f4e
  "On failure, diff the normalised test outputs"

* In 2011: 3019b1e409c129ef7af63e6a7408fb36ec44444b
  "When the output files differ, present the diffs between the *actual*
  output, not the normalised output. The latter may have newlines
  removed, making the diff unreadable."

* In 2015 (now): do something in between.
  - Do apply the normalisers again, to make the diff smaller (only
    showing the actual problem).
  - But don't apply normalise_whitespace, as it indeed makes the diff
    unreadable.

Differential Revision: https://phabricator.haskell.org/D984

1 files changed, 36 insertions, 29 deletions

diff --git a/testsuite/driver/testlib.py b/testsuite/driver/testlib.py
index 462c854267..b277a35e59 100644
--- a/testsuite/driver/testlib.py
+++ b/testsuite/driver/testlib.py
@@ -1053,9 +1053,9 @@ def do_compile( name, way, should_fail, top_mod, extra_mods, extra_hc_opts, over
 
     if not compare_outputs(way, 'stderr',
                            join_normalisers(getTestOpts().extra_errmsg_normaliser,
-                                            normalise_errmsg,
-                                            normalise_whitespace),
-                           expected_stderr_file, actual_stderr_file):
+                                            normalise_errmsg),
+                           expected_stderr_file, actual_stderr_file,
+                           whitespace_normaliser=normalise_whitespace):
         return failBecause('stderr mismatch')
 
     # no problems found, this test passed
@@ -1634,52 +1634,44 @@ def check_prof_ok(name, way):
     if not os.path.exists(expected_prof_file):
         return True
     else:
-        return compare_outputs(way, 'prof',
-                               join_normalisers(normalise_whitespace,normalise_prof), \
-                               expected_prof_file, prof_file)
+        return compare_outputs(way, 'prof', normalise_prof,
+                               expected_prof_file, 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.
-def compare_outputs(way, kind, normaliser, expected_file, actual_file):
+# otherwise. See Note [Output comparison] for the meaning of the
+# normaliser and whitespace_normaliser parameters.
+def compare_outputs(way, kind, normaliser, expected_file, actual_file,
+                    whitespace_normaliser=lambda x:x):
+
     if os.path.exists(expected_file):
-        expected_raw = read_no_crs(expected_file)
-        # print "norm:", normaliser(expected_raw)
-        expected_str = normaliser(expected_raw)
-        expected_file_for_diff = expected_file
+        expected_str = normaliser(read_no_crs(expected_file))
+        expected_normalised_file = expected_file + ".normalised"
     else:
         expected_str = ''
-        expected_file_for_diff = '/dev/null'
+        expected_normalised_file = '/dev/null'
 
     actual_raw = read_no_crs(actual_file)
     actual_str = normaliser(actual_raw)
 
-    if expected_str == actual_str:
+    # See Note [Output comparison].
+    if whitespace_normaliser(expected_str) == whitespace_normaliser(actual_str):
         return 1
     else:
         if config.verbose >= 1 and _expect_pass(way):
             print('Actual ' + kind + ' output differs from expected:')
 
-        if expected_file_for_diff == '/dev/null':
-            expected_normalised_file = '/dev/null'
-        else:
-            expected_normalised_file = expected_file + ".normalised"
+        if expected_normalised_file != '/dev/null':
             write_file(expected_normalised_file, expected_str)
 
         actual_normalised_file = actual_file + ".normalised"
         write_file(actual_normalised_file, actual_str)
 
-        # Ignore whitespace when diffing. We should only get to this
-        # point if there are non-whitespace differences
-        #
-        # Note we are diffing the *actual* output, not the normalised
-        # output.  The normalised output may have whitespace squashed
-        # (including newlines) so the diff would be hard to read.
-        # This does mean that the diff might contain changes that
-        # would be normalised away.
         if config.verbose >= 1 and _expect_pass(way):
-            r = os.system( 'diff -uw ' + expected_file_for_diff + \
-                                   ' ' + actual_file )
+            # See Note [Output comparison].
+            r = os.system('diff -uw ' + expected_normalised_file +
+                                  ' ' + actual_normalised_file)
 
             # If for some reason there were no non-whitespace differences,
             # then do a full diff
@@ -1698,11 +1690,26 @@ def compare_outputs(way, kind, normaliser, expected_file, actual_file):
         else:
             return 0
 
+# Note [Output comparison]
+#
+# We do two types of output comparison:
+#
+# 1. To decide whether a test has failed. We apply a `normaliser` and an
+#    optional `whitespace_normaliser` to the expected and the actual
+#    output, before comparing the two.
+#
+# 2. To show as a diff to the user when the test indeed failed. We apply
+#    the same `normaliser` function to the outputs, to make the diff as
+#    small as possible (only showing the actual problem). But we don't
+#    apply the `whitespace_normaliser` here, because it might completely
+#    squash all whitespace, making the diff unreadable. Instead we rely
+#    on the `diff` program to ignore whitespace changes as much as
+#    possible (#10152).
 
 def normalise_whitespace( str ):
     # Merge contiguous whitespace characters into a single space.
     str = re.sub('[ \t\n]+', ' ', str)
-    return str
+    return str.strip()
 
 def normalise_errmsg( str ):
     # remove " error:" and lower-case " Warning:" to make patch for