diff --git a/gclient_scm.py b/gclient_scm.py index 3c39729e2..7b8e238e4 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -742,12 +742,12 @@ class SVNWrapper(SCMWrapper): return # Get the existing scm url and the revision number of the current checkout. - from_info = scm.SVN.CaptureInfo(os.path.join(self.checkout_path, '.'), '.') - if not from_info: - raise gclient_utils.Error(('Can\'t update/checkout %r if an unversioned ' - 'directory is present. Delete the directory ' - 'and try again.') % - self.checkout_path) + try: + from_info = scm.SVN.CaptureInfo(os.path.join(self.checkout_path, '.')) + except gclient_utils.Error: + raise gclient_utils.Error( + ('Can\'t update/checkout %s if an unversioned directory is present. ' + 'Delete the directory and try again.') % self.checkout_path) # Look for locked directories. dir_info = scm.SVN.CaptureStatus(os.path.join(self.checkout_path, '.')) @@ -758,14 +758,15 @@ class SVNWrapper(SCMWrapper): # Retrieve the current HEAD version because svn is slow at null updates. if options.manually_grab_svn_rev and not revision: - from_info_live = scm.SVN.CaptureInfo(from_info['URL'], '.') + from_info_live = scm.SVN.CaptureInfo(from_info['URL']) revision = str(from_info_live['Revision']) rev_str = ' at %s' % revision if from_info['URL'] != base_url: # The repository url changed, need to switch. - to_info = scm.SVN.CaptureInfo(url, '.') - if not to_info.get('Repository Root') or not to_info.get('UUID'): + try: + to_info = scm.SVN.CaptureInfo(url) + except gclient_utils.Error: # The url is invalid or the server is not accessible, it's safer to bail # out right now. raise gclient_utils.Error('This url is unreachable: %s' % url) @@ -919,7 +920,10 @@ class SVNWrapper(SCMWrapper): def revinfo(self, options, args, file_list): """Display revision""" - return scm.SVN.CaptureBaseRevision(self.checkout_path) + try: + return scm.SVN.CaptureRevision(self.checkout_path) + except gclient_utils.Error: + return None def runhooks(self, options, args, file_list): self.status(options, args, file_list) diff --git a/scm.py b/scm.py index 9cbcc35d4..fadf7f3be 100644 --- a/scm.py +++ b/scm.py @@ -306,21 +306,12 @@ class SVN(object): current_version = None @staticmethod - def Capture(args, in_directory=None, print_error=True): - """Runs svn, capturing output sent to stdout as a string. + def Capture(args, **kwargs): + """Always redirect stderr. - Args: - args: A sequence of command line parameters to be passed to svn. - in_directory: The directory where svn is to be run. - - Returns: - The output sent to stdout as a string. - """ - stderr = None - if not print_error: - stderr = subprocess.PIPE - return gclient_utils.Popen(['svn'] + args, cwd=in_directory, - stdout=subprocess.PIPE, stderr=stderr).communicate()[0] + Throws an exception if non-0 is returned.""" + return gclient_utils.CheckCall(['svn'] + args, print_error=False, + **kwargs)[0] @staticmethod def RunAndGetFileList(verbose, args, cwd, file_list, stdout=None): @@ -425,15 +416,11 @@ class SVN(object): break @staticmethod - def CaptureInfo(relpath, in_directory=None, print_error=True): + def CaptureInfo(cwd): """Returns a dictionary from the svn info output for the given file. - Args: - relpath: The directory where the working copy resides relative to - the directory given by in_directory. - in_directory: The directory where svn is to be run. - """ - output = SVN.Capture(["info", "--xml", relpath], in_directory, print_error) + Throws an exception if svn info fails.""" + output = SVN.Capture(['info', '--xml', cwd]) dom = gclient_utils.ParseXML(output) result = {} if dom: @@ -468,24 +455,13 @@ class SVN(object): return result @staticmethod - def CaptureHeadRevision(url): - """Get the head revision of a SVN repository. - - Returns: - Int head revision - """ - info = SVN.Capture(["info", "--xml", url], os.getcwd()) - dom = xml.dom.minidom.parseString(info) - return dom.getElementsByTagName('entry')[0].getAttribute('revision') - - @staticmethod - def CaptureBaseRevision(cwd): + def CaptureRevision(cwd): """Get the base revision of a SVN repository. Returns: Int base revision """ - info = SVN.Capture(["info", "--xml"], cwd) + info = SVN.Capture(['info', '--xml'], cwd=cwd) dom = xml.dom.minidom.parseString(info) return dom.getElementsByTagName('entry')[0].getAttribute('revision') @@ -539,8 +515,9 @@ class SVN(object): if xml_item_status in status_letter: statuses[0] = status_letter[xml_item_status] else: - raise Exception('Unknown item status "%s"; please implement me!' % - xml_item_status) + raise gclient_utils.Error( + 'Unknown item status "%s"; please implement me!' % + xml_item_status) # Col 1 xml_props_status = wc_status[0].getAttribute('props') if xml_props_status == 'modified': @@ -551,8 +528,9 @@ class SVN(object): xml_props_status == 'normal'): pass else: - raise Exception('Unknown props status "%s"; please implement me!' % - xml_props_status) + raise gclient_utils.Error( + 'Unknown props status "%s"; please implement me!' % + xml_props_status) # Col 2 if wc_status[0].getAttribute('wc-locked') == 'true': statuses[2] = 'L' @@ -592,12 +570,10 @@ class SVN(object): is not set on the file. If the file is not under version control, the empty string is also returned. """ - output = SVN.Capture(["propget", property_name, filename]) - if (output.startswith("svn: ") and - output.endswith("is not under version control")): - return "" - else: - return output + try: + return SVN.Capture(['propget', property_name, filename]) + except gclient_utils.Error: + return '' @staticmethod def DiffItem(filename, full_move=False, revision=None): @@ -662,7 +638,7 @@ class SVN(object): else: if info.get("Node Kind") != "directory": # svn diff on a mv/cp'd file outputs nothing if there was no change. - data = SVN.Capture(command, None) + data = SVN.Capture(command) if not data: # We put in an empty Index entry so upload.py knows about them. data = "Index: %s\n" % filename.replace(os.sep, '/') @@ -670,7 +646,7 @@ class SVN(object): else: if info.get("Node Kind") != "directory": # Normal simple case. - data = SVN.Capture(command, None) + data = SVN.Capture(command) # Otherwise silently ignore directories. return data @@ -761,14 +737,15 @@ class SVN(object): @staticmethod def GetEmail(repo_root): """Retrieves the svn account which we assume is an email address.""" - infos = SVN.CaptureInfo(repo_root) - uuid = infos.get('UUID') - root = infos.get('Repository Root') - if not root: + try: + infos = SVN.CaptureInfo(repo_root) + except gclient_utils.Error: return None # Should check for uuid but it is incorrectly saved for https creds. + root = infos['Repository Root'] realm = root.rsplit('/', 1)[0] + uuid = infos['UUID'] if root.startswith('https') or not uuid: regexp = re.compile(r'<%s:\d+>.*' % realm) else: @@ -820,15 +797,16 @@ class SVN(object): The directory is returned as an absolute path. """ directory = os.path.abspath(directory) - infos = SVN.CaptureInfo(directory, print_error=False) - cur_dir_repo_root = infos.get("Repository Root") - if not cur_dir_repo_root: + try: + cur_dir_repo_root = SVN.CaptureInfo(directory)['Repository Root'] + except gclient_utils.Error: return None - while True: parent = os.path.dirname(directory) - if (SVN.CaptureInfo(parent, print_error=False).get( - "Repository Root") != cur_dir_repo_root): + try: + if SVN.CaptureInfo(parent)['Repository Root'] != cur_dir_repo_root: + break + except gclient_utils.Error: break directory = parent return GetCasedPath(directory) diff --git a/tests/gclient_scm_test.py b/tests/gclient_scm_test.py index 4e90a21e0..3267745bb 100755 --- a/tests/gclient_scm_test.py +++ b/tests/gclient_scm_test.py @@ -8,6 +8,7 @@ # Import before super_mox to keep valid references. from os import rename from shutil import rmtree +import StringIO from subprocess import Popen, PIPE, STDOUT import tempfile import __builtin__ @@ -46,6 +47,20 @@ class BaseTestCase(GCBaseTestCase): self._scm_wrapper = gclient_scm.CreateSCM gclient_scm.sys.stdout.flush = lambda: None gclient_scm.scm.SVN.current_version = None + self.stdout = StringIO.StringIO() + + def tearDown(self): + GCBaseTestCase.tearDown(self) + try: + self.stdout.getvalue() + self.fail() + except AttributeError: + pass + + def checkstdout(self, expected): + value = self.stdout.getvalue() + self.stdout.close() + self.assertEquals(expected, value) class SVNWrapperTestCase(BaseTestCase): @@ -58,7 +73,7 @@ class SVNWrapperTestCase(BaseTestCase): self.force = False self.reset = False self.nohooks = False - self.stdout = gclient_scm.sys.stdout + self.stdout = test_case.stdout def Options(self, *args, **kwargs): return self.OptionsObject(self, *args, **kwargs) @@ -80,11 +95,13 @@ class SVNWrapperTestCase(BaseTestCase): # If you add a member, be sure to add the relevant test! self.compareMembers(self._scm_wrapper('svn://a'), members) + self.checkstdout('') def testUnsupportedSCM(self): args = ['gopher://foo', self.root_dir, self.relpath] exception_msg = 'No SCM found for url gopher://foo' self.assertRaisesError(exception_msg, self._scm_wrapper, *args) + self.checkstdout('') def testSVNFullUrlForRelativeUrl(self): self.url = 'svn://a/b/c/d' @@ -93,6 +110,7 @@ class SVNWrapperTestCase(BaseTestCase): scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) self.assertEqual(scm.FullUrlForRelativeUrl('/crap'), 'svn://a/b/crap') + self.checkstdout('') def testGITFullUrlForRelativeUrl(self): self.url = 'git://a/b/c/d' @@ -101,6 +119,7 @@ class SVNWrapperTestCase(BaseTestCase): scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) self.assertEqual(scm.FullUrlForRelativeUrl('/crap'), 'git://a/b/c/crap') + self.checkstdout('') def testRunCommandException(self): options = self.Options(verbose=False) @@ -113,10 +132,11 @@ class SVNWrapperTestCase(BaseTestCase): exception = "Unsupported argument(s): %s" % ','.join(self.args) self.assertRaisesError(exception, scm.RunCommand, 'update', options, self.args) + self.checkstdout('') def testRunCommandUnknown(self): # TODO(maruel): if ever used. - pass + self.checkstdout('') def testRevertMissing(self): options = self.Options(verbose=True) @@ -127,7 +147,6 @@ class SVNWrapperTestCase(BaseTestCase): # It'll to a checkout instead. gclient_scm.os.path.exists(gclient_scm.os.path.join(base_path, '.git') ).AndReturn(False) - print("\n_____ %s is missing, synching instead" % self.relpath) # Checkout. gclient_scm.os.path.exists(base_path).AndReturn(False) files_list = self.mox.CreateMockAnything() @@ -142,6 +161,8 @@ class SVNWrapperTestCase(BaseTestCase): scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) scm.revert(options, self.args, files_list) + self.checkstdout( + ('\n_____ %s is missing, synching instead\n' % self.relpath)) def testRevertNone(self): options = self.Options(verbose=True) @@ -159,6 +180,7 @@ class SVNWrapperTestCase(BaseTestCase): relpath=self.relpath) file_list = [] scm.revert(options, self.args, file_list) + self.checkstdout('') def testRevert2Files(self): options = self.Options(verbose=True) @@ -182,14 +204,15 @@ class SVNWrapperTestCase(BaseTestCase): cwd=base_path, file_list=mox.IgnoreArg(), stdout=options.stdout) - print(gclient_scm.os.path.join(base_path, 'a')) - print(gclient_scm.os.path.join(base_path, 'b')) self.mox.ReplayAll() scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) file_list = [] scm.revert(options, self.args, file_list) + self.checkstdout( + ('%s\n%s\n' % (gclient_scm.os.path.join(base_path, 'a'), + gclient_scm.os.path.join(base_path, 'b')))) def testRevertDirectory(self): options = self.Options(verbose=True) @@ -200,7 +223,6 @@ class SVNWrapperTestCase(BaseTestCase): ] gclient_scm.scm.SVN.CaptureStatus(base_path).AndReturn(items) file_path = gclient_scm.os.path.join(base_path, 'a') - print(file_path) gclient_scm.os.path.exists(file_path).AndReturn(True) gclient_scm.os.path.isfile(file_path).AndReturn(False) gclient_scm.os.path.islink(file_path).AndReturn(False) @@ -218,6 +240,7 @@ class SVNWrapperTestCase(BaseTestCase): relpath=self.relpath) file_list2 = [] scm.revert(options, self.args, file_list2) + self.checkstdout(('%s\n' % file_path)) def testStatus(self): options = self.Options(verbose=True) @@ -233,7 +256,7 @@ class SVNWrapperTestCase(BaseTestCase): relpath=self.relpath) file_list = [] self.assertEqual(scm.status(options, self.args, file_list), None) - + self.checkstdout('') # TODO(maruel): TEST REVISIONS!!! # TODO(maruel): TEST RELOCATE!!! @@ -262,6 +285,7 @@ class SVNWrapperTestCase(BaseTestCase): scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) scm.update(options, (), files_list) + self.checkstdout('') def testUpdateUpdate(self): options = self.Options(verbose=True) @@ -284,10 +308,9 @@ class SVNWrapperTestCase(BaseTestCase): # Checkout or update. gclient_scm.os.path.exists(base_path).AndReturn(True) gclient_scm.scm.SVN.CaptureInfo( - gclient_scm.os.path.join(base_path, "."), '.' - ).AndReturn(file_info) + gclient_scm.os.path.join(base_path, ".")).AndReturn(file_info) # Cheat a bit here. - gclient_scm.scm.SVN.CaptureInfo(file_info['URL'], '.').AndReturn(file_info) + gclient_scm.scm.SVN.CaptureInfo(file_info['URL']).AndReturn(file_info) additional_args = [] if options.manually_grab_svn_rev: additional_args = ['--revision', str(file_info['Revision'])] @@ -304,6 +327,7 @@ class SVNWrapperTestCase(BaseTestCase): scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) scm.update(options, (), files_list) + self.checkstdout('') def testUpdateSingleCheckout(self): options = self.Options(verbose=True) @@ -339,9 +363,8 @@ class SVNWrapperTestCase(BaseTestCase): ).AndReturn(False) gclient_scm.os.path.exists(base_path).AndReturn(True) gclient_scm.scm.SVN.CaptureInfo( - gclient_scm.os.path.join(base_path, "."), '.' - ).AndReturn(file_info) - gclient_scm.scm.SVN.CaptureInfo(file_info['URL'], '.').AndReturn(file_info) + gclient_scm.os.path.join(base_path, ".")).AndReturn(file_info) + gclient_scm.scm.SVN.CaptureInfo(file_info['URL']).AndReturn(file_info) options.stdout.write("\n_____ %s at 42" % self.relpath) options.stdout.write('\n') @@ -349,6 +372,8 @@ class SVNWrapperTestCase(BaseTestCase): scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) scm.updatesingle(options, ['DEPS'], files_list) + self.checkstdout( + 2 * ('\n_____ %s at 42\n' % self.relpath)) def testUpdateSingleCheckoutSVN14(self): options = self.Options(verbose=True) @@ -376,6 +401,7 @@ class SVNWrapperTestCase(BaseTestCase): scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) scm.updatesingle(options, ['DEPS'], files_list) + self.checkstdout('') def testUpdateSingleCheckoutSVNUpgrade(self): options = self.Options(verbose=True) @@ -414,15 +440,15 @@ class SVNWrapperTestCase(BaseTestCase): ).AndReturn(False) gclient_scm.os.path.exists(base_path).AndReturn(True) gclient_scm.scm.SVN.CaptureInfo( - gclient_scm.os.path.join(base_path, "."), '.' - ).AndReturn(file_info) - gclient_scm.scm.SVN.CaptureInfo(file_info['URL'], '.').AndReturn(file_info) - print("\n_____ %s at 42" % self.relpath) + gclient_scm.os.path.join(base_path, ".")).AndReturn(file_info) + gclient_scm.scm.SVN.CaptureInfo(file_info['URL']).AndReturn(file_info) self.mox.ReplayAll() scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) scm.updatesingle(options, ['DEPS'], files_list) + self.checkstdout( + ('\n_____ %s at 42\n' % self.relpath)) def testUpdateSingleUpdate(self): options = self.Options(verbose=True) @@ -448,27 +474,27 @@ class SVNWrapperTestCase(BaseTestCase): ).AndReturn(False) gclient_scm.os.path.exists(base_path).AndReturn(True) gclient_scm.scm.SVN.CaptureInfo( - gclient_scm.os.path.join(base_path, "."), '.' - ).AndReturn(file_info) - gclient_scm.scm.SVN.CaptureInfo(file_info['URL'], '.').AndReturn(file_info) - print("\n_____ %s at 42" % self.relpath) + gclient_scm.os.path.join(base_path, ".")).AndReturn(file_info) + gclient_scm.scm.SVN.CaptureInfo(file_info['URL']).AndReturn(file_info) self.mox.ReplayAll() scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) scm.updatesingle(options, ['DEPS'], files_list) + self.checkstdout('\n_____ %s at 42\n' % self.relpath) def testUpdateGit(self): options = self.Options(verbose=True) file_path = gclient_scm.os.path.join(self.root_dir, self.relpath, '.git') gclient_scm.os.path.exists(file_path).AndReturn(True) - print("________ found .git directory; skipping %s" % self.relpath) self.mox.ReplayAll() scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir, relpath=self.relpath) file_list = [] scm.update(options, self.args, file_list) + self.checkstdout( + ('________ found .git directory; skipping %s\n' % self.relpath)) class GitWrapperTestCase(BaseTestCase): @@ -482,6 +508,7 @@ class GitWrapperTestCase(BaseTestCase): self.force = False self.reset = False self.nohooks = False + self.stdout = StringIO.StringIO() sample_git_import = """blob mark :1 @@ -560,10 +587,10 @@ from :3 self.relpath = '.' self.base_path = gclient_scm.os.path.join(self.root_dir, self.relpath) self.enabled = self.CreateGitRepo(self.sample_git_import, self.base_path) - SuperMoxBaseTestBase.setUp(self) + BaseTestBase.setUp(self) def tearDown(self): - SuperMoxBaseTestBase.tearDown(self) + BaseTestBase.tearDown(self) rmtree(self.root_dir) def testDir(self): diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 6add5acad..0df633537 100755 --- a/tests/scm_unittest.py +++ b/tests/scm_unittest.py @@ -28,9 +28,10 @@ class BaseTestCase(SuperMoxTestBase): class BaseSCMTestCase(BaseTestCase): def setUp(self): BaseTestCase.setUp(self) - self.mox.StubOutWithMock(scm.gclient_utils, 'Popen') + self.mox.StubOutWithMock(scm.gclient_utils, 'CheckCall') self.mox.StubOutWithMock(scm.gclient_utils, 'CheckCallAndFilter') self.mox.StubOutWithMock(scm.gclient_utils, 'CheckCallAndFilterAndHeader') + self.mox.StubOutWithMock(scm.gclient_utils, 'Popen') class RootTestCase(BaseSCMTestCase): @@ -153,13 +154,13 @@ class SVNTestCase(BaseSCMTestCase): self.args = self.Args() self.url = self.Url() self.relpath = 'asf' + self.mox.StubOutWithMock(scm.SVN, 'Capture') def testMembersChanged(self): self.mox.ReplayAll() members = [ - 'AssertVersion', 'Capture', 'CaptureBaseRevision', - 'CaptureHeadRevision', 'CaptureInfo', 'CaptureStatus', - 'current_version', 'DiffItem', 'GenerateDiff', + 'AssertVersion', 'Capture', 'CaptureRevision', 'CaptureInfo', + 'CaptureStatus', 'current_version', 'DiffItem', 'GenerateDiff', 'GetCheckoutRoot', 'GetEmail', 'GetFileProperty', 'IsMoved', 'IsMovedInfo', 'ReadSimpleAuth', 'RunAndGetFileList', ] @@ -172,10 +173,10 @@ class SVNTestCase(BaseSCMTestCase): scm.os.path.abspath(self.root_dir + 'x').AndReturn(self.root_dir) scm.GetCasedPath(self.root_dir).AndReturn(self.root_dir) result1 = { "Repository Root": "Some root" } - scm.SVN.CaptureInfo(self.root_dir, print_error=False).AndReturn(result1) + scm.SVN.CaptureInfo(self.root_dir).AndReturn(result1) results2 = { "Repository Root": "A different root" } - scm.SVN.CaptureInfo(scm.os.path.dirname(self.root_dir), - print_error=False).AndReturn(results2) + scm.SVN.CaptureInfo( + scm.os.path.dirname(self.root_dir)).AndReturn(results2) self.mox.ReplayAll() self.assertEquals(scm.SVN.GetCheckoutRoot(self.root_dir + 'x'), self.root_dir) @@ -196,8 +197,7 @@ class SVNTestCase(BaseSCMTestCase): """ % self.url - self.mox.StubOutWithMock(scm.SVN, 'Capture') - scm.SVN.Capture(['info', '--xml', self.url], '.', True).AndReturn(xml_text) + scm.SVN.Capture(['info', '--xml', self.url]).AndReturn(xml_text) expected = { 'URL': 'http://src.chromium.org/svn/trunk/src/chrome/app/d', 'UUID': None, @@ -211,7 +211,7 @@ class SVNTestCase(BaseSCMTestCase): 'Node Kind': 'file', } self.mox.ReplayAll() - file_info = scm.SVN.CaptureInfo(self.url, '.', True) + file_info = scm.SVN.CaptureInfo(self.url) self.assertEquals(sorted(file_info.items()), sorted(expected.items())) def testCaptureInfo(self): @@ -238,10 +238,9 @@ class SVNTestCase(BaseSCMTestCase): """ % (self.url, self.root_dir) - self.mox.StubOutWithMock(scm.SVN, 'Capture') - scm.SVN.Capture(['info', '--xml', self.url], '.', True).AndReturn(xml_text) + scm.SVN.Capture(['info', '--xml', self.url]).AndReturn(xml_text) self.mox.ReplayAll() - file_info = scm.SVN.CaptureInfo(self.url, '.', True) + file_info = scm.SVN.CaptureInfo(self.url) expected = { 'URL': self.url, 'UUID': '7b9385f5-0452-0410-af26-ad4892b7a1fb', @@ -293,12 +292,7 @@ class SVNTestCase(BaseSCMTestCase): """ - proc = self.mox.CreateMockAnything() - scm.gclient_utils.Popen(['svn', 'status', '--xml', '.'], - cwd=None, - stderr=None, - stdout=scm.subprocess.PIPE).AndReturn(proc) - proc.communicate().AndReturn((text, 0)) + scm.SVN.Capture(['status', '--xml', '.']).AndReturn(text) self.mox.ReplayAll() info = scm.SVN.CaptureStatus('.') @@ -318,12 +312,7 @@ class SVNTestCase(BaseSCMTestCase): path="perf"> """ - proc = self.mox.CreateMockAnything() - scm.gclient_utils.Popen(['svn', 'status', '--xml'], - cwd=None, - stderr=None, - stdout=scm.subprocess.PIPE).AndReturn(proc) - proc.communicate().AndReturn((text, 0)) + scm.SVN.Capture(['status', '--xml']).AndReturn(text) self.mox.ReplayAll() info = scm.SVN.CaptureStatus(None) self.assertEquals(info, [])