Make test environment setup consistent with gtest model.
Allow gtest to manage and destroy environments on our behalf
without any explicit cleanup. Rewrite unit_test_main.cpp to
take advantage of this design.
-- rename files to match class names.
-- deal with fallout in build files.
-- nest some xfa conditionals inside v8 conditionals.
-- add one #error case if !V8
-- remove an unnecessary slash in one .gn file.
Change-Id: I007debe12a74a91b25aedfa77ba73273d1c48fa6
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/71850
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/BUILD.gn b/BUILD.gn
index 24d61e1..945314b 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -209,7 +209,7 @@
testonly = true
public_deps = [
"core/fxcrt",
- "testing:test_support",
+ "testing:unit_test_support",
"//testing/gmock",
"//testing/gtest",
]
@@ -239,7 +239,6 @@
"core/fxcrt:unittests",
"core/fxge:unittests",
"fpdfsdk:unittests",
- "testing:test_support",
"testing:unit_test_support",
"//testing/gmock",
"//testing/gtest",
@@ -254,22 +253,21 @@
configs += [ "//v8:external_startup_data" ]
deps += [
"fxjs:unittests",
- "testing:pdfium_v8_gtest_support",
"//v8",
]
- }
- if (pdf_enable_xfa) {
- deps += [
- "core/fxcrt/css:unittests",
- "fxbarcode:unittests",
- "xfa/fde:unittests",
- "xfa/fgas:unittests",
- "xfa/fgas/layout:unittests",
- "xfa/fxfa:unittests",
- "xfa/fxfa/fm2js:unittests",
- "xfa/fxfa/parser:unittests",
- ]
+ if (pdf_enable_xfa) {
+ deps += [
+ "core/fxcrt/css:unittests",
+ "fxbarcode:unittests",
+ "xfa/fde:unittests",
+ "xfa/fgas:unittests",
+ "xfa/fgas/layout:unittests",
+ "xfa/fxfa:unittests",
+ "xfa/fxfa/fm2js:unittests",
+ "xfa/fxfa/parser:unittests",
+ ]
+ }
}
}
@@ -279,7 +277,6 @@
":pdfium_public_headers",
"core/fxcrt",
"testing:embedder_test_support",
- "testing:test_support",
"third_party:pdfium_base_test_support",
"//testing/gmock",
"//testing/gtest",
@@ -290,9 +287,6 @@
"fxjs/*",
"xfa/*",
]
- if (pdf_enable_v8) {
- public_deps += [ "testing:pdfium_v8_gtest_support" ]
- }
}
test("pdfium_embeddertests") {
@@ -308,7 +302,6 @@
"core/fxge:embeddertests",
"fpdfsdk:embeddertests",
"fpdfsdk/pwl:embeddertests",
- "testing:test_support",
"testing/image_diff",
"//testing/gmock",
"//testing/gtest",
diff --git a/fxjs/BUILD.gn b/fxjs/BUILD.gn
index bfff222..10cb49b 100644
--- a/fxjs/BUILD.gn
+++ b/fxjs/BUILD.gn
@@ -236,11 +236,7 @@
"gc/gced_tree_node_unittest.cpp",
"gc/heap_unittest.cpp",
]
- deps += [
- "../testing:pdfium_v8_gtest_support",
- "../testing:unit_test_support",
- "//v8:cppgc",
- ]
+ deps += [ "//v8:cppgc" ]
}
}
diff --git a/samples/BUILD.gn b/samples/BUILD.gn
index 581d05a..4717502 100644
--- a/samples/BUILD.gn
+++ b/samples/BUILD.gn
@@ -50,7 +50,7 @@
deps = [
"../:pdfium_public_headers",
"../fpdfsdk",
- "../testing/:test_support",
+ "../testing:test_support",
"../testing/image_diff",
"../third_party:pdfium_base",
"//build/win:default_exe_manifest",
diff --git a/testing/BUILD.gn b/testing/BUILD.gn
index 4f04ce0..4ebfef0 100644
--- a/testing/BUILD.gn
+++ b/testing/BUILD.gn
@@ -16,7 +16,6 @@
"string_write_stream.h",
"test_loader.cpp",
"test_loader.h",
- "test_support.cpp",
"test_support.h",
"utils/bitmap_saver.cpp",
"utils/bitmap_saver.h",
@@ -51,50 +50,50 @@
}
}
-if (pdf_enable_v8) {
- source_set("pdfium_v8_gtest_support") {
- testonly = true
- sources = [
+source_set("test_environments") {
+ testonly = true
+ sources = [
+ "pdf_test_environment.cpp",
+ "pdf_test_environment.h",
+ ]
+ deps = [
+ ":test_support",
+ "../core/fxcrt",
+ "../core/fxge",
+ "//testing/gtest",
+ ]
+ configs += [ "../:pdfium_core_config" ]
+
+ if (pdf_enable_v8) {
+ sources += [
"v8_test_environment.cpp",
"v8_test_environment.h",
]
- deps = [
- ":test_support",
- "../core/fxcrt",
- "//testing/gtest",
+ deps += [
"//v8",
"//v8:v8_libplatform",
]
- configs += [
- "../:pdfium_core_config",
- "//v8:external_startup_data",
+ configs += [ "//v8:external_startup_data" ]
+ }
+
+ if (pdf_enable_xfa) {
+ sources += [
+ "xfa_test_environment.cpp",
+ "xfa_test_environment.h",
+ ]
+ deps += [
+ "../xfa/fgas",
+ "//v8:cppgc",
]
}
}
source_set("unit_test_support") {
testonly = true
- sources = []
- deps = []
- configs += [ "../:pdfium_core_config" ]
- visibility = [ "../*" ]
-
- if (pdf_enable_v8) {
- deps += [ ":pdfium_v8_gtest_support" ]
-
- if (pdf_enable_xfa) {
- sources += [
- "xfa_unit_test_support.cpp",
- "xfa_unit_test_support.h",
- ]
- deps += [
- "../:pdfium",
- "../core/fxge",
- "../xfa/fgas",
- "//testing/gtest",
- ]
- }
- }
+ public_deps = [
+ ":test_environments",
+ ":test_support",
+ ]
}
source_set("embedder_test_support") {
@@ -112,7 +111,6 @@
"range_set.h",
]
deps = [
- ":test_support",
"../:pdfium_public_headers",
"../core/fdrm",
"../core/fxcrt",
@@ -120,6 +118,10 @@
"//testing/gmock",
"//testing/gtest",
]
+ public_deps = [
+ ":test_environments",
+ ":test_support",
+ ]
configs += [ "../:pdfium_core_config" ]
visibility = [ "../*" ]
@@ -131,8 +133,9 @@
"js_embedder_test.h",
]
deps += [
- ":pdfium_v8_gtest_support",
"../fxjs",
+ "//v8",
+ "//v8:v8_libplatform",
]
configs += [ "//v8:external_startup_data" ]
@@ -146,8 +149,6 @@
"../fpdfsdk/fpdfxfa",
"../xfa/fxfa",
"../xfa/fxfa/parser",
- "//v8:cppgc",
- "//v8:v8_libplatform",
]
}
}
diff --git a/testing/pdf_test_environment.cpp b/testing/pdf_test_environment.cpp
new file mode 100644
index 0000000..2ab98f4
--- /dev/null
+++ b/testing/pdf_test_environment.cpp
@@ -0,0 +1,20 @@
+// Copyright 2020 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/pdf_test_environment.h"
+
+#include "core/fxge/cfx_gemodule.h"
+
+PDFTestEnvironment::PDFTestEnvironment() = default;
+
+PDFTestEnvironment::~PDFTestEnvironment() = default;
+
+// testing::Environment:
+void PDFTestEnvironment::SetUp() {
+ CFX_GEModule::Create(nullptr);
+}
+
+void PDFTestEnvironment::TearDown() {
+ CFX_GEModule::Destroy();
+}
diff --git a/testing/pdf_test_environment.h b/testing/pdf_test_environment.h
new file mode 100644
index 0000000..3bb7ad0
--- /dev/null
+++ b/testing/pdf_test_environment.h
@@ -0,0 +1,20 @@
+// Copyright 2020 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef TESTING_PDF_TEST_ENVIRONMENT_H_
+#define TESTING_PDF_TEST_ENVIRONMENT_H_
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+class PDFTestEnvironment : public testing::Environment {
+ public:
+ PDFTestEnvironment();
+ ~PDFTestEnvironment() override;
+
+ // testing::Environment:
+ void SetUp() override;
+ void TearDown() override;
+};
+
+#endif // TESTING_PDF_TEST_ENVIRONMENT_H_
diff --git a/testing/test_support.cpp b/testing/test_support.cpp
deleted file mode 100644
index 94ce528..0000000
--- a/testing/test_support.cpp
+++ /dev/null
@@ -1,15 +0,0 @@
-// Copyright 2019 PDFium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "testing/test_support.h"
-
-#include "core/fxge/cfx_gemodule.h"
-
-void InitializePDFTestEnvironment() {
- CFX_GEModule::Create(nullptr);
-}
-
-void DestroyPDFTestEnvironment() {
- CFX_GEModule::Destroy();
-}
diff --git a/testing/test_support.h b/testing/test_support.h
index f237ea5..382ba7e 100644
--- a/testing/test_support.h
+++ b/testing/test_support.h
@@ -46,7 +46,4 @@
} // namespace pdfium
-void InitializePDFTestEnvironment();
-void DestroyPDFTestEnvironment();
-
#endif // TESTING_TEST_SUPPORT_H_
diff --git a/testing/unit_test_main.cpp b/testing/unit_test_main.cpp
index b36ac80..7ccfdf8 100644
--- a/testing/unit_test_main.cpp
+++ b/testing/unit_test_main.cpp
@@ -8,42 +8,36 @@
#include "core/fxcrt/fx_memory.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
-#include "testing/test_support.h"
+#include "testing/pdf_test_environment.h"
#ifdef PDF_ENABLE_V8
#include "testing/v8_test_environment.h"
#include "v8/include/v8-platform.h"
#include "v8/include/v8.h"
+#ifdef PDF_ENABLE_XFA
+#include "testing/xfa_test_environment.h"
+#endif // PDF_ENABLE_XFA
#endif // PDF_ENABLE_V8
-#ifdef PDF_ENABLE_XFA
-#include "testing/xfa_unit_test_support.h"
-#endif // PDF_ENABLE_XFA
-
// Can't use gtest-provided main since we need to initialize partition
-// alloc before invoking any test.
+// alloc before invoking any test, and add test environments.
int main(int argc, char** argv) {
FXMEM_InitializePartitionAlloc();
+ // PDF test environment will be deleted by gtest.
+ AddGlobalTestEnvironment(new PDFTestEnvironment());
+
#ifdef PDF_ENABLE_V8
// V8 test environment will be deleted by gtest.
AddGlobalTestEnvironment(new V8TestEnvironment(argv[0]));
-#endif // PDF_ENABLE_V8
-
- InitializePDFTestEnvironment();
-
#ifdef PDF_ENABLE_XFA
// XFA test environment will be deleted by gtest.
- InitializeXFATestEnvironment();
+ AddGlobalTestEnvironment(new XFATestEnvironment());
#endif // PDF_ENABLE_XFA
+#endif // PDF_ENABLE_V8
testing::InitGoogleTest(&argc, argv);
testing::InitGoogleMock(&argc, argv);
- int ret_val = RUN_ALL_TESTS();
-
- DestroyPDFTestEnvironment();
-
-
- return ret_val;
+ return RUN_ALL_TESTS();
}
diff --git a/testing/v8_test_environment.h b/testing/v8_test_environment.h
index 3cce117..b5dc49b 100644
--- a/testing/v8_test_environment.h
+++ b/testing/v8_test_environment.h
@@ -9,6 +9,10 @@
#include "testing/gtest/include/gtest/gtest.h"
+#ifndef PDF_ENABLE_V8
+#error "V8 must be enabled"
+#endif // PDF_ENABLE_V8
+
namespace v8 {
class Isolate;
class Platform;
diff --git a/testing/xfa_test_environment.cpp b/testing/xfa_test_environment.cpp
new file mode 100644
index 0000000..797341a
--- /dev/null
+++ b/testing/xfa_test_environment.cpp
@@ -0,0 +1,50 @@
+// Copyright 2019 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/xfa_test_environment.h"
+
+#include <memory>
+#include <utility>
+
+#include "core/fxge/cfx_fontmgr.h"
+#include "core/fxge/cfx_gemodule.h"
+#include "core/fxge/systemfontinfo_iface.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "xfa/fgas/font/cfgas_fontmgr.h"
+
+namespace {
+
+XFATestEnvironment* g_env = nullptr;
+
+} // namespace
+
+XFATestEnvironment::XFATestEnvironment() {
+ ASSERT(!g_env);
+ g_env = this;
+}
+
+XFATestEnvironment::~XFATestEnvironment() {
+ ASSERT(g_env);
+ g_env = nullptr;
+}
+
+void XFATestEnvironment::SetUp() {
+ // TODO(dsinclair): This font loading is slow. We should make a test font
+ // loader which loads up a single font we use in all tests.
+ CFX_GEModule::Get()->GetFontMgr()->SetSystemFontInfo(
+ SystemFontInfoIface::CreateDefault(nullptr));
+
+ auto font_mgr = std::make_unique<CFGAS_FontMgr>();
+ if (font_mgr->EnumFonts())
+ font_mgr_ = std::move(font_mgr);
+}
+
+void XFATestEnvironment::TearDown() {
+ font_mgr_.reset();
+}
+
+// static
+CFGAS_FontMgr* XFATestEnvironment::GetGlobalFontManager() {
+ return g_env->font_mgr_.get();
+}
diff --git a/testing/xfa_test_environment.h b/testing/xfa_test_environment.h
new file mode 100644
index 0000000..af991ac
--- /dev/null
+++ b/testing/xfa_test_environment.h
@@ -0,0 +1,38 @@
+// Copyright 2019 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef TESTING_XFA_UNIT_TEST_SUPPORT_H_
+#define TESTING_XFA_UNIT_TEST_SUPPORT_H_
+
+#include <memory>
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+#ifndef PDF_ENABLE_XFA
+#error "XFA must be enabled"
+#endif
+
+class CFGAS_FontMgr;
+
+// XFATestEnvironement loads a CFGAS_FontMgr, whose time is linear in the
+// number of times it is loaded. So, if a test suite has a lot of tests that
+// need a font manager they can end up executing very, very slowly.
+
+class XFATestEnvironment : public testing::Environment {
+ public:
+ XFATestEnvironment();
+ ~XFATestEnvironment();
+
+ // testing::TestEnvironment:
+ void SetUp() override;
+ void TearDown() override;
+
+ // Does not create if not present.
+ static CFGAS_FontMgr* GetGlobalFontManager();
+
+ private:
+ std::unique_ptr<CFGAS_FontMgr> font_mgr_;
+};
+
+#endif // TESTING_XFA_UNIT_TEST_SUPPORT_H_
diff --git a/testing/xfa_unit_test_support.cpp b/testing/xfa_unit_test_support.cpp
deleted file mode 100644
index ae7f480..0000000
--- a/testing/xfa_unit_test_support.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright 2019 PDFium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "testing/xfa_unit_test_support.h"
-
-#include <memory>
-#include <utility>
-
-#include "core/fxge/cfx_fontmgr.h"
-#include "core/fxge/cfx_gemodule.h"
-#include "core/fxge/systemfontinfo_iface.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "xfa/fgas/font/cfgas_fontmgr.h"
-
-namespace {
-
-// The loading time of the CFGAS_FontMgr is linear in the number of times it is
-// loaded. So, if a test suite has a lot of tests that need a font manager they
-// can end up executing very, very slowly.
-class XFATestEnvironment final : public testing::Environment {
- public:
- // testing::Environment:
- void SetUp() override {
- auto font_mgr = std::make_unique<CFGAS_FontMgr>();
- if (font_mgr->EnumFonts())
- font_mgr_ = std::move(font_mgr);
- }
- void TearDown() override { font_mgr_.reset(); }
-
- CFGAS_FontMgr* FontManager() const { return font_mgr_.get(); }
-
- private:
- std::unique_ptr<CFGAS_FontMgr> font_mgr_;
-};
-
-XFATestEnvironment* g_env = nullptr;
-
-} // namespace
-
-void InitializeXFATestEnvironment() {
- // |g_env| will be deleted by gtest.
- g_env = new XFATestEnvironment();
- AddGlobalTestEnvironment(g_env);
-
- // TODO(dsinclair): This font loading is slow. We should make a test font
- // loader which loads up a single font we use in all tests.
- CFX_GEModule::Get()->GetFontMgr()->SetSystemFontInfo(
- SystemFontInfoIface::CreateDefault(nullptr));
-}
-
-CFGAS_FontMgr* GetGlobalFontManager() {
- return g_env->FontManager();
-}
diff --git a/testing/xfa_unit_test_support.h b/testing/xfa_unit_test_support.h
deleted file mode 100644
index 0a54778..0000000
--- a/testing/xfa_unit_test_support.h
+++ /dev/null
@@ -1,18 +0,0 @@
-// Copyright 2019 PDFium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef TESTING_XFA_UNIT_TEST_SUPPORT_H_
-#define TESTING_XFA_UNIT_TEST_SUPPORT_H_
-
-#ifndef PDF_ENABLE_XFA
-#error "XFA must be enabled"
-#endif
-
-class CFGAS_FontMgr;
-
-void InitializeXFATestEnvironment();
-
-CFGAS_FontMgr* GetGlobalFontManager();
-
-#endif // TESTING_XFA_UNIT_TEST_SUPPORT_H_
diff --git a/xfa/fde/BUILD.gn b/xfa/fde/BUILD.gn
index 1806683..6c46b9a 100644
--- a/xfa/fde/BUILD.gn
+++ b/xfa/fde/BUILD.gn
@@ -35,7 +35,6 @@
deps = [
":fde",
"../../core/fxge",
- "../../testing:unit_test_support",
"../fgas",
]
pdfium_root_dir = "../../"
diff --git a/xfa/fde/cfde_texteditengine_unittest.cpp b/xfa/fde/cfde_texteditengine_unittest.cpp
index cef7628..6195cdb 100644
--- a/xfa/fde/cfde_texteditengine_unittest.cpp
+++ b/xfa/fde/cfde_texteditengine_unittest.cpp
@@ -6,7 +6,7 @@
#include "core/fxge/text_char_pos.h"
#include "testing/gtest/include/gtest/gtest.h"
-#include "testing/xfa_unit_test_support.h"
+#include "testing/xfa_test_environment.h"
#include "xfa/fgas/font/cfgas_gefont.h"
class CFDE_TextEditEngineTest : public testing::Test {
@@ -37,9 +37,9 @@
~CFDE_TextEditEngineTest() override {}
void SetUp() override {
- font_ =
- CFGAS_GEFont::LoadFont(L"Arial Black", 0, 0, GetGlobalFontManager());
- ASSERT_TRUE(font_.Get() != nullptr);
+ font_ = CFGAS_GEFont::LoadFont(L"Arial Black", 0, 0,
+ XFATestEnvironment::GetGlobalFontManager());
+ ASSERT_TRUE(font_);
engine_ = std::make_unique<CFDE_TextEditEngine>();
engine_->SetFont(font_);
diff --git a/xfa/fgas/layout/BUILD.gn b/xfa/fgas/layout/BUILD.gn
index 61d2e75..e991854 100644
--- a/xfa/fgas/layout/BUILD.gn
+++ b/xfa/fgas/layout/BUILD.gn
@@ -54,7 +54,6 @@
":layout",
"../:fgas",
"../../../core/fxge",
- "../../../testing:unit_test_support",
]
pdfium_root_dir = "../../../"
}
diff --git a/xfa/fgas/layout/cfx_rtfbreak_unittest.cpp b/xfa/fgas/layout/cfx_rtfbreak_unittest.cpp
index 7ccb919..e104a2f 100644
--- a/xfa/fgas/layout/cfx_rtfbreak_unittest.cpp
+++ b/xfa/fgas/layout/cfx_rtfbreak_unittest.cpp
@@ -12,7 +12,7 @@
#include "core/fxge/cfx_font.h"
#include "core/fxge/cfx_gemodule.h"
#include "testing/gtest/include/gtest/gtest.h"
-#include "testing/xfa_unit_test_support.h"
+#include "testing/xfa_test_environment.h"
#include "xfa/fgas/font/cfgas_fontmgr.h"
#include "xfa/fgas/font/cfgas_gefont.h"
#include "xfa/fgas/layout/cfx_char.h"
@@ -20,9 +20,9 @@
class CFX_RTFBreakTest : public testing::Test {
public:
void SetUp() override {
- font_ =
- CFGAS_GEFont::LoadFont(L"Arial Black", 0, 0, GetGlobalFontManager());
- ASSERT_TRUE(font_.Get());
+ font_ = CFGAS_GEFont::LoadFont(L"Arial Black", 0, 0,
+ XFATestEnvironment::GetGlobalFontManager());
+ ASSERT_TRUE(font_);
}
std::unique_ptr<CFX_RTFBreak> CreateBreak(uint32_t layout_styles) {
diff --git a/xfa/fgas/layout/cfx_txtbreak_unittest.cpp b/xfa/fgas/layout/cfx_txtbreak_unittest.cpp
index 9825a32..792f006 100644
--- a/xfa/fgas/layout/cfx_txtbreak_unittest.cpp
+++ b/xfa/fgas/layout/cfx_txtbreak_unittest.cpp
@@ -9,7 +9,7 @@
#include "core/fxge/cfx_font.h"
#include "testing/gtest/include/gtest/gtest.h"
-#include "testing/xfa_unit_test_support.h"
+#include "testing/xfa_test_environment.h"
#include "xfa/fgas/font/cfgas_fontmgr.h"
#include "xfa/fgas/font/cfgas_gefont.h"
#include "xfa/fgas/layout/cfx_char.h"
@@ -17,9 +17,9 @@
class CFX_TxtBreakTest : public testing::Test {
public:
void SetUp() override {
- font_ =
- CFGAS_GEFont::LoadFont(L"Arial Black", 0, 0, GetGlobalFontManager());
- ASSERT_TRUE(font_.Get());
+ font_ = CFGAS_GEFont::LoadFont(L"Arial Black", 0, 0,
+ XFATestEnvironment::GetGlobalFontManager());
+ ASSERT_TRUE(font_);
}
std::unique_ptr<CFX_TxtBreak> CreateBreak() {
