Use GTEST_SKIP() instead of custom DISABLED_TEST()

Since gtest 00938b2b228f, 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.

This was done previously in 79f4a3970a64, but was reverted in
bba9d0819c12 because Chromium’s test launcher did not support
GTEST_SKIP() at the time. The deficiency is on file as
https://crbug.com/912138.

While that bug was never specifically marked as “fixed” and I haven’t
found what changed in Chromium, I do now see some use of GTEST_SKIP() in
Chromium. I also prototyped this change in Chromium at
https://chromium-review.googlesource.com/c/1854691/ and found that
GTEST_SKIP() does indeed now appear to work.

Change-Id: I13fef8fe8bfd9854a40dfa5910a3282d1a85bc45
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1855380
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
This commit is contained in:
Mark Mentovai 2019-10-11 12:07:21 -04:00 committed by Commit Bot
parent 2fb4e9e6a4
commit c009b85449
14 changed files with 19 additions and 205 deletions

View File

@ -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"
@ -671,7 +670,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),
@ -717,7 +716,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.

View File

@ -38,7 +38,6 @@
#include "build/build_config.h"
#include "gtest/gtest.h"
#include "test/errors.h"
#include "test/gtest_disabled.h"
#include "test/linux/fake_ptrace_connection.h"
#include "test/linux/get_tls.h"
#include "test/multiprocess.h"
@ -809,7 +808,7 @@ TEST(ProcessReaderLinux, AbortMessage) {
// presence of a libc symbol which was introduced in Q.
if (!crashpad::internal::Dlsym(RTLD_DEFAULT,
"android_fdsan_close_with_tag")) {
DISABLED_TEST();
GTEST_SKIP();
}
android_set_abort_message(kTestAbortMessage);

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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);

View File

@ -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",

View File

@ -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

View File

@ -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.
//!
//! Theres 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 hasnt 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_

View File

@ -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)) {

View File

@ -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',

View File

@ -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);

View File

@ -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;

View File

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