Fix GitBackend.checkout

recipe_engine/unittests/fetch_test.py

 #!/usr/bin/env python
 # Copyright 2016 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 import base64
 import json
 import unittest
 
 import test_env
 
 import mock
 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):
 
   def setUp(self):
     self._patchers = [
         mock.patch('logging.warning'),
         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()
 
   def tearDown(self):
     for p in reversed(self._patchers):
       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.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', '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_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_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_existing_checkout_commit_present_no_fetch(self, git, isdir):
+    isdir.return_value = True
+    fetch.GitBackend().checkout(
+        'repo', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'dir',
+        allow_fetch=False)
+    isdir.assert_has_calls([
+      mock.call('dir'),
+      mock.call('dir/.git'),
+    ])
+    git.assert_has_calls([
+      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_existing_checkout_commit_not_present(self, git, isdir):
+    isdir.return_value = True
+    git.side_effect = [
+      subprocess42.CalledProcessError(1, ['git-rev-parse']),
+      None,
+      None,
+      None,
+    ]
+    isdir.return_value = True
+    fetch.GitBackend().checkout(
+        'repo', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'dir',
+        allow_fetch=True)
+    isdir.assert_has_calls([
+      mock.call('dir'),
+      mock.call('dir/.git'),
+    ])
+    git.assert_has_calls([
+      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_existing_checkout_commit_not_present_no_fetch(self, git, isdir):
+    isdir.return_value = True
+    git.side_effect = [
+      subprocess42.CalledProcessError(1, ['git-rev-parse']),
+    ]
+    isdir.return_value = True
+    with self.assertRaises(fetch.FetchNotAllowedError):
+      fetch.GitBackend().checkout(
+          'repo', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'dir',
+          allow_fetch=False)
+    isdir.assert_has_calls([
+      mock.call('dir'),
+      mock.call('dir/.git'),
+    ])
+    git.assert_has_calls([
+      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_clone_not_allowed(self, git):
+  def test_fetch_not_allowed(self, git):
     with self.assertRaises(fetch.FetchNotAllowedError):
       fetch.GitBackend().checkout(
-          'repo', 'revision', 'dir', allow_fetch=False)
+          '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', 'revision', 'dir', allow_fetch=False)
+          'repo', 'refs/heads/master', 'dir', allow_fetch=False)
     isdir.assert_has_calls([
       mock.call('dir'),
       mock.call('dir/.git'),
     ])
 
-  @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'
-    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)
-
-    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.patch('os.path.isdir')
-  @mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
-  def test_rev_parse_fail(self, git, isdir):
-    git.side_effect = [
-      None,
-      subprocess42.CalledProcessError(1, ['fakecmd']),
-      None,
-      None,
-    ]
-    isdir.return_value = True
-    fetch.GitBackend().checkout(
-        'repo', 'revision', '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.patch('os.path.isdir')
-  @mock.patch('recipe_engine.fetch.GitBackend.Git._execute')
-  def test_rev_parse_fetch_not_allowed(self, git, isdir):
-    git.side_effect = [
-      None,
-      subprocess42.CalledProcessError(1, ['fakecmd']),
-    ]
-    isdir.return_value = True
-    with self.assertRaises(fetch.FetchNotAllowedError):
-      fetch.GitBackend().checkout(
-          'repo', 'revision', '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.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'),
     ])
 
 
 class TestGitiles(unittest.TestCase):
   def setUp(self):
     requests_ssl.disable_check()
 
   @mock.patch('__builtin__.open', mock.mock_open())
   @mock.patch('shutil.rmtree')
   @mock.patch('os.makedirs')
@@ skipping 148 matching lines @@
       'author': 'author',
       'message': 'message',
     })
     self.assertEqual(counts['sleeps'], 4)
     requests_get.assert_has_calls([
       mock.call('repo/+/revision?format=JSON'),
     ] * 5)
 
 if __name__ == '__main__':
   unittest.main()