Packaged test running seems to be a ways off, but with a bit of path
fiddling in test_paths.cc we can actually use the paths where the tests
are copied, so do that instead to get all the tests re-enabled. The
setup in BUILD.gn should be mostly-useful once packaging is working as
all helper/data files will need to specified there anyway.
Also, attempted fix to flaky behaviour in
ProcessReaderFuchsia.ChildThreads exposed because the tests are now
being run. zx_object_wait_many() waits on *any* of the objects, not
*all* of them. Derp!
And finally, for the same test, work around some unintuitive behaviour
in zx_task_suspend(), in particular that the thread will not be
suspended for the purpose of reading registers right away, but instead
only "sometime later", which appears in pratice to be after the next
context switch. Have ScopedTaskSuspend block for a while to try to
ensure the registers become readble, and if they don't, at least fail
noisily at that point.
Bug: crashpad:196
Change-Id: I01fb3590ede96301c941c2a88eba47fdbfe74ea7
Reviewed-on: https://chromium-review.googlesource.com/1053797
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
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 <scottmg@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Extends MultiprocessExec to support running functions registered via
CRASHPAD_CHILD_TEST_MAIN() as the main of a new child process.
Additionally, implements Fuchsia exit code checking, and adds a
CRASHPAD_CHILD_TEST_MAIN()-based test for that.
Bug: crashpad:196, crashpad:215
Change-Id: I49ce3f4d95a3b9823813e6df5a602cee2583bcf8
Reviewed-on: https://chromium-review.googlesource.com/879563
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
The design for running all Crashpad unit tests on Chromium’s try- and
buildbots involves pulling all tests into a single monolithic
crashpad_tests executable. Many Crashpad tests base the name of their
child executables or modules on the name of the main test executable.
Since the main test executable will have a different name in the
in-Chromium build, knowledge of the test executable name (referred to as
“module” here) needs to be added to the tests themselves.
This introduces TestPaths::BuildArtifact(), which allows the module name
to be specified. For Crashpad’s standalone build, the module name is
verified against the main test executable’s name.
TestPaths::BuildArtifact() can also locate paths in the alternate 32-bit
output directory for 64-bit Windows tests, taking on the responsibility
for what the new (5e9ed4cb9f69) TestPaths::Output32BitDirectory(), now
obsolete, did.
Bug: chromium:779790
Change-Id: I64c4a2190b6319e487c999812a7cfc512a75a700
Reviewed-on: https://chromium-review.googlesource.com/747536
Reviewed-by: Scott Graham <scottmg@chromium.org>
gtest used to require (expected, actual) ordering for arguments to
EXPECT_EQ and ASSERT_EQ, and in failed test assertions would identify
each side as “expected” or “actual.” Tests in Crashpad adhered to this
traditional ordering. After a gtest change in February 2016, it is now
agnostic with respect to the order of these arguments.
This change mechanically updates all uses of these macros to (actual,
expected) by reversing them. This provides consistency with our use of
the logging CHECK_EQ and DCHECK_EQ macros, and makes for better
readability by ordinary native speakers. The rough (but working!)
conversion tool is
https://chromium-review.googlesource.com/c/466727/1/rewrite_expectassert_eq.py,
and “git cl format” cleaned up its output.
EXPECT_NE and ASSERT_NE never had a preferred ordering. gtest never made
a judgment that one side or the other needed to provide an “unexpected”
value. Consequently, some code used (unexpected, actual) while other
code used (actual, unexpected). For consistency with the new EXPECT_EQ
and ASSERT_EQ usage, as well as consistency with CHECK_NE and DCHECK_NE,
this change also updates these use sites to (actual, unexpected) where
one side can be called “unexpected” as, for example, std::string::npos
can be. Unfortunately, this portion was a manual conversion.
References:
https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#binary-comparison77d6b17338https://github.com/google/googletest/pull/713
Change-Id: I978fef7c94183b8b1ef63f12f5ab4d6693626be3
Reviewed-on: https://chromium-review.googlesource.com/466727
Reviewed-by: Scott Graham <scottmg@chromium.org>
This supports the “double handler” or “double handler with low
probability” models from https://crashpad.chromium.org/bug/143.
For crashpad_handler to be become its own client, it needs access to its
own executable path to pass to CrashpadClient::StartHandler(). This was
formerly available in the test-only test::Paths::Executable(). Bring
that function’s implementation to the non-test Paths::Executable() in
util/misc, and rename test::Paths to test::TestPaths to avoid future
confusion.
test::TestPaths must still be used to access TestDataRoot(), which does
not make any sense to non-test code.
test::TestPaths::Executable() is retained for use by tests, which most
likely prefer the fatal semantics of that function. Paths::Executable()
is not fatal because for the purposes of implementing the double
handler, a failure to locate the executable path (which may happen on
some systems in deeply-nested directory hierarchies) shouldn’t cause the
initial crashpad_handler to abort, even if it does prevent a second
crashpad_handler from being started.
Bug: crashpad:143
Test: crashpad_util_test Paths.*, crashpad_test_test TestPaths.*
Change-Id: I9f75bf61839ce51e33c9f7c0d7031cebead6a156
Reviewed-on: https://chromium-review.googlesource.com/466346
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
ReadFile() attempted to continue reading after a short read. In most
cases, this is fine. However, ReadFile() would keep trying to fill a
partially-filled buffer until experiencing a 0-length read(), signaling
end-of-file. For certain weird file descriptors like terminal input, EOF
is an ephemeral condition, and attempting to read beyond EOF doesn’t
actually return 0 (EOF) provided that they remain open, it will block
waiting for more input. Consequently, ReadFile() and anything based on
ReadFile() had an undocumented and quirky interface, which was that any
short read that it returned (not an underlying short read) actually
indicated EOF.
This facet of ReadFile() was unexpected, so it’s being removed. The new
behavior is that ReadFile() will return an underlying short read. The
behavior of FileReaderInterface::Read() is updated in accordance with
this change.
Upon experiencing a short read, the caller can determine the best
action. Most callers were already prepared for this behavior. Outside of
util/file, only crashpad_database_util properly implemented EOF
detection according to previous semantics, and adapting it to new
semantics is trivial.
Callers who require an exact-length read can use the new
ReadFileExactly(), or the newly renamed LoggingReadFileExactly() or
CheckedReadFileExactly(). These functions will retry following a short
read. The renamed functions were previously called LoggingReadFile() and
CheckedReadFile(), but those names implied that they were simply
wrapping ReadFile(), which is not the case. They wrapped ReadFile() and
further, insisted on a full read. Since ReadFile()’s semantics are now
changing but these functions’ are not, they’re now even more distinct
from ReadFile(), and must be renamed to avoid confusion.
Test: *
Change-Id: I06b77e0d6ad8719bd2eb67dab93a8740542dd908
Reviewed-on: https://chromium-review.googlesource.com/456676
Reviewed-by: Robert Sesek <rsesek@chromium.org>
This was done in Chromium’s local copy of Crashpad in 562827afb599. This
change is similar to that one, except more care was taken to avoid
including headers from a .cc or _test.cc when already included by the
associated .h. Rather than using <stddef.h> for size_t, Crashpad has
always used <sys/types.h>, so that’s used here as well.
This updates mini_chromium to 8a2363f486e3a0dc562a68884832d06d28d38dcc,
which removes base/basictypes.h.
e128dcf10122 Remove base/move.h; use std::move() instead of Pass()
8a2363f486e3 Move basictypes.h to macros.h
R=avi@chromium.org
Review URL: https://codereview.chromium.org/1566713002 .
After 9e79ea1da719, it no longer makes sense for crashpad_util_test_lib
to “hide” in util/util_test.gyp. All of util/test is moved to its own
top-level directory, test, which all other test code is allowed to
depend on. test, too, is allowed to depend on all other non-test code.
In a future change, when crashpad_util_test_lib gains a dependency on
crashpad_client, it won’t look so weird for something in util (even
though it’s in util/test) to depend on something in client, because the
thing that needs to depend on client will live in test, not util.
BUG=crashpad:33
R=scottmg@chromium.org
Review URL: https://codereview.chromium.org/1051533002
