Un-disable WinMultiprocess-based tests in Chromium

As of
00a0654929,
crashpad_util_test is able to run in Chromium. It uses Chromium’s own
base::TestLauncher rather than gtest’s RUN_ALL_TESTS() for proper
integration with Swarming.

Launching WinMultiprocess test children out of the same test executable
via WinChildProcess is not compatible with Chromium’s parallel, shardy,
Swarmy test launcher. When running these children, the standard gtest
RUN_ALL_TESTS() launcher will now be used, even in Chromium.

Two tests disabled in Chromium are now enabled:
ExceptionHandlerServerTest.MultipleConnections and
ScopedProcessSuspend.ScopedProcessSuspend.

As part of this work, I discovered that disabled tests chosen to run via
--gtest_also_run_disabled_tests did not actually work for
WinMultiprocess-based tests, because gtest’s test launcher would refuse
to run the child side of the test, believing it was disabled. This is
fixed by always supplying --gtest_also_run_disabled_tests to
WinChildProcess children, on the basis that if the parent is managing to
run and it’s disabled, disabled tests must actually be enabled.

Bug: crashpad:205
Change-Id: Ied22f16b9329ee13b6b07fd29de704f6fe2a058e
Reviewed-on: https://chromium-review.googlesource.com/742462
Reviewed-by: Scott Graham <scottmg@chromium.org>

test/gmock_main.cc

@@ -1,47 +0,0 @@
-// Copyright 2017 The Crashpad Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
-#include "test/gtest_disabled.h"
-#include "test/main_arguments.h"
-
-#if defined(CRASHPAD_IN_CHROMIUM)
-#include "base/bind.h"
-#include "base/test/launcher/unit_test_launcher.h"
-#include "base/test/test_suite.h"
-#endif  // CRASHPAD_IN_CHROMIUM
-
-int main(int argc, char* argv[]) {
-  crashpad::test::InitializeMainArguments(argc, argv);
-  testing::InitGoogleMock(&argc, argv);
-  testing::AddGlobalTestEnvironment(
-      crashpad::test::DisabledTestGtestEnvironment::Get());
-
-#if defined(CRASHPAD_IN_CHROMIUM)
-
-  // This supports --test-launcher-summary-output, which writes a JSON file
-  // containing test details needed by Swarming.
-  base::TestSuite test_suite(argc, argv);
-  return base::LaunchUnitTests(
-      argc,
-      argv,
-      base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
-
-#else  // CRASHPAD_IN_CHROMIUM
-
-  return RUN_ALL_TESTS();
-
-#endif  // CRASHPAD_IN_CHROMIUM
-}

test/gtest_main.cc

@@ -12,10 +12,19 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "build/build_config.h"
 #include "gtest/gtest.h"
 #include "test/gtest_disabled.h"
 #include "test/main_arguments.h"
 
+#if defined(CRASHPAD_TEST_LAUNCHER_GMOCK)
+#include "gmock/gmock.h"
+#endif  // CRASHPAD_TEST_LAUNCHER_GMOCK
+
+#if defined(OS_WIN)
+#include "test/win/win_child_process.h"
+#endif  // OS_WIN
+
 #if defined(CRASHPAD_IN_CHROMIUM)
 #include "base/bind.h"
 #include "base/test/launcher/unit_test_launcher.h"
@@ -24,23 +33,41 @@
 
 int main(int argc, char* argv[]) {
   crashpad::test::InitializeMainArguments(argc, argv);
+
+#if defined(CRASHPAD_TEST_LAUNCHER_GTEST)
   testing::InitGoogleTest(&argc, argv);
+#elif defined(CRASHPAD_TEST_LAUNCHER_GMOCK)
+  testing::InitGoogleMock(&argc, argv);
+#else  // CRASHPAD_TEST_LAUNCHER_GTEST
+#error #define CRASHPAD_TEST_LAUNCHER_GTEST or CRASHPAD_TEST_LAUNCHER_GMOCK
+#endif  // CRASHPAD_TEST_LAUNCHER_GTEST
+
   testing::AddGlobalTestEnvironment(
       crashpad::test::DisabledTestGtestEnvironment::Get());
 
 #if defined(CRASHPAD_IN_CHROMIUM)
 
-  // This supports --test-launcher-summary-output, which writes a JSON file
-  // containing test details needed by Swarming.
-  base::TestSuite test_suite(argc, argv);
-  return base::LaunchUnitTests(
-      argc,
-      argv,
-      base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
+#if defined(OS_WIN)
+  // Chromium’s test launcher interferes with WinMultiprocess-based tests. Allow
+  // their child processes to be launched by the standard gtest-based test
+  // runner.
+  const bool use_chromium_test_launcher =
+      !crashpad::test::WinChildProcess::IsChildProcess();
+#else  // OS_WIN
+  constexpr bool use_chromium_test_launcher = true;
+#endif  // OS_WIN
 
-#else  // CRASHPAD_IN_CHROMIUM
-
-  return RUN_ALL_TESTS();
+  if (use_chromium_test_launcher) {
+    // This supports --test-launcher-summary-output, which writes a JSON file
+    // containing test details needed by Swarming.
+    base::TestSuite test_suite(argc, argv);
+    return base::LaunchUnitTests(
+        argc,
+        argv,
+        base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
+  }
 
 #endif  // CRASHPAD_IN_CHROMIUM
+
+  return RUN_ALL_TESTS();
 }

test/test.gyp

@@ -101,21 +101,6 @@
         }],
       ],
     },
