Bug 36629

Summary:

Move commit_message_for_this_commit from scm to checkout

Product:

WebKit

Reporter:

Adam Barth <abarth>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, eric

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

OS X 10.5

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch for landing	none

Description Adam Barth 2010-03-25 17:12:04 PDT

Move commit_message_for_this_commit from scm to checkout

Comment 1 Adam Barth 2010-03-25 17:15:08 PDT

Created attachment 51695 [details]
Patch

Comment 2 Eric Seidel (no email) 2010-03-25 17:16:44 PDT

Comment on attachment 51695 [details]
Patch

Do you mean to move the FIXME?  Did you need to delete a mock after this?

Comment 3 Adam Barth 2010-03-25 17:28:48 PDT

Comment on attachment 51695 [details]
Patch

Yeah, I need to remove the FIXME.  There's no mock to remove because our SCM mock inherits from Mock and is really loose.

Comment 4 WebKit Commit Bot 2010-03-25 20:04:28 PDT

Comment on attachment 51695 [details]
Patch

Clearing flags on attachment: 51695

Committed r56592: <http://trac.webkit.org/changeset/56592>

Comment 5 WebKit Commit Bot 2010-03-25 20:04:33 PDT

All reviewed patches have been landed.  Closing bug.

Comment 6 Eric Seidel (no email) 2010-03-25 21:49:00 PDT

Committed r56599: <http://trac.webkit.org/changeset/56599>

Comment 7 Eric Seidel (no email) 2010-03-25 23:08:53 PDT

Created attachment 51715 [details]
Patch

Comment 8 Eric Seidel (no email) 2010-03-25 23:09:15 PDT

I should have opened a new bug, but oh well.

Comment 9 Adam Barth 2010-03-25 23:11:48 PDT

Comment on attachment 51715 [details]
Patch

Doesn't this test leave junk files in the file system?  We shouldn't be writing to the actual file system.  We should mock out that part too.

Comment 10 Eric Seidel (no email) 2010-03-25 23:13:05 PDT

I had it different originally.  It was using a temp svn checkout.  Then I realized I didn't need the checkout.  I'll fix.

Comment 11 Eric Seidel (no email) 2010-03-26 00:15:50 PDT

Created attachment 51717 [details]
Patch

Comment 12 Eric Seidel (no email) 2010-03-26 00:18:15 PDT

We could split this method up into multiple smaller methods to allow easier mocking.  But for now I've just used a temporary directory for holding these ChagneLogs.

If we're adding more API tests which don't want the setUp/tearDown methods, then we should consider making this its own Test class, or using try/finally to make sure the setup/teardown is always run.

Comment 13 Adam Barth 2010-03-26 12:28:58 PDT

Comment on attachment 51717 [details]
Patch

I don't think we should be writing to the file system in a unit test.

Comment 14 Eric Seidel (no email) 2010-03-26 12:46:16 PDT

Argued out in person.

Comment 15 Eric Seidel (no email) 2010-03-26 12:48:15 PDT

Committed r56638: <http://trac.webkit.org/changeset/56638>

Comment 16 Eric Seidel (no email) 2010-03-26 13:25:53 PDT

Created attachment 51775 [details]
Patch for landing

Comment 17 Eric Seidel (no email) 2010-03-26 13:26:21 PDT

Re-open to let the commit-queue see this bug.

Comment 18 Eric Seidel (no email) 2010-03-26 14:14:17 PDT

Comment on attachment 51775 [details]
Patch for landing

Hacking the commit-queue to land this first.

Comment 19 Eric Seidel (no email) 2010-03-26 14:48:18 PDT

Comment on attachment 51775 [details]
Patch for landing

Make our tools work!

Comment 20 Eric Seidel (no email) 2010-03-26 14:51:38 PDT

Comment on attachment 51775 [details]
Patch for landing

Clearing flags on attachment: 51775

Committed r56643: <http://trac.webkit.org/changeset/56643>

Comment 21 Eric Seidel (no email) 2010-03-26 14:51:44 PDT

All reviewed patches have been landed.  Closing bug.