From 85b4ce837e6bd213f3b6706ddbc1e6742230b51f Mon Sep 17 00:00:00 2001
From: Vladislav Zavialov <vlad.z.4096@gmail.com>
Date: Thu, 4 Apr 2024 20:51:58 +0300
Subject: [PATCH] Split stdout and stderr in viper/* tests

The `moc --viper` command outputs generated Viper code to stdout and
diagnostic messages to stderr.

* Prior to this change, test/run.sh used the `run` helper to produce
  .vpr files. The `run` helper always merges stdout and stderr, which
  results in ill-formed .vpr files if there are any diagnostic messages.

* Now test/run.sh has an additional helper `run_stderr` that keeps
  stdout and stderr separate. By using it instead of `run` we ensure
  that .vpr files are not contaminated with diagnostics.

Run `make -C test/viper` to confirm that this fixes broken Viper tests.

Note that `run_stderr` is a copy of `run` with minor tweaks, so this
solution introduces some technical debt. A more principled approach
would be to separate stdout and stderr in all commands (i.e. wholly
replace `run` with `run_stderr`), but I opted not to do that at this
time to avoid a massive overhaul of the test suite.
---
 test/run.sh                                   | 44 ++++++++++++++++++-
 test/viper/ok/private.tc.ok                   |  1 +
 test/viper/ok/private.vpr.stderr.ok           |  1 +
 test/viper/ok/unsupported.tc.ok               |  1 +
 ...orted.vpr.ok => unsupported.vpr.stderr.ok} |  1 +
 5 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 test/viper/ok/private.tc.ok
 create mode 100644 test/viper/ok/private.vpr.stderr.ok
 create mode 100644 test/viper/ok/unsupported.tc.ok
 rename test/viper/ok/{unsupported.vpr.ok => unsupported.vpr.stderr.ok} (50%)

diff --git a/test/run.sh b/test/run.sh
index 573e435a249..142d3a912df 100755
--- a/test/run.sh
+++ b/test/run.sh
@@ -140,6 +140,48 @@ function run () {
   return $ret
 }
 
+# 'run_stderr' is a variant of 'run' that redirects stdout and stderr into
+# separate files instead of merging them. It was created by copy&paste and
+# applying minor tweaks. If the 'run' function is changed, it is quite likely
+# that 'run_stderr' requires similar changes.
+#
+# Separation of stdout/stderr is necessary when e.g. producing .vpr files so
+# that diagnostic messages don't get interspersed with the generated Viper code.
+function run_stderr () {
+  # first argument: extension of the output file
+  # remaining argument: command line
+  # uses from scope: $out, $file, $base, $diff_files
+
+  local ext="$1"
+  shift
+
+  if grep -q "^//SKIP $ext$" $(basename $file); then return 1; fi
+
+  if test -e $out/$base.$ext
+  then
+    echo "Output $ext already exists."
+    exit 1
+  fi
+
+  $ECHO -n " [$ext]"
+  "$@" > $out/$base.$ext 2>$out/$base.$ext.stderr
+  local ret=$?
+
+  if [ $ret != 0 ]
+  then echo "Return code $ret" >> $out/$base.$ext.ret
+  else rm -f $out/$base.$ext.ret
+  fi
+  diff_files="$diff_files $base.$ext.ret"
+
+  normalize $out/$base.$ext.stderr
+  diff_files="$diff_files $base.$ext.stderr"
+
+  normalize $out/$base.$ext
+  diff_files="$diff_files $base.$ext"
+
+  return $ret
+}
+
 function run_if () {
   # first argument: a file extension
   # remaining argument: passed to run
@@ -294,7 +336,7 @@ do
         fi
       elif [ $VIPER = 'yes' ]
       then
-        run vpr $moc_with_flags --viper $base.mo -o $out/$base.vpr
+        run_stderr vpr $moc_with_flags --viper $base.mo -o $out/$base.vpr
         vpr_succeeded=$?
 
         normalize $out/$base.vpr
diff --git a/test/viper/ok/private.tc.ok b/test/viper/ok/private.tc.ok
new file mode 100644
index 00000000000..8ff67ecd9de
--- /dev/null
+++ b/test/viper/ok/private.tc.ok
@@ -0,0 +1 @@
+private.mo:11.16-11.22: warning [M0194], unused identifier reward (delete or rename to wildcard `_` or `_reward`)
diff --git a/test/viper/ok/private.vpr.stderr.ok b/test/viper/ok/private.vpr.stderr.ok
new file mode 100644
index 00000000000..8ff67ecd9de
--- /dev/null
+++ b/test/viper/ok/private.vpr.stderr.ok
@@ -0,0 +1 @@
+private.mo:11.16-11.22: warning [M0194], unused identifier reward (delete or rename to wildcard `_` or `_reward`)
diff --git a/test/viper/ok/unsupported.tc.ok b/test/viper/ok/unsupported.tc.ok
new file mode 100644
index 00000000000..a9b46438288
--- /dev/null
+++ b/test/viper/ok/unsupported.tc.ok
@@ -0,0 +1 @@
+unsupported.mo:5.7-5.8: warning [M0194], unused identifier x (delete or rename to wildcard `_` or `_x`)
diff --git a/test/viper/ok/unsupported.vpr.ok b/test/viper/ok/unsupported.vpr.stderr.ok
similarity index 50%
rename from test/viper/ok/unsupported.vpr.ok
rename to test/viper/ok/unsupported.vpr.stderr.ok
index 30d4be4428b..b01792b2298 100644
--- a/test/viper/ok/unsupported.vpr.ok
+++ b/test/viper/ok/unsupported.vpr.stderr.ok
@@ -1,3 +1,4 @@
+unsupported.mo:5.7-5.8: warning [M0194], unused identifier x (delete or rename to wildcard `_` or `_x`)
 unsupported.mo:5.3-5.13: viper error [0], translation to viper failed:
 (LetD (VarP x) (LitE (TextLit )))