Fix up all the tests to work with "blaze run --direct_run".

In particular, update run_test so that the little test binary used to check ANSI escape code stripping explicitly marks the beginning of its output and delete a bunch of tests run RunCommandTest that verified the output of the binary. They are not relevant anymore now that the output is not piped through the server.

Also drive-by fix a few linter issues in RunCommand.

RELNOTES: None.
PiperOrigin-RevId: 193655720

2 files changed, 17 insertions, 39 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
index 9e2ea3fd42..05ab21232a 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -109,6 +109,7 @@ import javax.annotation.Nullable;
          completion = "label-bin",
          binaryStdErr = true)
 public class RunCommand implements BlazeCommand  {
+  /** Options for the "run" command. */
   public static class RunOptions extends OptionsBase {
     @Option(
         name = "as_test",
@@ -612,11 +613,10 @@ public class RunCommand implements BlazeCommand  {
   }
 
   private boolean writeScript(CommandEnvironment env, PathFragment scriptPathFrag, String cmd) {
-    final String SH_SHEBANG = "#!/bin/sh";
     Path scriptPath = env.getWorkingDirectory().getRelative(scriptPathFrag);
     try {
       FileSystemUtils.writeContent(scriptPath, StandardCharsets.ISO_8859_1,
-          SH_SHEBANG + "\n" + cmd + " \"$@\"");
+          "#!/bin/sh\n" + cmd + " \"$@\"");
       scriptPath.setExecutable(true);
     } catch (IOException e) {
       env.getReporter().handle(Event.error("Error writing run script:" + e.getMessage()));
diff --git a/src/test/shell/integration/run_test.sh b/src/test/shell/integration/run_test.sh
index fb0391db89..f102359c7a 100755
--- a/src/test/shell/integration/run_test.sh
+++ b/src/test/shell/integration/run_test.sh
@@ -175,37 +175,6 @@ function test_consistent_command_line_encoding {
     || fail "${PRODUCT_NAME} test failed (--batch)"
 }
 
-function test_interrupt_kills_child() {
-  mkdir -p foo || fail "mkdir foo failed"
-  pipe_file="${TEST_TMPDIR}/sleep-minute-pipe"
-  rm -f "$pipe_file"
-  mkfifo "$pipe_file" || fail "make pipe failed"
-  echo 'sh_binary(name = "sleep-minute", srcs = ["sleep-minute.sh"])' > foo/BUILD
-  echo -e "#!/bin/sh\n"'echo $$ >'"${pipe_file}\n"'sleep 60' > foo/sleep-minute.sh
-  chmod +x foo/sleep-minute.sh
-  # Note that if bazel info is not executed before the actual bazel run, this script would have to
-  # be run in "monitor mode" (with the command set -m) for bazel or the server to receive SIGINT.
-  local serverpid=$(bazel info server_pid)
-  if [ -z $serverpid ]; then
-    fail "Couldn't get ${PRODUCT_NAME} server PID"
-  fi
-  (bazel run //foo:sleep-minute || true) &
-  local sleeppid
-  read sleeppid <"$pipe_file"
-  if [ -z $sleeppid ]; then
-    fail "${PRODUCT_NAME} run did not invoke shell script"
-  fi
-  kill -SIGINT $serverpid
-
-  # This test is a bit flaky, so we wait a bit more when the process still runs
-  # after 0.25s.
-  for i in 0.25 0.5 1 2; do
-    sleep $i
-    kill -0 $sleeppid 2> /dev/null || return 0
-  done
-  fail "Shell script still running after SIGINT sent to server"
-}
-
 # Tests bazel run with --color=no on a failed build does not produce color.
 function test_no_color_on_failed_run() {
   mkdir -p x || fail "mkdir failed"
@@ -232,7 +201,9 @@ function test_no_ansi_stripping_in_stdout_or_stderr() {
   echo "cc_binary(name = 'x', srcs = ['x.cc'])" > x/BUILD
   cat > x/x.cc <<EOF
 #include <unistd.h>
+#include <stdio.h>
 int main(int, char**) {
+  fprintf(stderr, "\nRUN START\n");
   const char out[] = {'<', 0x1B, '[', 'a', ',', 0x1B, '[', '1', '>', 0x0A};
   const char err[] = {'<', 0x1B, '[', 'b', ',', 0x1B, '[', '2', '>', 0x0A};
   write(1, out, 10);
@@ -248,6 +219,8 @@ EOF
   err1color=$(mktemp x/XXXXXX)
   err1nocolor=$(mktemp x/XXXXXX)
   err2=$(mktemp x/XXXXXX)
+  err2raw=$(mktemp x/XXXXXX)
+
 
   # TODO(katre): Figure out why progress rate limiting is required for this on darwin.
   add_to_bazelrc common --show_progress_rate_limit=0.03
@@ -256,21 +229,26 @@ EOF
   echo >> $err1raw_color
   echo >> $err1raw_nocolor
 
-  ${PRODUCT_NAME}-bin/x/x >$out2 2>$err2
-  echo >> $err2
+  ${PRODUCT_NAME}-bin/x/x >$out2 2>$err2raw
+  echo >> $err2raw
+
+  # Remove the first newline that is printed so that the output of Bazel can be
+  # separated from the output of the test binary
+  tail -n +2 $err2raw > $err2
 
   # Extract the binary's stderr from the raw stderr, which also contains bazel's
   # stderr; if present, remove a trailing ^[[0m (reset terminal to defaults).
   bazel_stderr_line_count_color=$(cat $err1raw_color \
-    | grep -n "Running command line: .*/x/x" \
+    | grep -n 'RUN START' \
     | awk -F ':' '{print $1}')
-  start=$(($bazel_stderr_line_count_color+1))
+  start="$bazel_stderr_line_count_color"
   tail -n +$start $err1raw_color | sed -e 's/.\[0m$//' >$err1color
 
+  cat $err1raw_nocolor
   bazel_stderr_line_count_nocolor=$(cat $err1raw_nocolor \
-    | grep -n "Running command line: .*/x/x" \
+    | grep -n 'RUN START' \
     | awk -F ':' '{print $1}')
-  start=$(($bazel_stderr_line_count_nocolor+1))
+  start="$bazel_stderr_line_count_nocolor"
   tail -n +$start $err1raw_nocolor >$err1nocolor
 
   diff $out1color $out2 >&$TEST_log || fail "stdout with --color=yes differs"