Merge pull request #7672 from EricCousineau-TRI/hotfix/7527_6

call_python: Ensure that the tests accommodate SIGPIPE failures on C++ side.
RobotLocomotion · Jan 5, 2018 · 1530b72 · 1530b72
2 parents 56fc40b + f7e3131
commit 1530b72
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 15 deletions.
diff --git a/common/proto/BUILD.bazel b/common/proto/BUILD.bazel
@@ -110,17 +110,6 @@ sh_test(
     # Certain versions of Mac don't like this script using `ps`. For this
     # reason, make the test run locally.
     local = 1,
-    # TODO(eric.cousineau): The `call_python_test` binary is flaky with asan /
-    # lsan; however, when the binary fails, it simply aborts without
-    # diagnostic information. When this is resolved, these tags should be
-    # removed.
-    # TODO(jamiesnape): Tagged "manual" because fails with latest Matplotlib
-    # (~2.1.1) on macOS (and possibly other platforms).
-    tags = [
-        "manual",
-        "no_asan",
-        "no_lsan",
-    ],
 )
 
 drake_cc_googletest(

diff --git a/common/proto/test/call_python_full_test.sh b/common/proto/test/call_python_full_test.sh
@@ -4,6 +4,9 @@ set -e -u
 # @file
 # @brief Tests the `call_python_client` CLI and `call_python` test together.
 
+# TODO(eric.cousineau): Rewrite this in Python for an easier-to-understand
+# testing API (#7703).
+
 no_plotting=
 # By default, set backend so that the test does not open windows.
 export MPLBACKEND="ps"
@@ -93,6 +96,29 @@ py-check() {
     fi
 }
 
+SIGPIPE_STATUS=141
+
+cc-check() {
+    if [[ ${py_fail} -eq 0 ]]; then
+        "$@" || { echo "C++ binary failed"; exit 1; }
+    else
+        # If the C++ binary has not finished by the time the Python client
+        # exits due to failure, then the C++ binary will fail with SIGPIPE.
+        set +e
+        "$@"
+        status=$?
+        set -e
+        if [[ ${status} -eq 0 ]]; then
+            :
+        elif [[ ${status} -eq ${SIGPIPE_STATUS} ]]; then
+            echo "C++ binary failed with SIGPIPE; expected behavior, continuing."
+        else
+            echo "C++ binary failed"
+            exit ${status}
+        fi
+    fi
+}
+
 do-setup() {
     py_fail=${1}
     py_stop_on_error=${2}
@@ -121,7 +147,7 @@ no_threading-no_loop() {
     ${py_client_cli} --no_threading --no_loop ${py_flags} &
     pid=$!
     # Execute C++.
-    ${cc_bin} ${cc_bin_flags} ${cc_flags}
+    cc-check ${cc_bin} ${cc_bin_flags} ${cc_flags}
     # When this is done, Python client should exit.
     py-check wait ${pid}
 }
@@ -130,19 +156,19 @@ sub-tests no_threading-no_loop
 threading-no_loop() {
     ${py_client_cli} --no_loop ${py_flags} &
     pid=$!
-    ${cc_bin} ${cc_bin_flags} ${cc_flags}
+    cc-check ${cc_bin} ${cc_bin_flags} ${cc_flags}
     py-check wait ${pid}
 }
 sub-tests threading-no_loop
 
 threading-loop() {
     ${py_client_cli} ${py_flags} &
     pid=$!
-    ${cc_bin} ${cc_bin_flags} ${cc_flags}
+    cc-check ${cc_bin} ${cc_bin_flags} ${cc_flags}
     if [[ ${py_stop_on_error} -ne 1 ]]; then
         # If the client will not halt execution based on an error, execute C++
         # client once more.
-        ${cc_bin} ${cc_bin_flags} ${cc_flags}
+        cc-check ${cc_bin} ${cc_bin_flags} ${cc_flags}
         # Ensure that we wait until the client is fully done with both
         # executions.
         done_count=0

diff --git a/common/proto/test/call_python_test.cc b/common/proto/test/call_python_test.cc
@@ -1,7 +1,9 @@
 #include "drake/common/proto/call_python.h"
 
+#include <chrono>
 #include <cmath>
 #include <string>
+#include <thread>
 
 #include <gflags/gflags.h>
 #include <gtest/gtest.h>
@@ -15,6 +17,9 @@ DEFINE_string(done_file, "/tmp/python_rpc_done",
               "Signifies last Python command has been executed.");
 // Ensure that we test error behavior.
 DEFINE_bool(with_error, false, "Inject an error towards the end.");
+DEFINE_bool(sleep_at_end, false,
+            "Sleep at end to check behavior of C++ if the Python client "
+            "fails.");
 
 // TODO(eric.cousineau): Instrument client to verify output (and make this a
 // unittest).
@@ -158,6 +163,12 @@ GTEST_TEST(TestCallPython, Plot3d) {
 }
 
 GTEST_TEST(TestCallPython, Finish) {
+  if (FLAGS_sleep_at_end) {
+    // If the Python client closes before this program closes the FIFO pipe,
+    // then this executable should receive a SIGPIPE signal (and fail).
+    // Sleeping here simulates this condition for manual testing.
+    std::this_thread::sleep_for(std::chrono::seconds(1));
+  }
   // Signal finishing to client.
   CallPython("execution_check.finish");
   // Signal finishing to `call_python_full_test.sh`.