pre-upload: rework main and its tests
The current main func tries to uses doctest module for verifying its
behavior. The trouble is that we don't use this module elsewhere in
CrOS, so no one ever notices it. The tests contained therein have
also rotted a bit and don't pass.
Rip the --test support out of the pre-upload.py file and merge them
into the existing unittest module.
BUG=None
TEST=`./pre-upload_unittest.py` passes
Change-Id: I64e1407884e9f25efc8a18b30c46fd0a9defbb6a
Reviewed-on: https://chromium-review.googlesource.com/236640
Reviewed-by: Nam Nguyen <namnguyen@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index c469824..2f2b196 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -305,9 +305,8 @@
def _check_no_long_lines(_project, commit):
- """Checks that there aren't any lines longer than maxlen characters in any of
- the text files to be submitted.
- """
+ """Checks there are no lines longer than MAX_LEN in any of the text files."""
+
MAX_LEN = 80
SKIP_REGEXP = re.compile('|'.join([
r'https?://',
@@ -1020,9 +1019,7 @@
def _check_project_prefix(_project, commit):
- """Fails if the change is project specific and the commit message is not
- prefixed by the project_name.
- """
+ """Require the commit message have a project specific prefix as needed."""
files = _get_affected_files(commit, relative=True)
prefix = os.path.commonprefix(files)
@@ -1303,63 +1300,11 @@
stderr=subprocess.PIPE, cwd=path).strip()
-def direct_main(args, verbose=False):
+def direct_main(argv):
"""Run hooks directly (outside of the context of repo).
- # Setup for doctests below.
- # ...note that some tests assume that running pre-upload on this CWD is fine.
- # TODO: Use mock and actually mock out _run_project_hooks() for tests.
- >>> mydir = os.path.dirname(os.path.abspath(__file__))
- >>> olddir = os.getcwd()
-
- # OK to run w/ no arugments; will run with CWD.
- >>> os.chdir(mydir)
- >>> direct_main(['prog_name'], verbose=True)
- Running hooks on chromiumos/repohooks
- 0
- >>> os.chdir(olddir)
-
- # Run specifying a dir
- >>> direct_main(['prog_name', '--dir=%s' % mydir], verbose=True)
- Running hooks on chromiumos/repohooks
- 0
-
- # Not a problem to use a bogus project; we'll just get default settings.
- >>> direct_main(['prog_name', '--dir=%s' % mydir, '--project=X'],verbose=True)
- Running hooks on X
- 0
-
- # Run with project but no dir
- >>> os.chdir(mydir)
- >>> direct_main(['prog_name', '--project=X'], verbose=True)
- Running hooks on X
- 0
- >>> os.chdir(olddir)
-
- # Try with a non-git CWD
- >>> os.chdir('/tmp')
- >>> direct_main(['prog_name'])
- Traceback (most recent call last):
- ...
- BadInvocation: The current directory is not part of a git project.
-
- # Check various bad arguments...
- >>> direct_main(['prog_name', 'bogus'])
- Traceback (most recent call last):
- ...
- BadInvocation: Unexpected arguments: bogus
- >>> direct_main(['prog_name', '--project=bogus', '--dir=bogusdir'])
- Traceback (most recent call last):
- ...
- BadInvocation: Invalid dir: bogusdir
- >>> direct_main(['prog_name', '--project=bogus', '--dir=/tmp'])
- Traceback (most recent call last):
- ...
- BadInvocation: Not a git directory: /tmp
-
Args:
- args: The value of sys.argv
- verbose: Log verbose info while running
+ argv: The command line args to process
Returns:
0 if no pre-upload failures, 1 if failures.
@@ -1392,7 +1337,7 @@
parser.usage = "pre-upload.py [options] [commits]"
- opts, args = parser.parse_args(args[1:])
+ opts, args = parser.parse_args(argv)
if opts.rerun_since:
if args:
@@ -1436,9 +1381,6 @@
if not opts.project:
raise BadInvocation("Repo couldn't identify the project of %s" % opts.dir)
- if verbose:
- print("Running hooks on %s" % (opts.project))
-
found_error = _run_project_hooks(opts.project, proj_dir=opts.dir,
commit_list=args,
presubmit=opts.pre_submit)
@@ -1447,21 +1389,5 @@
return 0
-def _test():
- """Run any built-in tests."""
- import doctest
- doctest.testmod()
-
-
if __name__ == '__main__':
- if sys.argv[1:2] == ["--test"]:
- _test()
- exit_code = 0
- else:
- prog_name = os.path.basename(sys.argv[0])
- try:
- exit_code = direct_main(sys.argv)
- except BadInvocation, err:
- print("%s: %s" % (prog_name, str(err)), file=sys.stderr)
- exit_code = 1
- sys.exit(exit_code)
+ sys.exit(direct_main(sys.argv[1:]))
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index 5f2af69..9bdcfa9 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -16,16 +16,19 @@
# pylint: disable=W0212
# We access private members of the pre_upload module all over the place.
-# If repo imports us, the __name__ will be __builtin__, and the wrapper will
-# be in $CHROMEOS_CHECKOUT/.repo/repo/main.py, so we need to go two directories
-# up. The same logic also happens to work if we're executed directly.
-if __name__ in ('__builtin__', '__main__'):
- sys.path.insert(0, os.path.join(os.path.dirname(sys.argv[0]), '..', '..'))
+# Make sure we can find the chromite paths.
+sys.path.insert(0, os.path.join(os.path.dirname(os.path.realpath(__file__)),
+ '..', '..'))
+from chromite.cbuildbot import constants
+from chromite.lib import cros_build_lib
from chromite.lib import cros_test_lib
from chromite.lib import git
from chromite.lib import osutils
+# Needs to be after chromite imports so we use the bundled copy.
+import mock
+
pre_upload = __import__('pre-upload')
@@ -36,7 +39,7 @@
def runTest(self):
self.assertEquals(u'', pre_upload._try_utf8_decode(''))
self.assertEquals(u'abc', pre_upload._try_utf8_decode('abc'))
- self.assertEquals(u'你好布萊恩', pre_upload._try_utf8_decode('你好布萊恩'))
+ self.assertEquals(u'ä½ å¥½å¸èæ©', pre_upload._try_utf8_decode('ä½ å¥½å¸èæ©'))
# Invalid UTF-8
self.assertEquals('\x80', pre_upload._try_utf8_decode('\x80'))
@@ -798,5 +801,65 @@
DiffEntry(src_file='c/p/p-9999.ebuild', status='M')])
+class DirectMainTest(cros_test_lib.MockTempDirTestCase):
+ """Tests for direct_main()"""
+
+ def setUp(self):
+ self.hooks_mock = self.PatchObject(pre_upload, '_run_project_hooks',
+ return_value=None)
+
+ def testNoArgs(self):
+ """If run w/no args, should check the current dir."""
+ ret = pre_upload.direct_main([])
+ self.assertEqual(ret, 0)
+ self.hooks_mock.assert_called_once_with(
+ mock.ANY, proj_dir=os.getcwd(), commit_list=[], presubmit=mock.ANY)
+
+ def testExplicitDir(self):
+ """Verify we can run on a diff dir."""
+ # Use the chromite dir since we know it exists.
+ ret = pre_upload.direct_main(['--dir', constants.CHROMITE_DIR])
+ self.assertEqual(ret, 0)
+ self.hooks_mock.assert_called_once_with(
+ mock.ANY, proj_dir=constants.CHROMITE_DIR, commit_list=[],
+ presubmit=mock.ANY)
+
+ def testBogusProject(self):
+ """A bogus project name should be fine (use default settings)."""
+ # Use the chromite dir since we know it exists.
+ ret = pre_upload.direct_main(['--dir', constants.CHROMITE_DIR,
+ '--project', 'foooooooooo'])
+ self.assertEqual(ret, 0)
+ self.hooks_mock.assert_called_once_with(
+ 'foooooooooo', proj_dir=constants.CHROMITE_DIR, commit_list=[],
+ presubmit=mock.ANY)
+
+ def testBogustProjectNoDir(self):
+ """Make sure --dir is detected even with --project."""
+ ret = pre_upload.direct_main(['--project', 'foooooooooo'])
+ self.assertEqual(ret, 0)
+ self.hooks_mock.assert_called_once_with(
+ 'foooooooooo', proj_dir=os.getcwd(), commit_list=[],
+ presubmit=mock.ANY)
+
+ def testNoGitDir(self):
+ """We should die when run on a non-git dir."""
+ self.assertRaises(pre_upload.BadInvocation, pre_upload.direct_main,
+ ['--dir', self.tempdir])
+
+ def testNoDir(self):
+ """We should die when run on a missing dir."""
+ self.assertRaises(pre_upload.BadInvocation, pre_upload.direct_main,
+ ['--dir', os.path.join(self.tempdir, 'foooooooo')])
+
+ def testCommitList(self):
+ """Any args on the command line should be treated as commits."""
+ commits = ['sha1', 'sha2', 'shaaaaaaaaaaaan']
+ ret = pre_upload.direct_main(commits)
+ self.assertEqual(ret, 0)
+ self.hooks_mock.assert_called_once_with(
+ mock.ANY, proj_dir=mock.ANY, commit_list=commits, presubmit=mock.ANY)
+
+
if __name__ == '__main__':
cros_test_lib.main()
