From 31703a585fb3f191b060b534b6ae13b1f9c36dc3 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 4 May 2018 17:06:13 -0700 Subject: [PATCH] fuchsia: When in Fuchsia tree, disable tests requiring external files The package deployment/running is in flux at the moment. In order to get all the other tests on to the main Fuchsia waterfall, disable the ~25 tests that require external files (for launching child processes, loading modules, or data files) because those operations all fail on Fuchsia-without-packages right now. Upstream this is PKG-46. Once test packaging and running has been resolved, this can be reverted. These tests are still run when building Crashpad standalone on Fuchsia as the standalone build simply copies all the relevant data files to the device in /tmp. Bug: crashpad:196 Change-Id: I1677c394a2b9d709c59363ebeea8aff193d4c21d Reviewed-on: https://chromium-review.googlesource.com/1045547 Commit-Queue: Scott Graham Reviewed-by: Joshua Peraza --- build/BUILD.gn | 6 ++++++ snapshot/crashpad_info_client_options_test.cc | 7 +++++++ snapshot/elf/elf_image_reader_test.cc | 4 ++++ .../fuchsia/process_reader_fuchsia_test.cc | 5 +++++ test/BUILD.gn | 5 ++++- test/multiprocess_exec_test.cc | 4 ++++ test/test_paths.cc | 9 +++++++++ test/test_paths.h | 9 +++++++++ test/test_paths_test.cc | 4 ++++ util/net/http_body_test.cc | 10 ++++++++++ util/net/http_multipart_builder_test.cc | 10 ++++++++++ util/net/http_transport_test.cc | 19 +++++++++++++++++++ 12 files changed, 91 insertions(+), 1 deletion(-) diff --git a/build/BUILD.gn b/build/BUILD.gn index ceae81ec..5e7aed0c 100644 --- a/build/BUILD.gn +++ b/build/BUILD.gn @@ -24,6 +24,12 @@ config("crashpad_is_in_chromium") { } } +config("crashpad_is_in_fuchsia") { + if (crashpad_is_in_fuchsia) { + defines = [ "CRASHPAD_IS_IN_FUCHSIA" ] + } +} + group("default_exe_manifest_win") { if (crashpad_is_in_chromium) { deps = [ diff --git a/snapshot/crashpad_info_client_options_test.cc b/snapshot/crashpad_info_client_options_test.cc index 1fe38acf..f23a179f 100644 --- a/snapshot/crashpad_info_client_options_test.cc +++ b/snapshot/crashpad_info_client_options_test.cc @@ -22,6 +22,7 @@ #include "client/crashpad_info.h" #include "gtest/gtest.h" #include "test/errors.h" +#include "test/gtest_disabled.h" #include "test/scoped_module_handle.h" #include "test/test_paths.h" @@ -147,6 +148,9 @@ TEST(CrashpadInfoClientOptions, OneModule) { } TEST(CrashpadInfoClientOptions, TwoModules) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + // Open the module, which has its own CrashpadInfo structure. base::FilePath module_path = TestPaths::BuildArtifact(FILE_PATH_LITERAL("snapshot"), @@ -240,6 +244,9 @@ class CrashpadInfoSizes_ClientOptions : public testing::TestWithParam {}; TEST_P(CrashpadInfoSizes_ClientOptions, DifferentlySizedStruct) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + base::FilePath::StringType artifact(FILE_PATH_LITERAL("module_")); artifact += GetParam(); diff --git a/snapshot/elf/elf_image_reader_test.cc b/snapshot/elf/elf_image_reader_test.cc index d64fd5c4..e0f0ea3d 100644 --- a/snapshot/elf/elf_image_reader_test.cc +++ b/snapshot/elf/elf_image_reader_test.cc @@ -21,6 +21,7 @@ #include "base/logging.h" #include "build/build_config.h" #include "gtest/gtest.h" +#include "test/gtest_disabled.h" #include "test/multiprocess_exec.h" #include "test/process_type.h" #include "test/scoped_module_handle.h" @@ -314,6 +315,9 @@ TEST(ElfImageReader, OneModuleChild) { // TODO(scottmg): Separately, the location of the ELF on Android needs some // work, and then the test could also be enabled there. TEST(ElfImageReader, DtHashAndDtGnuHashMatch) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + base::FilePath module_path = TestPaths::BuildArtifact(FILE_PATH_LITERAL("snapshot"), FILE_PATH_LITERAL("both_dt_hash_styles"), diff --git a/snapshot/fuchsia/process_reader_fuchsia_test.cc b/snapshot/fuchsia/process_reader_fuchsia_test.cc index e9d04834..b5997fe6 100644 --- a/snapshot/fuchsia/process_reader_fuchsia_test.cc +++ b/snapshot/fuchsia/process_reader_fuchsia_test.cc @@ -19,7 +19,9 @@ #include #include "gtest/gtest.h" +#include "test/gtest_disabled.h" #include "test/multiprocess_exec.h" +#include "test/test_paths.h" #include "util/fuchsia/scoped_task_suspend.h" namespace crashpad { @@ -159,6 +161,9 @@ class ThreadsChildTest : public MultiprocessExec { }; TEST(ProcessReaderFuchsia, ChildThreads) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + ThreadsChildTest test; test.Run(); } diff --git a/test/BUILD.gn b/test/BUILD.gn index ab37dc16..ec51e7d2 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -99,7 +99,10 @@ static_library("test") { public_configs = [ "..:crashpad_config" ] - configs += [ "../build:crashpad_is_in_chromium" ] + configs += [ + "../build:crashpad_is_in_chromium", + "../build:crashpad_is_in_fuchsia" + ] data = [ "test_paths_test_data_root.txt", diff --git a/test/multiprocess_exec_test.cc b/test/multiprocess_exec_test.cc index 99a1b01f..1f3c2449 100644 --- a/test/multiprocess_exec_test.cc +++ b/test/multiprocess_exec_test.cc @@ -19,6 +19,7 @@ #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "gtest/gtest.h" +#include "test/gtest_disabled.h" #include "test/test_paths.h" #include "util/file/file_io.h" @@ -48,6 +49,9 @@ class TestMultiprocessExec final : public MultiprocessExec { }; TEST(MultiprocessExec, MultiprocessExec) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + TestMultiprocessExec multiprocess_exec; base::FilePath child_test_executable = TestPaths::BuildArtifact( FILE_PATH_LITERAL("test"), diff --git a/test/test_paths.cc b/test/test_paths.cc index 0d7db061..674d04f4 100644 --- a/test/test_paths.cc +++ b/test/test_paths.cc @@ -222,5 +222,14 @@ bool TestPaths::Has32BitBuildArtifacts() { #endif // defined(OS_WIN) && defined(ARCH_CPU_64_BITS) +// static +bool TestPaths::ExternalFilesUnavailable() { +#if defined(OS_FUCHSIA) && defined(CRASHPAD_IS_IN_FUCHSIA) + return true; +#else + return false; +#endif +} + } // namespace test } // namespace crashpad diff --git a/test/test_paths.h b/test/test_paths.h index 5540f77d..df07b5fe 100644 --- a/test/test_paths.h +++ b/test/test_paths.h @@ -138,6 +138,15 @@ class TestPaths { static bool Has32BitBuildArtifacts(); #endif // OS_WIN && ARCH_CPU_64_BITS + //! \return `true` if no external data files are available. + //! + //! TODO(scottmg): https://crashpad.chromium.org/bug/196: This is a temporary + //! hack to disable tests that require external files, either to relaunch + //! subprocesses, or to load data files. The Fuchsia packaging system is in + //! flux (see PKG-46 upstream) and when that's resolved this should be + //! deleted. + static bool ExternalFilesUnavailable(); + DISALLOW_IMPLICIT_CONSTRUCTORS(TestPaths); }; diff --git a/test/test_paths_test.cc b/test/test_paths_test.cc index 9d43b594..cec54543 100644 --- a/test/test_paths_test.cc +++ b/test/test_paths_test.cc @@ -16,6 +16,7 @@ #include "base/files/file_path.h" #include "gtest/gtest.h" +#include "test/gtest_disabled.h" #include "util/file/file_io.h" namespace crashpad { @@ -23,6 +24,9 @@ namespace test { namespace { TEST(TestPaths, TestDataRoot) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + base::FilePath test_data_root = TestPaths::TestDataRoot(); ScopedFileHandle file(LoggingOpenFileForRead( test_data_root.Append(FILE_PATH_LITERAL("test")) diff --git a/util/net/http_body_test.cc b/util/net/http_body_test.cc index e48cbf91..caa53c28 100644 --- a/util/net/http_body_test.cc +++ b/util/net/http_body_test.cc @@ -17,6 +17,7 @@ #include #include "gtest/gtest.h" +#include "test/gtest_disabled.h" #include "test/test_paths.h" #include "util/misc/implicit_cast.h" #include "util/net/http_body_test_util.h" @@ -97,6 +98,9 @@ TEST(StringHTTPBodyStream, MultipleReads) { } TEST(FileReaderHTTPBodyStream, ReadASCIIFile) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + base::FilePath path = TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt")); @@ -117,6 +121,9 @@ TEST(FileReaderHTTPBodyStream, ReadASCIIFile) { } TEST(FileReaderHTTPBodyStream, ReadBinaryFile) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + // HEX contents of file: |FEEDFACE A11A15|. base::FilePath path = TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("util/net/testdata/binary_http_body.dat")); @@ -187,6 +194,9 @@ TEST_P(CompositeHTTPBodyStreamBufferSize, ThreeStringParts) { } TEST_P(CompositeHTTPBodyStreamBufferSize, StringsAndFile) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + std::string string1("Hello! "); std::string string2(" Goodbye :)"); diff --git a/util/net/http_multipart_builder_test.cc b/util/net/http_multipart_builder_test.cc index dab9e7e7..d1bcf544 100644 --- a/util/net/http_multipart_builder_test.cc +++ b/util/net/http_multipart_builder_test.cc @@ -20,6 +20,7 @@ #include "gtest/gtest.h" #include "test/gtest_death.h" +#include "test/gtest_disabled.h" #include "test/test_paths.h" #include "util/net/http_body.h" #include "util/net/http_body_test_util.h" @@ -99,6 +100,9 @@ TEST(HTTPMultipartBuilder, ThreeStringFields) { } TEST(HTTPMultipartBuilder, ThreeFileAttachments) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + HTTPMultipartBuilder builder; base::FilePath ascii_http_body_path = TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt")); @@ -183,6 +187,9 @@ TEST(HTTPMultipartBuilder, OverwriteFormDataWithEscapedKey) { } TEST(HTTPMultipartBuilder, OverwriteFileAttachment) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + HTTPMultipartBuilder builder; static constexpr char kValue[] = "1 2 3 test"; builder.SetFormData("a key", kValue); @@ -241,6 +248,9 @@ TEST(HTTPMultipartBuilder, OverwriteFileAttachment) { } TEST(HTTPMultipartBuilder, SharedFormDataAndAttachmentKeyNamespace) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + HTTPMultipartBuilder builder; static constexpr char kValue1[] = "11111"; builder.SetFormData("one", kValue1); diff --git a/util/net/http_transport_test.cc b/util/net/http_transport_test.cc index abaa1727..f8413338 100644 --- a/util/net/http_transport_test.cc +++ b/util/net/http_transport_test.cc @@ -30,6 +30,7 @@ #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "gtest/gtest.h" +#include "test/gtest_disabled.h" #include "test/multiprocess_exec.h" #include "test/test_paths.h" #include "util/file/file_io.h" @@ -210,6 +211,9 @@ void ValidFormData(HTTPTransportTestFixture* fixture, } TEST(HTTPTransport, ValidFormData) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + HTTPMultipartBuilder builder; builder.SetFormData("key1", "test"); builder.SetFormData("key2", "--abcdefg123"); @@ -223,6 +227,9 @@ TEST(HTTPTransport, ValidFormData) { } TEST(HTTPTransport, ValidFormData_Gzip) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + HTTPMultipartBuilder builder; builder.SetGzipEnabled(true); builder.SetFormData("key1", "test"); @@ -246,6 +253,9 @@ void ErrorResponse(HTTPTransportTestFixture* fixture, } TEST(HTTPTransport, ErrorResponse) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + HTTPMultipartBuilder builder; HTTPHeaders headers; headers[kContentType] = kTextPlain; @@ -274,6 +284,9 @@ void UnchunkedPlainText(HTTPTransportTestFixture* fixture, } TEST(HTTPTransport, UnchunkedPlainText) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + std::unique_ptr body_stream( new StringHTTPBodyStream(kTextBody)); @@ -314,10 +327,16 @@ void RunUpload33k(bool has_content_length) { } TEST(HTTPTransport, Upload33k) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + RunUpload33k(true); } TEST(HTTPTransport, Upload33k_LengthUnknown) { + if (TestPaths::ExternalFilesUnavailable()) + DISABLED_TEST(); + // The same as Upload33k, but without declaring Content-Length ahead of time. RunUpload33k(false); }