Revert "Use GTEST_SKIP() instead of custom DISABLED_TEST()"

This reverts commit 79f4a3970a6425ef0475263974bf9a012279ba4f.

Chromium’s test launcher is not prepared to handle GTEST_SKIP().

Bug: chromium:912138
Change-Id: Iaeffaedcd92093ec61b013f2a919dc4670094581
Reviewed-on: https://chromium-review.googlesource.com/c/1464099
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>

client/crash_report_database_test.cc

@@ -20,6 +20,7 @@
 #include "test/errors.h"
 #include "test/file.h"
 #include "test/filesystem.h"
+#include "test/gtest_disabled.h"
 #include "test/scoped_temp_dir.h"
 #include "util/file/file_io.h"
 #include "util/file/filesystem.h"
@@ -672,7 +673,7 @@ TEST_F(CrashReportDatabaseTest, RequestUpload) {
 TEST_F(CrashReportDatabaseTest, Attachments) {
 #if defined(OS_MACOSX) || defined(OS_WIN)
   // Attachments aren't supported on Mac and Windows yet.
-  GTEST_SKIP();
+  DISABLED_TEST();
 #else
   std::unique_ptr<CrashReportDatabase::NewReport> new_report;
   ASSERT_EQ(db()->PrepareNewCrashReport(&new_report),
@@ -718,7 +719,7 @@ TEST_F(CrashReportDatabaseTest, Attachments) {
 TEST_F(CrashReportDatabaseTest, OrphanedAttachments) {
 #if defined(OS_MACOSX) || defined(OS_WIN)
   // Attachments aren't supported on Mac and Windows yet.
-  GTEST_SKIP();
+  DISABLED_TEST();
 #else
   // TODO: This is using paths that are specific to the generic implementation
   // and will need to be generalized for Mac and Windows.

snapshot/win/exception_snapshot_win_test.cc

@@ -23,6 +23,7 @@
 #include "gtest/gtest.h"
 #include "snapshot/win/process_snapshot_win.h"
 #include "test/errors.h"
+#include "test/gtest_disabled.h"
 #include "test/test_paths.h"
 #include "test/win/child_launcher.h"
 #include "util/file/file_io.h"
@@ -175,7 +176,7 @@ TEST(ExceptionSnapshotWinTest, MAYBE_ChildCrash) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ExceptionSnapshotWinTest, ChildCrashWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   TestCrashingChild(TestPaths::Architecture::k32Bit);
@@ -292,7 +293,7 @@ TEST(SimulateCrash, MAYBE_ChildDumpWithoutCrashing) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(SimulateCrash, ChildDumpWithoutCrashingWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   TestDumpWithoutCrashingChild(TestPaths::Architecture::k32Bit);

snapshot/win/extra_memory_ranges_test.cc

@@ -25,6 +25,7 @@
 #include "client/simple_address_range_bag.h"
 #include "gtest/gtest.h"
 #include "snapshot/win/process_snapshot_win.h"
+#include "test/gtest_disabled.h"
 #include "test/test_paths.h"
 #include "test/win/child_launcher.h"
 #include "util/file/file_io.h"
@@ -109,7 +110,7 @@ TEST(ExtraMemoryRanges, CrashDebugBreak) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ExtraMemoryRanges, DontCrashWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   TestExtraMemoryRanges(kDontCrash, TestPaths::Architecture::k32Bit);
@@ -117,7 +118,7 @@ TEST(ExtraMemoryRanges, DontCrashWOW64) {
 
 TEST(ExtraMemoryRanges, CrashDebugBreakWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   TestExtraMemoryRanges(kCrashDebugBreak, TestPaths::Architecture::k32Bit);

snapshot/win/module_snapshot_win_test.cc

@@ -30,6 +30,7 @@
 #include "snapshot/annotation_snapshot.h"
 #include "snapshot/win/pe_image_reader.h"
 #include "snapshot/win/process_reader_win.h"
+#include "test/gtest_disabled.h"
 #include "test/test_paths.h"
 #include "test/win/child_launcher.h"
 #include "util/file/file_io.h"
@@ -151,7 +152,7 @@ TEST(ModuleSnapshotWinTest, CrashDebugBreak) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ModuleSnapshotWinTest, DontCrashWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   TestAnnotationsOnCrash(kDontCrash, TestPaths::Architecture::k32Bit);
@@ -159,7 +160,7 @@ TEST(ModuleSnapshotWinTest, DontCrashWOW64) {
 
 TEST(ModuleSnapshotWinTest, CrashDebugBreakWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   TestAnnotationsOnCrash(kCrashDebugBreak, TestPaths::Architecture::k32Bit);

snapshot/win/process_snapshot_win_test.cc

@@ -20,6 +20,7 @@
 #include "snapshot/win/pe_image_reader.h"
 #include "snapshot/win/process_reader_win.h"
 #include "test/errors.h"
+#include "test/gtest_disabled.h"
 #include "test/test_paths.h"
 #include "test/win/child_launcher.h"
 #include "util/file/file_io.h"
@@ -119,7 +120,7 @@ TEST(ProcessSnapshotTest, CrashpadInfoChild) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ProcessSnapshotTest, CrashpadInfoChildWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   TestImageReaderChild(TestPaths::Architecture::k32Bit);

test/BUILD.gn

@@ -25,6 +25,8 @@ static_library("test") {
     "filesystem.cc",
     "filesystem.h",
     "gtest_death.h",
+    "gtest_disabled.cc",
+    "gtest_disabled.h",
     "hex_string.cc",
     "hex_string.h",
     "main_arguments.cc",

test/gtest_disabled.cc

@@ -0,0 +1,83 @@
+// 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 "test/gtest_disabled.h"
+
+#include <stdio.h>
+
+#include "base/format_macros.h"
+#include "base/logging.h"
+#include "base/strings/stringprintf.h"
+
+namespace crashpad {
+namespace test {
+
+namespace {
+
+DisabledTestGtestEnvironment* g_instance;
+
+}  // namespace
+
+// static
+DisabledTestGtestEnvironment* DisabledTestGtestEnvironment::Get() {
+  if (!g_instance) {
+    g_instance = new DisabledTestGtestEnvironment();
+  }
+  return g_instance;
+}
+
+void DisabledTestGtestEnvironment::DisabledTest() {
+  const testing::TestInfo* test_info =
+      testing::UnitTest::GetInstance()->current_test_info();
+  std::string disabled_test = base::StringPrintf(
+      "%s.%s", test_info->test_case_name(), test_info->name());
+
+  // Show a DISABLED message using a format similar to gtest, along with a hint
+  // explaining that OK or FAILED will also appear.
+  printf(
+      "This test has been disabled dynamically.\n"
+      "It will appear as both DISABLED and OK or FAILED.\n"
+      "[ DISABLED ] %s\n",
+      disabled_test.c_str());
+
+  disabled_tests_.push_back(disabled_test);
+}
+
+DisabledTestGtestEnvironment::DisabledTestGtestEnvironment()
+    : testing::Environment(),
+      disabled_tests_() {
+  DCHECK(!g_instance);
+}
+
+DisabledTestGtestEnvironment::~DisabledTestGtestEnvironment() {
+  DCHECK_EQ(this, g_instance);
+  g_instance = nullptr;
+}
+
+void DisabledTestGtestEnvironment::TearDown() {
+  if (!disabled_tests_.empty()) {
+    printf(
+        "[ DISABLED ] %" PRIuS " dynamically disabled test%s, listed below:\n"
+        "[ DISABLED ] %s also counted in PASSED or FAILED below.\n",
+        disabled_tests_.size(),
+        disabled_tests_.size() == 1 ? "" : "s",
+        disabled_tests_.size() == 1 ? "This test is" : "These tests are");
+    for (const std::string& disabled_test : disabled_tests_) {
+      printf("[ DISABLED ] %s\n", disabled_test.c_str());
+    }
+  }
+}
+
+}  // namespace test
+}  // namespace crashpad

test/gtest_disabled.h

@@ -0,0 +1,87 @@
+// 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.
+
+#ifndef CRASHPAD_TEST_GTEST_DISABLED_H_
+#define CRASHPAD_TEST_GTEST_DISABLED_H_
+
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "gtest/gtest.h"
+
+//! \file
+
+namespace crashpad {
+namespace test {
+
+//! \brief Provides support for dynamically disabled gtest tests.
+//!
+//! A test runner must register this with gtest as follows prior to calling
+//! `RUN_ALL_TESTS()`:
+//! \code
+//!   testing::AddGlobalTestEnvironment(
+//!       crashpad::test::DisabledTestGtestEnvironment::Get());
+//! \endcode
+class DisabledTestGtestEnvironment final : public testing::Environment {
+ public:
+  //! \brief Returns the DisabledTestGtestEnvironment singleton instance,
+  //!     creating it if necessary.
+  static DisabledTestGtestEnvironment* Get();
+
+  //! \brief Displays a message about a test being disabled, and arranges for
+  //!     this information to be duplicated in TearDown().
+  //!
+  //! This method is for the internal use of the DISABLED_TEST() macro. Do not
+  //! call it directly, use the macro instead.
+  void DisabledTest();
+
+ private:
+  DisabledTestGtestEnvironment();
+  ~DisabledTestGtestEnvironment() override;
+
+  // testing::Environment:
+  void TearDown() override;
+
+  std::vector<std::string> disabled_tests_;
+
+  DISALLOW_COPY_AND_ASSIGN(DisabledTestGtestEnvironment);
+};
+
+}  // namespace test
+}  // namespace crashpad
+
+//! \brief Displays a message about a test being disabled, and returns early.
+//!
+//! gtest only provides a mechanism for tests to be disabled statically, by
+//! prefixing test case names or test names with `DISABLED_`. When it is
+//! necessary to disable tests dynamically, gtest provides no assistance. This
+//! macro displays a message about the disabled test and returns early. The
+//! dynamically disabled test will also be displayed during gtest global test
+//! environment tear-down before the test executable exits.
+//!
+//! This macro may only be invoked from the context of a gtest test.
+//!
+//! There’s a long-standing <a
+//! href="https://groups.google.com/d/topic/googletestframework/Nwh3u7YFuN4">gtest
+//! feature request</a> to provide this functionality directly in gtest, but
+//! since it hasn’t been implemented, this macro provides a local mechanism to
+//! achieve it.
+#define DISABLED_TEST()                                                    \
+  do {                                                                     \
+    ::crashpad::test::DisabledTestGtestEnvironment::Get()->DisabledTest(); \
+    return;                                                                \
+  } while (false)
+
+#endif  // CRASHPAD_TEST_GTEST_DISABLED_H_

test/gtest_main.cc

@@ -14,6 +14,7 @@
 
 #include "build/build_config.h"
 #include "gtest/gtest.h"
+#include "test/gtest_disabled.h"
 #include "test/main_arguments.h"
 #include "test/multiprocess_exec.h"
 
@@ -50,6 +51,8 @@ bool GetChildTestFunctionName(std::string* child_func_name) {
 
 int main(int argc, char* argv[]) {
   crashpad::test::InitializeMainArguments(argc, argv);
+  testing::AddGlobalTestEnvironment(
+      crashpad::test::DisabledTestGtestEnvironment::Get());
 
   std::string child_func_name;
   if (GetChildTestFunctionName(&child_func_name)) {

test/test.gyp

@@ -37,6 +37,8 @@
         'filesystem.cc',
         'filesystem.h',
         'gtest_death.h',
+        'gtest_disabled.cc',
+        'gtest_disabled.h',
         'hex_string.cc',
         'hex_string.h',
         'linux/fake_ptrace_connection.cc',

util/file/directory_reader_test.cc

@@ -22,6 +22,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "gtest/gtest.h"
 #include "test/filesystem.h"
+#include "test/gtest_disabled.h"
 #include "test/scoped_temp_dir.h"
 #include "util/file/file_io.h"
 #include "util/file/filesystem.h"
@@ -47,7 +48,7 @@ TEST(DirectoryReader, BadPaths) {
 
 TEST(DirectoryReader, BadPaths_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   ScopedTempDir temp_dir;
@@ -143,7 +144,7 @@ TEST(DirectoryReader, FilesAndDirectories) {
 
 TEST(DirectoryReader, FilesAndDirectories_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   TestFilesAndDirectories(true);

util/file/filesystem_test.cc

@@ -21,6 +21,7 @@
 #include "gtest/gtest.h"
 #include "test/errors.h"
 #include "test/filesystem.h"
+#include "test/gtest_disabled.h"
 #include "test/scoped_temp_dir.h"
 #include "util/misc/time.h"
 
@@ -92,7 +93,7 @@ TEST(Filesystem, FileModificationTime) {
 
 TEST(Filesystem, FileModificationTime_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   ScopedTempDir temp_dir;
@@ -223,7 +224,7 @@ TEST(Filesystem, MoveFileOrDirectory) {
 
 TEST(Filesystem, MoveFileOrDirectory_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   ScopedTempDir temp_dir;
@@ -301,7 +302,7 @@ TEST(Filesystem, IsRegularFile) {
 
 TEST(Filesystem, IsRegularFile_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   ScopedTempDir temp_dir;
@@ -343,7 +344,7 @@ TEST(Filesystem, IsDirectory) {
 
 TEST(Filesystem, IsDirectory_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   ScopedTempDir temp_dir;
@@ -392,7 +393,7 @@ TEST(Filesystem, RemoveFile) {
 
 TEST(Filesystem, RemoveFile_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   ScopedTempDir temp_dir;
@@ -449,7 +450,7 @@ TEST(Filesystem, RemoveDirectory) {
 
 TEST(Filesystem, RemoveDirectory_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   ScopedTempDir temp_dir;

util/win/process_info_test.cc

@@ -26,6 +26,7 @@
 #include "build/build_config.h"
 #include "gtest/gtest.h"
 #include "test/errors.h"
+#include "test/gtest_disabled.h"
 #include "test/scoped_temp_dir.h"
 #include "test/test_paths.h"
 #include "test/win/child_launcher.h"
@@ -201,7 +202,7 @@ TEST(ProcessInfo, OtherProcess) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ProcessInfo, OtherProcessWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    GTEST_SKIP();
+    DISABLED_TEST();
   }
 
   TestOtherProcess(TestPaths::Architecture::k32Bit);