Issue 3307016: Change scm.SVN.Capture

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 3307016: Change scm.SVN.Capture (Closed)

Created:
10 years, 3 months ago by M-A Ruel

Modified:
9 years, 7 months ago

Reviewers:
bradnelson, bradn

CC:
chromium-reviews, M-A Ruel

Visibility:
Public.

Description

Change scm.SVN.Capture to redirect stderr and throw an exception on call failure. Added try/except to the places where errors are tolerated. Renamed scm.SVN.CaptureBaseRevision to CaptureRevision and removed CaptureHeadRevision. BUG=54084 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=58932

Patch Set 1 #

Patch Set 2 : fix svn unit tests #

Patch Set 3 : fix more tests #

Patch Set 4 : . #

Patch Set 5 : patchset 4 was wrong #

Total comments: 2

Created: 10 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+113 lines, -115 lines)			Patch
M	gclient_scm.py	View	1	3 chunks	+14 lines, -10 lines	0 comments	Download
M	scm.py	View	1 2 4	10 chunks	+34 lines, -56 lines	0 comments	Download
M	tests/gclient_scm_test.py	View	1 4	24 chunks	+51 lines, -24 lines	2 comments	Download
M	tests/scm_unittest.py	View	2 4	8 chunks	+14 lines, -25 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

bradn

LGTM http://codereview.chromium.org/3307016/diff/10002/13003 File tests/gclient_scm_test.py (right): http://codereview.chromium.org/3307016/diff/10002/13003#newcode495 tests/gclient_scm_test.py:495: scm.update(options, self.args, file_list) I suppose you could add ...

10 years, 3 months ago (2010-09-09 05:13:38 UTC) #2

M-A Ruel

10 years, 3 months ago (2010-09-09 13:24:15 UTC) #3

http://codereview.chromium.org/3307016/diff/10002/13003
File tests/gclient_scm_test.py (right):

http://codereview.chromium.org/3307016/diff/10002/13003#newcode495
tests/gclient_scm_test.py:495: scm.update(options, self.args, file_list)
On 2010/09/09 05:13:38, bradn wrote:
> I suppose you could add self.expected_stdout defaulting to '' and check it in
> teardown? Nah, I like pedantic for tests.

scm.update() is the actual function being tested, I'm not sure how I could hack
it to add a parameter, I'd prefer to test scm functions as-is.

Note that in tearDown() at line 55, the test fails if self.checkstdout() was not
called during the test. Is that pedantic enough? :)

Expand Messages | Collapse Messages