From 46b9ae6961155988a859e7aa8be8f2a578941aa8 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Tue, 12 May 2009 00:37:10 +0000 Subject: [PATCH] Starts reusing functions in gclient.py from gcl.py. Review URL: http://codereview.chromium.org/113218 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@15824 0039d316-1c4b-4281-b951-d872f2087c98 --- gcl.py | 64 +++++++----------------------- tests/gcl_unittest.py | 77 ++++++++++++++++++++----------------- tests/trychange_unittest.py | 72 ++++++++++++++++++++++++++++++++++ trychange.py | 3 +- 4 files changed, 128 insertions(+), 88 deletions(-) create mode 100644 tests/trychange_unittest.py diff --git a/gcl.py b/gcl.py index dd2b63979..6e64702d8 100755 --- a/gcl.py +++ b/gcl.py @@ -18,6 +18,8 @@ import upload import urllib2 import xml.dom.minidom +# gcl now depends on gclient. +import gclient __version__ = '1.0' @@ -48,60 +50,16 @@ MISSING_TEST_MSG = "Change contains new or modified methods, but no new tests!" read_gcl_info = False -### Simplified XML processing functions. - -def ParseXML(output): - try: - return xml.dom.minidom.parseString(output) - except xml.parsers.expat.ExpatError: - return None - -def GetNamedNodeText(node, node_name): - child_nodes = node.getElementsByTagName(node_name) - if not child_nodes: - return None - assert len(child_nodes) == 1 and child_nodes[0].childNodes.length == 1 - return child_nodes[0].firstChild.nodeValue - - -def GetNodeNamedAttributeText(node, node_name, attribute_name): - child_nodes = node.getElementsByTagName(node_name) - if not child_nodes: - return None - assert len(child_nodes) == 1 - return child_nodes[0].getAttribute(attribute_name) - - ### SVN Functions def IsSVNMoved(filename): """Determine if a file has been added through svn mv""" - info = GetSVNFileInfo(filename) + info = gclient.CaptureSVNInfo(filename) return (info.get('Copied From URL') and info.get('Copied From Rev') and info.get('Schedule') == 'add') -def GetSVNFileInfo(file): - """Returns a dictionary from the svn info output for the given file.""" - dom = ParseXML(RunShell(["svn", "info", "--xml", file])) - result = {} - if dom: - # /info/entry/ - # url - # reposityory/(root|uuid) - # wc-info/(schedule|depth) - # commit/(author|date) - result['Node Kind'] = GetNodeNamedAttributeText(dom, 'entry', 'kind') - result['Repository Root'] = GetNamedNodeText(dom, 'root') - result['Schedule'] = GetNamedNodeText(dom, 'schedule') - result['URL'] = GetNamedNodeText(dom, 'url') - result['Path'] = GetNodeNamedAttributeText(dom, 'entry', 'path') - result['Copied From URL'] = GetNamedNodeText(dom, 'copy-from-url') - result['Copied From Rev'] = GetNamedNodeText(dom, 'copy-from-rev') - return result - - def GetSVNFileProperty(file, property_name): """Returns the value of an SVN property for the given file. @@ -147,7 +105,7 @@ def GetSVNStatus(file): 'unversioned': '?', # TODO(maruel): Find the corresponding strings for X, ~ } - dom = ParseXML(RunShell(command)) + dom = gclient.ParseXML(RunShell(command)) results = [] if dom: # /status/target/entry/(wc-status|commit|author|date) @@ -202,14 +160,16 @@ def GetRepositoryRoot(): """ global repository_root if not repository_root: - cur_dir_repo_root = GetSVNFileInfo(os.getcwd()).get("Repository Root") + infos = gclient.CaptureSVNInfo(os.getcwd(), print_error=False) + cur_dir_repo_root = infos.get("Repository Root") if not cur_dir_repo_root: raise Exception("gcl run outside of repository") repository_root = os.getcwd() while True: parent = os.path.dirname(repository_root) - if GetSVNFileInfo(parent).get("Repository Root") != cur_dir_repo_root: + if (gclient.CaptureSVNInfo(parent).get("Repository Root") != + cur_dir_repo_root): break repository_root = parent return repository_root @@ -232,7 +192,7 @@ def GetCodeReviewSetting(key): cached_settings_file = os.path.join(GetInfoDir(), CODEREVIEW_SETTINGS_FILE) if (not os.path.exists(cached_settings_file) or os.stat(cached_settings_file).st_mtime > 60*60*24*3): - dir_info = GetSVNFileInfo(".") + dir_info = gclient.CaptureSVNInfo(".") repo_root = dir_info["Repository Root"] url_path = dir_info["URL"] settings = "" @@ -334,11 +294,13 @@ class ChangeInfo: files: a list of 2 tuple containing (status, filename) of changed files, with paths being relative to the top repository directory. """ - def __init__(self, name="", issue="", description="", files=[]): + def __init__(self, name="", issue="", description="", files=None): self.name = name self.issue = issue self.description = description self.files = files + if self.files is None: + self.files = [] self.patch = None def FileList(self): @@ -750,7 +712,7 @@ def GenerateDiff(files, root=None): for file in files: # Use svn info output instead of os.path.isdir because the latter fails # when the file is deleted. - if GetSVNFileInfo(file).get("Node Kind") in ("dir", "directory"): + if gclient.CaptureSVNInfo(file).get("Node Kind") in ("dir", "directory"): continue # If the user specified a custom diff command in their svn config file, # then it'll be used when we do svn diff, which we don't want to happen diff --git a/tests/gcl_unittest.py b/tests/gcl_unittest.py index b36dd9a57..54e6d70be 100755 --- a/tests/gcl_unittest.py +++ b/tests/gcl_unittest.py @@ -5,7 +5,9 @@ """Unit tests for gcl.py.""" +import StringIO import os +import sys import unittest # Local imports @@ -46,15 +48,14 @@ class GclUnittest(GclTestsBase): 'ErrorExit', 'GenerateChangeName', 'GenerateDiff', 'GetCLs', 'GetChangelistInfoFile', 'GetCodeReviewSetting', 'GetEditor', 'GetFilesNotInCL', 'GetInfoDir', 'GetIssueDescription', - 'GetModifiedFiles', 'GetNamedNodeText', 'GetNodeNamedAttributeText', - 'GetRepositoryRoot', 'GetSVNFileInfo', 'GetSVNStatus', + 'GetModifiedFiles', 'GetRepositoryRoot', 'GetSVNStatus', 'GetSVNFileProperty', 'Help', 'IGNORE_PATHS', 'IsSVNMoved', 'IsTreeOpen', 'Lint', 'LoadChangelistInfo', 'LoadChangelistInfoForMultiple', - 'MISSING_TEST_MSG', 'Opened', 'ParseXML', 'PresubmitCL', 'ReadFile', + 'MISSING_TEST_MSG', 'Opened', 'PresubmitCL', 'ReadFile', 'RunShell', 'RunShellWithReturnCode', 'SEPARATOR', 'SendToRietveld', 'TryChange', - 'UnknownFiles', 'UploadCL', 'Warn', 'WriteFile', 'gcl_info_dir', - 'getpass', 'main', 'os', 'random', 're', 'read_gcl_info', + 'UnknownFiles', 'UploadCL', 'Warn', 'WriteFile', 'gclient', + 'gcl_info_dir', 'getpass', 'main', 'os', 'random', 're', 'read_gcl_info', 'repository_root', 'string', 'subprocess', 'sys', 'tempfile', 'upload', 'urllib2', 'use_shell', 'xml', @@ -62,37 +63,6 @@ class GclUnittest(GclTestsBase): # If this test fails, you should add the relevant test. self.compareMembers(gcl, members) - def testGetSVNFileInfo(self): - def RunShellMock(command): - return r""" - - -http://src.chromium.org/svn/trunk/src/chrome/app/d -http://src.chromium.org/svn - -add -infinity -http://src.chromium.org/svn/trunk/src/chrome/app/DEPS -14628 -369f59057ba0e6d9017e28f8bdfb1f43 - - - -""" % command[3] - gcl.RunShell = RunShellMock - filename = os.path.join('app', 'd') - info = gcl.GetSVNFileInfo(filename) - expected = { - 'URL': 'http://src.chromium.org/svn/trunk/src/chrome/app/d', - 'Repository Root': 'http://src.chromium.org/svn', - 'Schedule': 'add', - 'Copied From URL': 'http://src.chromium.org/svn/trunk/src/chrome/app/DEPS', - 'Copied From Rev': '14628', - 'Path': filename, - 'Node Kind': 'file', - } - self.assertEquals(sorted(info.items()), sorted(expected.items())) - def testGetSVNStatus(self): def RunShellMock(command): return r""" @@ -156,6 +126,41 @@ class GclUnittest(GclTestsBase): info = gcl.GetSVNStatus(None) self.assertEquals(info, []) + def testHelp(self): + stdout = sys.stdout + dummy = StringIO.StringIO() + sys.stdout = dummy + gcl.Help() + sys.stdout = stdout + self.assertEquals(len(dummy.getvalue()), 1718) + + def testGetRepositoryRoot(self): + try: + gcl.GetRepositoryRoot() + except Exception,e: + self.assertEquals(e.args[0], "gcl run outside of repository") + + +class ChangeInfoUnittest(GclTestsBase): + def testChangeInfoMembers(self): + members = [ + 'CloseIssue', 'Delete', 'FileList', 'MissingTests', 'Save', + 'UpdateRietveldDescription', 'description', 'files', 'issue', 'name', + 'patch' + ] + # If this test fails, you should add the relevant test. + self.compareMembers(gcl.ChangeInfo(), members) + + def testChangeInfoBase(self): + files = [('M', 'foo'), ('A', 'bar')] + o = gcl.ChangeInfo('name2', 'issue2', 'description2', files) + self.assertEquals(o.name, 'name2') + self.assertEquals(o.issue, 'issue2') + self.assertEquals(o.description, 'description2') + self.assertEquals(o.files, files) + self.assertEquals(o.patch, None) + self.assertEquals(o.FileList(), ['foo', 'bar']) + if __name__ == '__main__': unittest.main() diff --git a/tests/trychange_unittest.py b/tests/trychange_unittest.py new file mode 100644 index 000000000..1e68dfc3e --- /dev/null +++ b/tests/trychange_unittest.py @@ -0,0 +1,72 @@ +#!/usr/bin/python +# Copyright (c) 2009 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +"""Unit tests for trychange.py.""" + +import os +import unittest + +# Local imports +import trychange + + +class TryChangeTestsBase(unittest.TestCase): + """Setups and tear downs the mocks but doesn't test anything as-is.""" + def setUp(self): + pass + + def tearDown(self): + pass + + def compareMembers(self, object, members): + """If you add a member, be sure to add the relevant test!""" + # Skip over members starting with '_' since they are usually not meant to + # be for public use. + actual_members = [x for x in sorted(dir(object)) + if not x.startswith('_')] + expected_members = sorted(members) + if actual_members != expected_members: + diff = ([i for i in actual_members if i not in expected_members] + + [i for i in expected_members if i not in actual_members]) + print diff + self.assertEqual(actual_members, expected_members) + + +class TryChangeUnittest(TryChangeTestsBase): + """General trychange.py tests.""" + def testMembersChanged(self): + members = [ + 'EscapeDot', 'ExecuteTryServerScript', 'GIT', 'GetSourceRoot', 'GuessVCS', + 'HELP_STRING', 'InvalidScript', 'NoTryServerAccess', 'PathDifference', + 'RunCommand', 'SCM', 'SCRIPT_PATH', 'SVN', 'TryChange', 'USAGE', + 'datetime', 'gcl', 'gclient', 'getpass', 'logging', 'optparse', 'os', + 'shutil', 'sys', 'tempfile', 'traceback', 'urllib', + ] + # If this test fails, you should add the relevant test. + self.compareMembers(trychange, members) + + +class SVNUnittest(TryChangeTestsBase): + """General trychange.py tests.""" + def testMembersChanged(self): + members = [ + 'GenerateDiff', 'ProcessOptions', 'options' + ] + # If this test fails, you should add the relevant test. + self.compareMembers(trychange.SVN(None), members) + + +class TryChangeUnittest(TryChangeTestsBase): + """General trychange.py tests.""" + def testMembersChanged(self): + members = [ + 'GenerateDiff', 'GetEmail', 'GetPatchName', 'ProcessOptions', 'options' + ] + # If this test fails, you should add the relevant test. + self.compareMembers(trychange.GIT(None), members) + + +if __name__ == '__main__': + unittest.main() diff --git a/trychange.py b/trychange.py index 990f57525..3666b984e 100755 --- a/trychange.py +++ b/trychange.py @@ -20,6 +20,7 @@ import traceback import urllib import gcl +import gclient __version__ = '1.1' @@ -132,7 +133,7 @@ class SVN(SCM): for file in files: # Use svn info output instead of os.path.isdir because the latter fails # when the file is deleted. - if gcl.GetSVNFileInfo(file).get("Node Kind") == "directory": + if gclient.CaptureSVNInfo(file).get("Node Kind") in ("dir", "directory"): continue # If the user specified a custom diff command in their svn config file, # then it'll be used when we do svn diff, which we don't want to happen