Locate test data more robustly.

Test code that requires test data should call Paths::TestDataRoot() to
obtain the test data root. This will use the CRASHPAD_TEST_DATA_ROOT
environment variable if set. Otherwise, it will look for test data at
known locations relative to the executable path. If the test data is not
found in any of these locations, it falls back to using the working
directory, the same as the current behavior.

BUG=crashpad:4
TEST=crashpad_util_test Paths.TestDataRoot and others
R=rsesek@chromium.org, scottmg@chromium.org

Review URL: https://codereview.chromium.org/992503002

build/run_tests.py

@@ -27,13 +27,10 @@ def main(args):
     print >>sys.stderr, 'usage: run_tests.py {Debug|Release}'
     return 1;
 
-  # Until https://code.google.com/p/crashpad/issues/detail?id=4 is fixed, tests
-  # need to be run from a specific working directory.
   crashpad_dir = \
       os.path.join(os.path.dirname(os.path.abspath(__file__)), os.pardir)
-  os.chdir(crashpad_dir)
 
-  binary_dir = os.path.join('out', args[0])
+  binary_dir = os.path.join(crashpad_dir, 'out', args[0])
   tests = [
       'crashpad_client_test',
       'crashpad_minidump_test',

util/net/http_body_test.cc

@@ -16,6 +16,7 @@
 
 #include "gtest/gtest.h"
 #include "util/net/http_body_test_util.h"
+#include "util/test/paths.h"
 
 namespace crashpad {
 namespace test {
@@ -93,9 +94,7 @@ TEST(StringHTTPBodyStream, MultipleReads) {
 }
 
 TEST(FileHTTPBodyStream, ReadASCIIFile) {
-  // TODO(rsesek): Use a more robust mechanism to locate testdata
-  // <https://code.google.com/p/crashpad/issues/detail?id=4>.
-  base::FilePath path = base::FilePath(
+  base::FilePath path = Paths::TestDataRoot().Append(
       FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt"));
   FileHTTPBodyStream stream(path);
   std::string contents = ReadStreamToString(&stream, 32);
@@ -113,9 +112,7 @@ TEST(FileHTTPBodyStream, ReadASCIIFile) {
 
 TEST(FileHTTPBodyStream, ReadBinaryFile) {
   // HEX contents of file: |FEEDFACE A11A15|.
-  // TODO(rsesek): Use a more robust mechanism to locate testdata
-  // <https://code.google.com/p/crashpad/issues/detail?id=4>.
-  base::FilePath path = base::FilePath(
+  base::FilePath path = Paths::TestDataRoot().Append(
       FILE_PATH_LITERAL("util/net/testdata/binary_http_body.dat"));
   // This buffer size was chosen so that reading the file takes multiple reads.
   uint8_t buf[4];
@@ -199,8 +196,9 @@ TEST_P(CompositeHTTPBodyStreamBufferSize, StringsAndFile) {
 
   std::vector<HTTPBodyStream*> parts;
   parts.push_back(new StringHTTPBodyStream(string1));
-  parts.push_back(new FileHTTPBodyStream(base::FilePath(
-      FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt"))));
+  base::FilePath path = Paths::TestDataRoot().Append(
+      FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt"));
+  parts.push_back(new FileHTTPBodyStream(path));
   parts.push_back(new StringHTTPBodyStream(string2));
 
   CompositeHTTPBodyStream stream(parts);

util/net/http_multipart_builder_test.cc

@@ -19,6 +19,7 @@
 #include "gtest/gtest.h"
 #include "util/net/http_body.h"
 #include "util/net/http_body_test_util.h"
+#include "util/test/paths.h"
 
 namespace crashpad {
 namespace test {
@@ -95,22 +96,19 @@ TEST(HTTPMultipartBuilder, ThreeStringFields) {
 
 TEST(HTTPMultipartBuilder, ThreeFileAttachments) {
   HTTPMultipartBuilder builder;
-  // TODO(rsesek): Use a more robust mechanism to locate testdata
-  // <https://code.google.com/p/crashpad/issues/detail?id=4>.
+  base::FilePath ascii_http_body_path = Paths::TestDataRoot().Append(
+      FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt"));
   builder.SetFileAttachment("first",
                             "minidump.dmp",
-                            base::FilePath(FILE_PATH_LITERAL(
-                                "util/net/testdata/ascii_http_body.txt")),
+                            ascii_http_body_path,
                             "");
   builder.SetFileAttachment("second",
                             "minidump.dmp",
-                            base::FilePath(FILE_PATH_LITERAL(
-                                "util/net/testdata/ascii_http_body.txt")),
+                            ascii_http_body_path,
                             "text/plain");
   builder.SetFileAttachment("\"third 50% silly\"",
                             "test%foo.txt",
-                            base::FilePath(FILE_PATH_LITERAL(
-                                "util/net/testdata/ascii_http_body.txt")),
+                            ascii_http_body_path,
                             "text/plain");
 
   const char kFileContents[] = "This is a test.\n";
@@ -182,22 +180,22 @@ TEST(HTTPMultipartBuilder, OverwriteFileAttachment) {
   HTTPMultipartBuilder builder;
   const char kValue[] = "1 2 3 test";
   builder.SetFormData("a key", kValue);
-  // TODO(rsesek): Use a more robust mechanism to locate testdata
-  // <https://code.google.com/p/crashpad/issues/detail?id=4>.
+  base::FilePath testdata_path =
+      Paths::TestDataRoot().Append(FILE_PATH_LITERAL("util/net/testdata"));
   builder.SetFileAttachment("minidump",
                             "minidump.dmp",
-                            base::FilePath(FILE_PATH_LITERAL(
-                                "util/net/testdata/binary_http_body.dat")),
+                            testdata_path.Append(FILE_PATH_LITERAL(
+                                "binary_http_body.dat")),
                             "");
   builder.SetFileAttachment("minidump2",
                             "minidump.dmp",
-                            base::FilePath(FILE_PATH_LITERAL(
-                                "util/net/testdata/binary_http_body.dat")),
+                            testdata_path.Append(FILE_PATH_LITERAL(
+                                "binary_http_body.dat")),
                             "");
   builder.SetFileAttachment("minidump",
                             "minidump.dmp",
-                            base::FilePath(FILE_PATH_LITERAL(
-                                "util/net/testdata/ascii_http_body.txt")),
+                            testdata_path.Append(FILE_PATH_LITERAL(
+                                "ascii_http_body.txt")),
                             "text/plain");
   scoped_ptr<HTTPBodyStream> body(builder.GetBodyStream());
   ASSERT_TRUE(body.get());
@@ -239,10 +237,11 @@ TEST(HTTPMultipartBuilder, SharedFormDataAndAttachmentKeyNamespace) {
   HTTPMultipartBuilder builder;
   const char kValue1[] = "11111";
   builder.SetFormData("one", kValue1);
+  base::FilePath ascii_http_body_path = Paths::TestDataRoot().Append(
+      FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt"));
   builder.SetFileAttachment("minidump",
                             "minidump.dmp",
-                            base::FilePath(FILE_PATH_LITERAL(
-                                "util/net/testdata/ascii_http_body.txt")),
+                            ascii_http_body_path,
                             "");
   const char kValue2[] = "this is not a file";
   builder.SetFormData("minidump", kValue2);

util/net/http_transport_test.cc

@@ -20,11 +20,13 @@
 
 #include <vector>
 
+#include "base/files/file_path.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/rand_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "gtest/gtest.h"
 #include "util/file/file_io.h"
@@ -32,6 +34,7 @@
 #include "util/net/http_headers.h"
 #include "util/net/http_multipart_builder.h"
 #include "util/test/multiprocess_exec.h"
+#include "util/test/paths.h"
 
 namespace crashpad {
 namespace test {
@@ -51,17 +54,17 @@ class HTTPTransportTestFixture : public MultiprocessExec {
         body_stream_(body_stream.Pass()),
         response_code_(http_response_code),
         request_validator_(request_validator) {
-    // TODO(rsesek): Use a more robust mechanism to locate testdata
-    // <https://code.google.com/p/crashpad/issues/detail?id=4>.
+    base::FilePath server_path = Paths::TestDataRoot().Append(
+        FILE_PATH_LITERAL("util/net/http_transport_test_server.py"));
 #if defined(OS_POSIX)
-    SetChildCommand("util/net/http_transport_test_server.py", nullptr);
+    SetChildCommand(server_path.value(), nullptr);
 #elif defined(OS_WIN)
     // Explicitly invoke a shell and python so that python can be found in the
     // path, and run the test script.
     std::vector<std::string> args;
     args.push_back("/c");
     args.push_back("python");
-    args.push_back("util/net/http_transport_test_server.py");
+    args.push_back(base::UTF16ToUTF8(server_path.value()));
     SetChildCommand(getenv("COMSPEC"), &args);
 #endif  // OS_POSIX
   }

util/test/executable_path.h

@@ -1,29 +0,0 @@
-// Copyright 2014 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_UTIL_TEST_EXECUTABLE_PATH_H_
-#define CRASHPAD_UTIL_TEST_EXECUTABLE_PATH_H_
-
-#include "base/files/file_path.h"
-
-namespace crashpad {
-namespace test {
-
-//! \brief Returns the pathname of the currently-running test executable.
-base::FilePath ExecutablePath();
-
-}  // namespace test
-}  // namespace crashpad
-
-#endif  // CRASHPAD_UTIL_TEST_EXECUTABLE_PATH_H_

util/test/multiprocess_exec_test.cc

@@ -19,7 +19,7 @@
 #include "build/build_config.h"
 #include "gtest/gtest.h"
 #include "util/file/file_io.h"
-#include "util/test/executable_path.h"
+#include "util/test/paths.h"
 
 namespace crashpad {
 namespace test {
@@ -48,7 +48,7 @@ class TestMultiprocessExec final : public MultiprocessExec {
 
 TEST(MultiprocessExec, MultiprocessExec) {
   TestMultiprocessExec multiprocess_exec;
-  base::FilePath test_executable = ExecutablePath();
+  base::FilePath test_executable = Paths::Executable();
 #if defined(OS_POSIX)
   std::string child_test_executable = test_executable.value();
 #elif defined(OS_WIN)

util/test/paths.cc

@@ -0,0 +1,101 @@
+// Copyright 2015 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 "util/test/paths.h"
+
+#include <stdlib.h>
+#include <sys/stat.h>
+
+#include "base/logging.h"
+#include "build/build_config.h"
+
+namespace crashpad {
+namespace test {
+
+namespace {
+
+bool IsTestDataRoot(const base::FilePath& candidate) {
+  const base::FilePath marker_path =
+      candidate.Append(FILE_PATH_LITERAL("util"))
+          .Append(FILE_PATH_LITERAL("test"))
+          .Append(FILE_PATH_LITERAL("paths_test_data_root.txt"));
+
+#if !defined(OS_WIN)
+  struct stat stat_buf;
+  int rv = stat(marker_path.value().c_str(), &stat_buf);
+#else
+  struct _stat stat_buf;
+  int rv = _wstat(marker_path.value().c_str(), &stat_buf);
+#endif
+
+  return rv == 0;
+}
+
+base::FilePath TestDataRootInternal() {
+#if !defined(OS_WIN)
+  const char* environment_value = getenv("CRASHPAD_TEST_DATA_ROOT");
+#else  // defined(OS_WIN)
+  const wchar_t* environment_value = _wgetenv(L"CRASHPAD_TEST_DATA_ROOT");
+#endif
+
+  if (environment_value) {
+    // It was specified explicitly, so use it even if it seems incorrect.
+    if (!IsTestDataRoot(base::FilePath(environment_value))) {
+      LOG(WARNING) << "CRASHPAD_TEST_DATA_ROOT seems invalid, honoring anyway";
+    }
+
+    return base::FilePath(environment_value);
+  }
+
+  // In a standalone build, the test executable is usually at
+  // out/{Debug,Release} relative to the Crashpad root.
+  const base::FilePath executable = Paths::Executable();
+  base::FilePath candidate =
+      base::FilePath(executable.DirName()
+                         .Append(base::FilePath::kParentDirectory)
+                         .Append(base::FilePath::kParentDirectory));
+  if (IsTestDataRoot(candidate)) {
+    return candidate;
+  }
+
+  // In an in-Chromium build, the test executable is usually at
+  // out/{Debug,Release} relative to the Chromium root, and the Crashpad root is
+  // at third_party/crashpad/crashpad relative to the Chromium root.
+  candidate = candidate.Append(FILE_PATH_LITERAL("third_party"))
+                  .Append(FILE_PATH_LITERAL("crashpad"))
+                  .Append(FILE_PATH_LITERAL("crashpad"));
+  if (IsTestDataRoot(candidate)) {
+    return candidate;
+  }
+
+  // If nothing else worked, use the current directory, issuing a warning if it
+  // doesn’t seem right.
+  if (!IsTestDataRoot(base::FilePath(base::FilePath::kCurrentDirectory))) {
+    LOG(WARNING) << "could not locate a valid test data root";
+  }
+
+  return base::FilePath(base::FilePath::kCurrentDirectory);
+}
+
+}  // namespace
+
+// static
+base::FilePath Paths::TestDataRoot() {
+  static base::FilePath* test_data_root =
+      new base::FilePath(TestDataRootInternal());
+  return *test_data_root;
+}
+
+}  // namespace test
+}  // namespace crashpad

util/test/paths.h

@@ -0,0 +1,49 @@
+// Copyright 2015 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_UTIL_TEST_PATHS_H_
+#define CRASHPAD_UTIL_TEST_PATHS_H_
+
+#include "base/basictypes.h"
+#include "base/files/file_path.h"
+
+namespace crashpad {
+namespace test {
+
+//! \brief Functions to obtain paths from within tests.
+class Paths {
+ public:
+  //! \brief Returns the pathname of the currently-running test executable.
+  static base::FilePath Executable();
+
+  //! \brief Returns the pathname of the test data root.
+  //!
+  //! If the `CRASHPAD_TEST_DATA_ROOT` environment variable is set, its value
+  //! will be returned. Otherwise, this function will attempt to locate the test
+  //! data root relative to the executable path. If this fails, it will fall
+  //! back to returning the current working directory.
+  //!
+  //! At present, the test data root is normally the root of the Crashpad source
+  //! tree, although this may not be the case indefinitely. This function may
+  //! only be used to locate test data, not for arbitrary access to source
+  //! files.
+  static base::FilePath TestDataRoot();
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(Paths);
+};
+
+}  // namespace test
+}  // namespace crashpad
+
+#endif  // CRASHPAD_UTIL_TEST_PATHS_H_

util/test/paths_mac.cc

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "util/test/executable_path.h"
+#include "util/test/paths.h"
 
 #include <mach-o/dyld.h>
 #include <stdint.h>
@@ -22,7 +22,8 @@
 namespace crashpad {
 namespace test {
 
-base::FilePath ExecutablePath() {
+// static
+base::FilePath Paths::Executable() {
   uint32_t executable_length = 0;
   _NSGetExecutablePath(nullptr, &executable_length);
   DCHECK_GT(executable_length, 1u);

util/test/paths_test.cc

@@ -12,18 +12,19 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "util/test/executable_path.h"
+#include "util/test/paths.h"
 
 #include "base/files/file_path.h"
 #include "build/build_config.h"
 #include "gtest/gtest.h"
+#include "util/file/file_io.h"
 
 namespace crashpad {
 namespace test {
 namespace {
 
-TEST(ExecutablePath, ExecutablePath) {
-  base::FilePath executable_path = ExecutablePath();
+TEST(Paths, Executable) {
+  base::FilePath executable_path = Paths::Executable();
   base::FilePath executable_name = executable_path.BaseName();
 #if defined(OS_WIN)
   EXPECT_EQ(FILE_PATH_LITERAL("crashpad_util_test.exe"),
@@ -33,6 +34,15 @@ TEST(ExecutablePath, ExecutablePath) {
 #endif  // OS_WIN
 }
 
+TEST(Paths, TestDataRoot) {
+  base::FilePath test_data_root = Paths::TestDataRoot();
+  ScopedFileHandle file(LoggingOpenFileForRead(
+      test_data_root.Append(FILE_PATH_LITERAL("util"))
+          .Append(FILE_PATH_LITERAL("test"))
+          .Append(FILE_PATH_LITERAL("paths_test_data_root.txt"))));
+  EXPECT_TRUE(file.is_valid());
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad

util/test/paths_test_data_root.txt

@@ -0,0 +1,16 @@
+# Copyright 2015 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.
+
+This file is used by Paths::TestDataRoot() to locate the test data root. It
+is present at a known path from the test data root.

util/test/paths_win.cc

@@ -12,14 +12,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "util/test/executable_path.h"
+#include "util/test/paths.h"
 
 #include <windows.h>
 
 namespace crashpad {
 namespace test {
 
-base::FilePath ExecutablePath() {
+// static
+base::FilePath Paths::Executable() {
   wchar_t executable_path[_MAX_PATH];
   GetModuleFileName(nullptr, executable_path, sizeof(executable_path));
   return base::FilePath(executable_path);

util/util.gyp

@@ -224,9 +224,6 @@
       'sources': [
         'test/errors.cc',
         'test/errors.h',
-        'test/executable_path.h',
-        'test/executable_path_mac.cc',
-        'test/executable_path_win.cc',
         'test/mac/dyld.h',
         'test/mac/mach_errors.cc',
         'test/mac/mach_errors.h',
@@ -237,6 +234,10 @@
         'test/multiprocess_exec_posix.cc',
         'test/multiprocess_exec_win.cc',
         'test/multiprocess_posix.cc',
+        'test/paths.cc',
+        'test/paths.h',
+        'test/paths_mac.cc',
+        'test/paths_win.cc',
         'test/scoped_temp_dir.cc',
         'test/scoped_temp_dir.h',
         'test/scoped_temp_dir_posix.cc',
@@ -308,10 +309,10 @@
         'stdlib/strlcpy_test.cc',
         'stdlib/strnlen_test.cc',
         'synchronization/semaphore_test.cc',
-        'test/executable_path_test.cc',
         'test/mac/mach_multiprocess_test.cc',
         'test/multiprocess_exec_test.cc',
         'test/multiprocess_posix_test.cc',
+        'test/paths_test.cc',
         'test/scoped_temp_dir_test.cc',
         'win/process_info_test.cc',
         'win/time_test.cc',

util/win/process_info_test.cc

@@ -21,7 +21,7 @@
 #include "build/build_config.h"
 #include "gtest/gtest.h"
 #include "util/misc/uuid.h"
-#include "util/test/executable_path.h"
+#include "util/test/paths.h"
 #include "util/win/scoped_handle.h"
 
 namespace crashpad {
@@ -77,7 +77,7 @@ TEST(ProcessInfo, SomeOtherProcess) {
       CreateEvent(nullptr, true, false, done_uuid.ToString16().c_str()));
   ASSERT_TRUE(done.get());
 
-  base::FilePath test_executable = ExecutablePath();
+  base::FilePath test_executable = Paths::Executable();
   std::wstring child_test_executable =
       test_executable.RemoveFinalExtension().value() +
       L"_process_info_test_child.exe";