Merge lp:~albaguirre/mir/fix-1499229 into lp:mir

[Alberto Aguirre]

Workaround the fact that a child exit code may be modified by valgrind.

[Brandon Schaefer (brandontschaefer)]

LGTM

review: Approve

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3467
https://mir-jenkins.ubuntu.com/job/mir-ci/877/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/898
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/935
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/926
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/926
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/908
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/908/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/908
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/908/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/908
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/908/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/908
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/908/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/908
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/908/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/877/rebuild

review: Approve (continuous-integration)

=== modified file 'include/test/mir_test_framework/process.h'
--- include/test/mir_test_framework/process.h	2016-01-29 08:18:22 +0000
+++ include/test/mir_test_framework/process.h	2016-04-20 20:58:42 +0000
@@ -54,6 +54,9 @@
     // Was the process terminated by a signal?
     bool signalled() const;
 
+    // Was the process terminated normally?
+    bool exited_normally() const;
+
     TerminationReason reason;
     int exit_code;
     int signal;

=== modified file 'tests/mir_test_framework/process.cpp'
--- tests/mir_test_framework/process.cpp	2015-02-22 07:46:25 +0000
+++ tests/mir_test_framework/process.cpp	2016-04-20 20:58:42 +0000
@@ -73,6 +73,11 @@
     return reason == TerminationReason::child_terminated_by_signal;
 }
 
+bool mtf::Result::exited_normally() const
+{
+    return reason == TerminationReason::child_terminated_normally;
+}
+
 mtf::Process::Process(pid_t pid)
     : pid(pid)
     , terminated(false)

=== modified file 'tests/unit-tests/dispatch/test_threaded_dispatcher.cpp'
--- tests/unit-tests/dispatch/test_threaded_dispatcher.cpp	2016-03-23 06:39:56 +0000
+++ tests/unit-tests/dispatch/test_threaded_dispatcher.cpp	2016-04-20 20:58:42 +0000
@@ -326,6 +326,7 @@
 {
     using namespace std::chrono_literals;
     mt::CrossProcessAction stop_and_restart_process;
+    mt::CrossProcessSync exit_success_sync;
 
     auto child = mir_test_framework::fork_and_run_in_a_different_process(
         [&]
@@ -360,11 +361,14 @@
                 dispatchable->trigger();
                 EXPECT_TRUE(dispatched->wait_for(5s));
 
+                if (!HasFailure())
+                    exit_success_sync.signal_ready();
+
                 // Because we terminate this process with an explicit call to
                 // exit(), objects on the stack are not destroyed.
-                // We need to destroy the stop_and_restart_process object 
-                // manually to avoid fd leaks.
+                // We need to destroy these objects manually to avoid fd leaks.
                 stop_and_restart_process.~CrossProcessAction();
+                exit_success_sync.~CrossProcessSync();
             }
 
             exit(HasFailure() ? EXIT_FAILURE : EXIT_SUCCESS);
@@ -383,7 +387,17 @@
         });
 
     auto const result = child->wait_for_termination(30s);
-    EXPECT_TRUE(result.succeeded());
+    EXPECT_TRUE(result.exited_normally());
+
+    // The test may run under valgrind which may change the exit code of the
+    // forked child if it detects any issues. Issues detected by valgrind are
+    // outside the scope of the test - they are for the parent caller to
+    // examine and determine their validity; hence here we ignore the exit
+    // code and instead use a sync object to determine if the child ran the
+    // test successfully.
+    // The child has already exited here so there's no reason to wait.
+    auto const exit_success = exit_success_sync.wait_for_signal_ready_for(0s);
+    EXPECT_TRUE(exit_success);
 }
 
 TEST_F(ThreadedDispatcherDeathTest, destroying_dispatcher_from_a_callback_is_an_error)