Bug 33041 - [bzt] Speed up CredentialsTest.test_read_credentials_with_SVN to make run-webkit-unittest run faster
Summary: [bzt] Speed up CredentialsTest.test_read_credentials_with_SVN to make run-web...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-29 21:49 PST by Daniel Bates
Modified: 2009-12-29 22:30 PST (History)
3 users (show)

See Also:


Attachments
Patch (2.55 KB, patch)
2009-12-29 21:51 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (2.75 KB, patch)
2009-12-29 22:14 PST, Daniel Bates
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2009-12-29 21:49:28 PST
Following up on performance concerns raised by Adam and Eric on IRC about the unit test included in the patch for bug #32896.

Currently the test method CredentialsTest.test_read_credentials_with_SVN calls SVNTestRepository.setup and SVNTestRepository.tear_down. But these calls are expensive as they perform system calls to create an actual SVN repository and perform some test commits. Instead, it is sufficient to create a temporary directory that does not contain a Git repository.
Comment 1 Daniel Bates 2009-12-29 21:51:23 PST
Created attachment 45638 [details]
Patch
Comment 2 Eric Seidel (no email) 2009-12-29 21:53:38 PST
Comment on attachment 45638 [details]
Patch

Why are we keeping around the disabled path?
Comment 3 WebKit Review Bot 2009-12-29 21:54:18 PST
style-queue ran check-webkit-style on attachment 45638 [details] without any errors.
Comment 4 Daniel Bates 2009-12-29 22:00:12 PST
Comment on attachment 45638 [details]
Patch

On second thought, we don't need the optional parameter shouldMakeActualSVNRepo and related code as it does add much if any value. Original thought was to make the unit test match the actual environment as close as possible. But it is sufficient to just create a temporary directory (one in which we know does not contain a not Git repo).
Comment 5 Daniel Bates 2009-12-29 22:14:59 PST
Created attachment 45639 [details]
Patch

Removed unnecessary code that made use of modules.scm_unittest.SVNTestRepository.

Also, renamed method test_read_credentials_with_SVN to test_read_credentials_without_Git_repo to better reflect what it is testing.
Comment 6 Eric Seidel (no email) 2009-12-29 22:16:24 PST
Comment on attachment 45639 [details]
Patch

LGTM.
Comment 7 Eric Seidel (no email) 2009-12-29 22:16:55 PST
Comment on attachment 45639 [details]
Patch

test_read_credentials_without_Git_repo
capitalization is a bit strange. I would have just called it "git"
Comment 8 Daniel Bates 2009-12-29 22:23:53 PST
(In reply to comment #7)
> (From update of attachment 45639 [details])
> test_read_credentials_without_Git_repo
> capitalization is a bit strange. I would have just called it "git"

I'll fix this before I land.
Comment 9 Daniel Bates 2009-12-29 22:30:55 PST
Committed in <http://trac.webkit.org/changeset/52644>.