Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(410)

Issue 240001: Add unit tests for OS X MinidumpGenerator implementation (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 9 months ago by Ted Mielczarek
Modified:
13 years, 9 months ago
Reviewers:
Mark Mentovai
CC:
google-breakpad-dev_googlegroups.com
Visibility:
Public.

Description

Add some unit tests for the OS X implementation of MinidumpGenerator. This is
mostly a foundation so I could easily write tests for another patch.

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
M src/client/mac/Breakpad.xcodeproj/project.pbxproj View 12 chunks +36 lines, -5 lines 0 comments Download
R src/client/mac/handler/minidump_generator_test.cc View 1 chunk +0 lines, -81 lines 0 comments Download
M src/client/mac/tests/exception_handler_test.cc View 2 chunks +4 lines, -4 lines 2 comments Download
A src/client/mac/tests/minidump_generator_test.cc View 1 chunk +251 lines, -0 lines 2 comments Download

Messages

Total messages: 3
Ted Mielczarek
13 years, 9 months ago #1
Mark Mentovai
http://breakpad.appspot.com/240001/diff/1/4 File src/client/mac/tests/exception_handler_test.cc (right): http://breakpad.appspot.com/240001/diff/1/4#newcode152 Line 152: ASSERT_EQ(0, pipe(fds)); Why did you flip the FDs? ...
13 years, 9 months ago #2
Ted Mielczarek
13 years, 9 months ago #3
http://breakpad.appspot.com/240001/diff/1/4
File src/client/mac/tests/exception_handler_test.cc (right):

http://breakpad.appspot.com/240001/diff/1/4#newcode152
Line 152: ASSERT_EQ(0, pipe(fds));
On 2010/12/08 20:59:03, Mark Mentovai wrote:
> Why did you flip the FDs?

Turns out this code never actually worked, the child would get EBADF on the
read() and exit immediately, and the only reason the test would pass is because
the parent pretty reliably won the race. The POSIX spec on pipe says:
"Data can be written to the file descriptor fildes[1] and read from the file
descriptor fildes[0]. A read on the file descriptor fildes[0] shall access data
written to the file descriptor fildes[1] on a first-in-first-out basis. It is
unspecified whether fildes[0] is also open for writing and whether fildes[1] is
also open for reading."
http://www.opengroup.org/onlinepubs/009695399/functions/pipe.html

I guess I either got this wrong or Linux is more permissive and I copied it from
some Linux test code. When I copied this to the MinidumpGenerator tests they
were consistently failing until I figured this out, so I fixed it in both
places.

http://breakpad.appspot.com/240001/diff/1/5
File src/client/mac/tests/minidump_generator_test.cc (right):

http://breakpad.appspot.com/240001/diff/1/5#newcode1
Line 1: // Copyright (c) 2010, Google Inc.
On 2010/12/08 20:59:03, Mark Mentovai wrote:
> Is this a straight copy of the other file? Why doesn’t it show up as one?

No, I rm'ed the old and added this as a new file, since they're not actually
related. I could redo it as a rename if you want, but I don't think preserving
the history really adds anything.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1004:630ec63f810e-tainted