-    {
-      'target_name': 'crashpad_gtest_main',
-      'type': 'static_library',
-      'dependencies': [
-        'crashpad_test',
-        '../third_party/gtest/gtest.gyp:gtest',
-        '../third_party/mini_chromium/mini_chromium.gyp:base',
-      ],
-      'include_dirs': [
-        '..',
-      ],
-      'sources': [
-        'gtest_main.cc',
-      ],
-    },
     {
       'target_name': 'crashpad_gmock_main',
       'type': 'static_library',
@@ -128,8 +113,29 @@
       'include_dirs': [
         '..',
       ],
+      'defines': [
+        'CRASHPAD_TEST_LAUNCHER_GMOCK=1',
+      ],
       'sources': [
-        'gmock_main.cc',
+        'gtest_main.cc',
+      ],
+    },
+    {
+      'target_name': 'crashpad_gtest_main',
+      'type': 'static_library',
+      'dependencies': [
+        'crashpad_test',
+        '../third_party/gtest/gtest.gyp:gtest',
+        '../third_party/mini_chromium/mini_chromium.gyp:base',
+      ],
+      'include_dirs': [
+        '..',
+      ],
+      'defines': [
+        'CRASHPAD_TEST_LAUNCHER_GTEST=1',
+      ],
+      'sources': [
+        'gtest_main.cc',
       ],
     },
   ],

test/win/win_child_process.cc

@@ -182,17 +182,20 @@ std::unique_ptr<WinChildProcess::Handles> WinChildProcess::Launch() {
   }
 
   // Build a command line for the child process that tells it only to run the
-  // current test, and to pass down the values of the pipe handles.
+  // current test, and to pass down the values of the pipe handles. Use
+  // --gtest_also_run_disabled_tests because the test may be DISABLED_, but if
+  // it managed to run in the parent, disabled tests must be running.
   const ::testing::TestInfo* const test_info =
       ::testing::UnitTest::GetInstance()->current_test_info();
   std::wstring command_line =
-      TestPaths::Executable().value() + L" " +
-      base::UTF8ToUTF16(base::StringPrintf("--gtest_filter=%s.%s %s=0x%x|0x%x",
-                                           test_info->test_case_name(),
-                                           test_info->name(),
-                                           kIsMultiprocessChild,
-                                           HandleToInt(write_for_child.get()),
-                                           HandleToInt(read_for_child.get())));
+      TestPaths::Executable().value() +
+      base::UTF8ToUTF16(base::StringPrintf(
+          " --gtest_filter=%s.%s %s=0x%x|0x%x --gtest_also_run_disabled_tests",
+          test_info->test_case_name(),
+          test_info->name(),
+          kIsMultiprocessChild,
+          HandleToInt(write_for_child.get()),
+          HandleToInt(read_for_child.get())));
 
   // Command-line buffer cannot be constant, per CreateProcess signature.
   handles_for_parent->process = LaunchCommandLine(&command_line[0]);

util/win/exception_handler_server_test.cc

@@ -180,13 +180,7 @@ class TestClient final : public WinChildProcess {
   DISALLOW_COPY_AND_ASSIGN(TestClient);
 };
 
-// https://crashpad.chromium.org/bug/205
-#if defined(CRASHPAD_IN_CHROMIUM)
-#define MAYBE_MultipleConnections DISABLED_MultipleConnections
-#else  // CRASHPAD_IN_CHROMIUM
-#define MAYBE_MultipleConnections MultipleConnections
-#endif  // CRASHPAD_IN_CHROMIUM
-TEST_F(ExceptionHandlerServerTest, MAYBE_MultipleConnections) {
+TEST_F(ExceptionHandlerServerTest, MultipleConnections) {
   WinChildProcess::EntryPoint<TestClient>();
 
   std::unique_ptr<WinChildProcess::Handles> handles_1 =

util/win/scoped_process_suspend_test.cc

@@ -89,13 +89,7 @@ class ScopedProcessSuspendTest final : public WinChildProcess {
   DISALLOW_COPY_AND_ASSIGN(ScopedProcessSuspendTest);
 };
 
-// https://crashpad.chromium.org/bug/205
-#if defined(CRASHPAD_IN_CHROMIUM)
-#define MAYBE_ScopedProcessSuspend DISABLED_ScopedProcessSuspend
-#else  // CRASHPAD_IN_CHROMIUM
-#define MAYBE_ScopedProcessSuspend ScopedProcessSuspend
-#endif  // CRASHPAD_IN_CHROMIUM
-TEST(ScopedProcessSuspend, MAYBE_ScopedProcessSuspend) {
+TEST(ScopedProcessSuspend, ScopedProcessSuspend) {
   WinChildProcess::EntryPoint<ScopedProcessSuspendTest>();
   std::unique_ptr<WinChildProcess::Handles> handles = WinChildProcess::Launch();