Use GTEST_SKIP() instead of custom DISABLED_TEST()

Since gtest 00938b2b228f3, gtest has built-in first-class support for
skipping tests, which is functionally identical (at least in Crashpad’s
usage) to the home-grown support for run-time dynamically disabled tests
introduced in Crashpad 5e9ed4cb9f69.

Use the new standard pattern, and remove all vestiges of the custom
local one.

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

client/crash_report_database_test.cc

@@ -20,7 +20,6 @@
 #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"
@@ -673,7 +672,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.
-  DISABLED_TEST();
+  GTEST_SKIP();
 #else
   std::unique_ptr<CrashReportDatabase::NewReport> new_report;
   ASSERT_EQ(db()->PrepareNewCrashReport(&new_report),
@@ -719,7 +718,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.
-  DISABLED_TEST();
+  GTEST_SKIP();
 #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,7 +23,6 @@
 #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"
@@ -176,7 +175,7 @@ TEST(ExceptionSnapshotWinTest, MAYBE_ChildCrash) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ExceptionSnapshotWinTest, ChildCrashWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   TestCrashingChild(TestPaths::Architecture::k32Bit);
@@ -293,7 +292,7 @@ TEST(SimulateCrash, MAYBE_ChildDumpWithoutCrashing) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(SimulateCrash, ChildDumpWithoutCrashingWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   TestDumpWithoutCrashingChild(TestPaths::Architecture::k32Bit);

snapshot/win/extra_memory_ranges_test.cc

@@ -25,7 +25,6 @@
 #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"
@@ -110,7 +109,7 @@ TEST(ExtraMemoryRanges, CrashDebugBreak) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ExtraMemoryRanges, DontCrashWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   TestExtraMemoryRanges(kDontCrash, TestPaths::Architecture::k32Bit);
@@ -118,7 +117,7 @@ TEST(ExtraMemoryRanges, DontCrashWOW64) {
 
 TEST(ExtraMemoryRanges, CrashDebugBreakWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   TestExtraMemoryRanges(kCrashDebugBreak, TestPaths::Architecture::k32Bit);

snapshot/win/module_snapshot_win_test.cc

@@ -30,7 +30,6 @@
 #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"
@@ -152,7 +151,7 @@ TEST(ModuleSnapshotWinTest, CrashDebugBreak) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ModuleSnapshotWinTest, DontCrashWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   TestAnnotationsOnCrash(kDontCrash, TestPaths::Architecture::k32Bit);
@@ -160,7 +159,7 @@ TEST(ModuleSnapshotWinTest, DontCrashWOW64) {
 
 TEST(ModuleSnapshotWinTest, CrashDebugBreakWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   TestAnnotationsOnCrash(kCrashDebugBreak, TestPaths::Architecture::k32Bit);

snapshot/win/process_snapshot_win_test.cc

@@ -20,7 +20,6 @@
 #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"
@@ -120,7 +119,7 @@ TEST(ProcessSnapshotTest, CrashpadInfoChild) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ProcessSnapshotTest, CrashpadInfoChildWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   TestImageReaderChild(TestPaths::Architecture::k32Bit);

test/BUILD.gn

@@ -25,8 +25,6 @@ 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

@@ -1,83 +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 "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

@@ -1,87 +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.
-
-#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,7 +14,6 @@
 
 #include "build/build_config.h"
 #include "gtest/gtest.h"
-#include "test/gtest_disabled.h"
 #include "test/main_arguments.h"
 #include "test/multiprocess_exec.h"
 
@@ -51,8 +50,6 @@ 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,8 +37,6 @@
         '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,7 +22,6 @@
 #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"
@@ -48,7 +47,7 @@ TEST(DirectoryReader, BadPaths) {
 
 TEST(DirectoryReader, BadPaths_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   ScopedTempDir temp_dir;
@@ -144,7 +143,7 @@ TEST(DirectoryReader, FilesAndDirectories) {
 
 TEST(DirectoryReader, FilesAndDirectories_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   TestFilesAndDirectories(true);

util/file/filesystem_test.cc

@@ -21,7 +21,6 @@
 #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"
 
@@ -93,7 +92,7 @@ TEST(Filesystem, FileModificationTime) {
 
 TEST(Filesystem, FileModificationTime_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   ScopedTempDir temp_dir;
@@ -224,7 +223,7 @@ TEST(Filesystem, MoveFileOrDirectory) {
 
 TEST(Filesystem, MoveFileOrDirectory_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   ScopedTempDir temp_dir;
@@ -302,7 +301,7 @@ TEST(Filesystem, IsRegularFile) {
 
 TEST(Filesystem, IsRegularFile_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   ScopedTempDir temp_dir;
@@ -344,7 +343,7 @@ TEST(Filesystem, IsDirectory) {
 
 TEST(Filesystem, IsDirectory_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   ScopedTempDir temp_dir;
@@ -393,7 +392,7 @@ TEST(Filesystem, RemoveFile) {
 
 TEST(Filesystem, RemoveFile_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   ScopedTempDir temp_dir;
@@ -450,7 +449,7 @@ TEST(Filesystem, RemoveDirectory) {
 
 TEST(Filesystem, RemoveDirectory_SymbolicLinks) {
   if (!CanCreateSymbolicLinks()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   ScopedTempDir temp_dir;

util/win/process_info_test.cc

@@ -26,7 +26,6 @@
 #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"
@@ -202,7 +201,7 @@ TEST(ProcessInfo, OtherProcess) {
 #if defined(ARCH_CPU_64_BITS)
 TEST(ProcessInfo, OtherProcessWOW64) {
   if (!TestPaths::Has32BitBuildArtifacts()) {
-    DISABLED_TEST();
+    GTEST_SKIP();
   }
 
   TestOtherProcess(TestPaths::Architecture::k32Bit);