WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
33041
[bzt] Speed up CredentialsTest.test_read_credentials_with_SVN to make run-webkit-unittest run faster
https://bugs.webkit.org/show_bug.cgi?id=33041
Summary
[bzt] Speed up CredentialsTest.test_read_credentials_with_SVN to make run-web...
Daniel Bates
Reported
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.
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2009-12-29 21:51:23 PST
Created
attachment 45638
[details]
Patch
Eric Seidel (no email)
Comment 2
2009-12-29 21:53:38 PST
Comment on
attachment 45638
[details]
Patch Why are we keeping around the disabled path?
WebKit Review Bot
Comment 3
2009-12-29 21:54:18 PST
style-queue ran check-webkit-style on
attachment 45638
[details]
without any errors.
Daniel Bates
Comment 4
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).
Daniel Bates
Comment 5
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.
Eric Seidel (no email)
Comment 6
2009-12-29 22:16:24 PST
Comment on
attachment 45639
[details]
Patch LGTM.
Eric Seidel (no email)
Comment 7
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"
Daniel Bates
Comment 8
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.
Daniel Bates
Comment 9
2009-12-29 22:30:55 PST
Committed in <
http://trac.webkit.org/changeset/52644
>.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug