Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(387)

Unified Diff: recipe_engine/unittests/fetch_test.py

Issue 2720853002: Fix GitBackend.checkout (Closed)
Patch Set: tests Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: recipe_engine/unittests/fetch_test.py
diff --git a/recipe_engine/unittests/fetch_test.py b/recipe_engine/unittests/fetch_test.py
index caa355a7e4ee201cf00d9ba3da1dd548b3cf595f..5d35a0a4007688e4f05a93535b72686f32aeed47 100755
--- a/recipe_engine/unittests/fetch_test.py
+++ b/recipe_engine/unittests/fetch_test.py
@@ -15,6 +15,9 @@ import subprocess42
from recipe_engine import fetch
from recipe_engine import requests_ssl
+def mock_git_dir(*args):
+ return mock.call('GIT', '-C', 'dir', *args)
+
class TestGit(unittest.TestCase):
@@ -24,6 +27,8 @@ class TestGit(unittest.TestCase):
mock.patch('logging.exception'),
mock.patch('recipe_engine.fetch.GitBackend.Git._resolve_git',
return_value='GIT'),
+ mock.patch('os.path.isdir', return_value=False),
+ mock.patch('os.makedirs'),
]
for p in self._patchers:
p.start()
@@ -33,152 +38,186 @@ class TestGit(unittest.TestCase):
p.stop()
@mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
- def test_fresh_clone(self, git):
+ def test_fresh_fetch_branch(self, git):
fetch.GitBackend().checkout(
- 'repo', 'revision', 'dir', allow_fetch=True)
+ 'repo', 'refs/heads/master', 'dir', allow_fetch=True)
+ git.assert_has_calls([
+ mock_git_dir('init'),
+ mock_git_dir('fetch', 'repo', 'refs/heads/master'),
+ mock_git_dir('checkout', '-q', '-f', 'FETCH_HEAD'),
+ ])
+
+ @mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
+ def test_fresh_fetch_commit(self, git):
+ fetch.GitBackend().checkout(
+ 'repo', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'dir',
+ allow_fetch=True)
git.assert_has_calls([
- mock.call('GIT', 'clone', '-q', 'repo', 'dir'),
- mock.call('GIT', '-C', 'dir', 'config', 'remote.origin.url', 'repo'),
- mock.call('GIT', '-C', 'dir', 'rev-parse', '-q', '--verify',
- 'revision^{commit}'),
- mock.call('GIT', '-C', 'dir', 'reset', '-q', '--hard', 'revision'),
+ mock_git_dir('init'),
+ mock_git_dir('fetch', 'repo', 'refs/heads/master'),
+ mock_git_dir(
+ 'checkout', '-q', '-f', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'),
])
@mock.patch('time.sleep')
- @mock.patch('os.path.isdir')
@mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
- def test_fresh_clone_retries(self, git, isdir, sleep):
- isdir.return_value = False
+ def test_fresh_fetch_retries(self, git, sleep):
- clone_fails = []
- def fail_four_clones(*args):
- if 'clone' in args and len(clone_fails) < 4:
- clone_fails.append(True)
+ fetch_fails = []
+ def fail_four_fetches(*args):
+ if 'fetch' in args and len(fetch_fails) < 4:
+ fetch_fails.append(True)
raise subprocess42.CalledProcessError(1, args)
return None
- git.side_effect = fail_four_clones
+ git.side_effect = fail_four_fetches
fetch.GitBackend().checkout(
- 'repo', 'revision', 'dir', allow_fetch=True)
+ 'repo', 'refs/heads/master', 'dir', allow_fetch=True)
git.assert_has_calls([
- mock.call('GIT', 'clone', '-q', 'repo', 'dir')] * 5 + [
- mock.call('GIT', '-C', 'dir', 'config', 'remote.origin.url', 'repo'),
- mock.call('GIT', '-C', 'dir', 'rev-parse', '-q', '--verify',
- 'revision^{commit}'),
- mock.call('GIT', '-C', 'dir', 'reset', '-q', '--hard', 'revision'),
+ mock_git_dir('init'),
+ mock_git_dir('fetch', 'repo', 'refs/heads/master'),
+ mock_git_dir('checkout', '-q', '-f', 'FETCH_HEAD'),
])
+ self.assertEqual(len(fetch_fails), 4)
@mock.patch('os.path.isdir')
@mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
- def test_existing_checkout(self, git, isdir):
+ def test_existing_checkout_commit_present(self, git, isdir):
isdir.return_value = True
fetch.GitBackend().checkout(
- 'repo', 'revision', 'dir', allow_fetch=True)
+ 'repo', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'dir',
+ allow_fetch=True)
isdir.assert_has_calls([
mock.call('dir'),
mock.call('dir/.git'),
])
git.assert_has_calls([
- mock.call('GIT', '-C', 'dir', 'config', 'remote.origin.url', 'repo'),
- mock.call('GIT', '-C', 'dir', 'rev-parse', '-q', '--verify',
- 'revision^{commit}'),
- mock.call('GIT', '-C', 'dir', 'reset', '-q', '--hard', 'revision'),
- ])
-
- @mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
- def test_clone_not_allowed(self, git):
- with self.assertRaises(fetch.FetchNotAllowedError):
- fetch.GitBackend().checkout(
- 'repo', 'revision', 'dir', allow_fetch=False)
-
- @mock.patch('os.path.isdir')
- @mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
- def test_unclean_filesystem(self, git, isdir):
- isdir.side_effect = [True, False]
- with self.assertRaises(fetch.UncleanFilesystemError):
- fetch.GitBackend().checkout(
- 'repo', 'revision', 'dir', allow_fetch=False)
- isdir.assert_has_calls([
- mock.call('dir'),
- mock.call('dir/.git'),
+ mock_git_dir(
+ 'rev-parse', '-q', '--verify',
+ 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa^{commit}'),
+ mock_git_dir('checkout', '-q', '-f',
+ 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'),
])
@mock.patch('os.path.isdir')
@mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
- def test_origin_mismatch(self, git, isdir):
- git.return_value = 'not-repo'
+ def test_existing_checkout_commit_present_no_fetch(self, git, isdir):
isdir.return_value = True
-
- # This should not raise UncleanFilesystemError, but instead
- # set the right origin automatically.
fetch.GitBackend().checkout(
- 'repo', 'revision', 'dir', allow_fetch=False)
-
+ 'repo', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'dir',
+ allow_fetch=False)
isdir.assert_has_calls([
mock.call('dir'),
mock.call('dir/.git'),
])
git.assert_has_calls([
- mock.call('GIT', '-C', 'dir', 'config', 'remote.origin.url', 'repo'),
- ])
+ mock_git_dir(
+ 'rev-parse', '-q', '--verify',
+ 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa^{commit}'),
+ mock_git_dir('checkout', '-q', '-f',
+ 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'),
+ ])
@mock.patch('os.path.isdir')
@mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
- def test_rev_parse_fail(self, git, isdir):
+ def test_existing_checkout_commit_not_present(self, git, isdir):
+ isdir.return_value = True
git.side_effect = [
+ subprocess42.CalledProcessError(1, ['git-rev-parse']),
None,
- subprocess42.CalledProcessError(1, ['fakecmd']),
None,
None,
]
isdir.return_value = True
fetch.GitBackend().checkout(
- 'repo', 'revision', 'dir', allow_fetch=True)
+ 'repo', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'dir',
+ allow_fetch=True)
isdir.assert_has_calls([
mock.call('dir'),
mock.call('dir/.git'),
])
git.assert_has_calls([
- mock.call('GIT', '-C', 'dir', 'config', 'remote.origin.url', 'repo'),
- mock.call('GIT', '-C', 'dir', 'rev-parse', '-q', '--verify',
- 'revision^{commit}'),
- mock.call('GIT', '-C', 'dir', 'fetch'),
- mock.call('GIT', '-C', 'dir', 'reset', '-q', '--hard', 'revision'),
+ mock_git_dir(
+ 'rev-parse', '-q', '--verify',
+ 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa^{commit}'),
+ mock_git_dir('init'),
+ mock_git_dir('fetch', 'repo', 'refs/heads/master'),
+ mock_git_dir('checkout', '-q', '-f',
+ 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'),
])
@mock.patch('os.path.isdir')
@mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
- def test_rev_parse_fetch_not_allowed(self, git, isdir):
+ def test_existing_checkout_commit_not_present_no_fetch(self, git, isdir):
+ isdir.return_value = True
git.side_effect = [
- None,
- subprocess42.CalledProcessError(1, ['fakecmd']),
+ subprocess42.CalledProcessError(1, ['git-rev-parse']),
]
isdir.return_value = True
with self.assertRaises(fetch.FetchNotAllowedError):
fetch.GitBackend().checkout(
- 'repo', 'revision', 'dir', allow_fetch=False)
+ 'repo', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'dir',
+ allow_fetch=False)
isdir.assert_has_calls([
mock.call('dir'),
mock.call('dir/.git'),
])
git.assert_has_calls([
- mock.call('GIT', '-C', 'dir', 'config', 'remote.origin.url', 'repo'),
- mock.call('GIT', '-C', 'dir', 'rev-parse', '-q', '--verify',
- 'revision^{commit}'),
+ mock_git_dir(
+ 'rev-parse', '-q', '--verify',
+ 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa^{commit}'),
+ ])
+
+
+ @mock.patch('os.path.isdir')
+ @mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
+ def test_existing_checkout_branch(self, git, isdir):
+ isdir.return_value = True
+ fetch.GitBackend().checkout('repo', 'refs/heads/master', 'dir',
+ allow_fetch=True)
+ isdir.assert_has_calls([
+ mock.call('dir'),
+ mock.call('dir/.git'),
+ ])
+ git.assert_has_calls([
+ mock_git_dir('init'),
+ mock_git_dir('fetch', 'repo', 'refs/heads/master'),
+ mock_git_dir('checkout', '-q', '-f', 'FETCH_HEAD'),
+ ])
+
+ @mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
+ def test_fetch_not_allowed(self, git):
+ with self.assertRaises(fetch.FetchNotAllowedError):
+ fetch.GitBackend().checkout(
+ 'repo', 'refs/heads/master', 'dir', allow_fetch=False)
+
+ @mock.patch('os.path.isdir')
+ @mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
+ def test_unclean_filesystem(self, git, isdir):
+ isdir.side_effect = [True, False]
+ with self.assertRaises(fetch.UncleanFilesystemError):
+ fetch.GitBackend().checkout(
+ 'repo', 'refs/heads/master', 'dir', allow_fetch=False)
+ isdir.assert_has_calls([
+ mock.call('dir'),
+ mock.call('dir/.git'),
])
@mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
def test_commit_metadata(self, git):
git.side_effect = ['author', 'message']
result = fetch.GitBackend().commit_metadata(
- 'repo', 'revision', 'dir', allow_fetch=True)
+ 'repo', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'dir',
+ allow_fetch=True)
self.assertEqual(result, {
'author': 'author',
'message': 'message',
})
git.assert_has_calls([
- mock.call('GIT', '-C', 'dir', 'show', '-s', '--pretty=%aE', 'revision'),
- mock.call('GIT', '-C', 'dir', 'show', '-s', '--pretty=%B', 'revision'),
+ mock_git_dir('show', '-s', '--pretty=%aE',
+ 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'),
+ mock_git_dir('show', '-s', '--pretty=%B',
+ 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'),
])

Powered by Google App Engine
This is Rietveld 408576698