Check that desired repo was actually checked out.

Change GitRepository._determine_remote_name() into a classmethod _remote_name_for_url that takes in a url, for more clarity in how the remote name is being picked.

manic/repository_git.py

@@ -194,7 +194,7 @@ def compare_refs(current_ref, expected_ref):
             if self._url == LOCAL_PATH_INDICATOR:
                 expected_ref = self._branch
             else:
-                remote_name = self._determine_remote_name()
+                remote_name = self._remote_name_for_url(self._url)
                 if not remote_name:
                     # git doesn't know about this remote. by definition
                     # this is a modified state.
@@ -232,27 +232,23 @@ def compare_refs(current_ref, expected_ref):
 
         os.chdir(cwd)
 
-    def _determine_remote_name(self):
-        """Return the remote name.
-
-        Note that this is for the *future* repo url and branch, not
-        the current working copy!
+    @classmethod
+    def _remote_name_for_url(cls, remote_url, dir=None):
+        """Return the remote name matching remote_url (or None)
 
         """
-        git_output = self._git_remote_verbose()
+        git_output = cls._git_remote_verbose(dir)
         git_output = git_output.splitlines()
-        remote_name = ''
         for line in git_output:
             data = line.strip()
             if not data:
                 continue
             data = data.split()
             name = data[0].strip()
             url = data[1].strip()
-            if self._url == url:
-                remote_name = name
-                break
-        return remote_name
+            if remote_url == url:
+                return name
+        return None
 
     def _create_remote_name(self):
         """The url specified in the externals description file was not known
@@ -353,7 +349,7 @@ def _checkout_external_ref(self, verbosity, submodules):
         else:
             ref = self._hash
 
-        remote_name = self._determine_remote_name()
+        remote_name = self._remote_name_for_url(self._url)
         if not remote_name:
             remote_name = self._create_remote_name()
             self._git_remote_add(remote_name, self._url)
@@ -612,7 +608,7 @@ def _status_v1z_is_dirty(git_output):
     def _git_current_hash(dir=None):
         """Return the full hash of the currently checked-out version.
 
-        if dir is None, uses the cwd.
+        If dir is None, uses the cwd.
 
         Returns a tuple, (hash_found, hash), where hash_found is a
         logical specifying whether a hash was found for HEAD (False
@@ -787,11 +783,21 @@ def _git_status_verbose():
         return git_output
 
     @staticmethod
-    def _git_remote_verbose():
+    def _git_remote_verbose(dir=None):
         """Run the git remote command to obtain repository information.
+
+        If dir is None, uses the cwd.
+        Returned string is of the form:
+        myfork  git@github.com:johnpaulalex/manage_externals_jp.git (fetch)
+        myfork  git@github.com:johnpaulalex/manage_externals_jp.git (push)
         """
+        if dir:
+            cwd = os.getcwd()
+            os.chdir(dir)
         cmd = ['git', 'remote', '--verbose']
         git_output = execute_subprocess(cmd, output_to_caller=True)
+        if dir:
+            os.chdir(cwd)
         return git_output
 
     @staticmethod

test/test_sys_checkout.py

@@ -269,6 +269,12 @@ def create_metadata(self):
         self._config.set(DESCRIPTION_SECTION, VERSION_ITEM,
                          self._schema_version)
 
+    def url_for_repo_path(self, repo_path, repo_path_abs=None):
+        if repo_path_abs is not None:
+            return repo_path_abs
+        else:
+            return os.path.join(self._bare_root, repo_path)
+
     def create_section(self, repo_path, name, tag='', branch='',
                        ref_hash='', required=True, path=EXTERNALS_PATH,
                        sub_externals='', repo_path_abs=None, from_submodule=False,
@@ -304,10 +310,7 @@ def create_section(self, repo_path, name, tag='', branch='',
             ref_hash = ''
             branch = ''
 
-        if repo_path_abs is not None:
-            repo_url = repo_path_abs
-        else:
-            repo_url = os.path.join(self._bare_root, repo_path)
+        repo_url = self.url_for_repo_path(repo_path, repo_path_abs)
 
         if not from_submodule:
             self._config.set(name, ExternalsDescription.REPO_URL, repo_url)
@@ -639,22 +642,29 @@ def test_required_bytag(self):
         # externals start out 'empty' aka not checked out.
         tree = self.execute_checkout_in_dir(cloned_repo_dir,
                                             self.status_args)
-        local_path = self._external_path(TAG_SECTION)
-        self._check_sync_clean(tree[local_path],
+        local_path_rel = self._external_path(TAG_SECTION)
+        self._check_sync_clean(tree[local_path_rel],
                                ExternalStatus.EMPTY,
                                ExternalStatus.DEFAULT)
-        tag_full_path = os.path.join(cloned_repo_dir, local_path)
-        self.assertFalse(os.path.exists(tag_full_path))
+        local_path_abs = os.path.join(cloned_repo_dir, local_path_rel)
+        self.assertFalse(os.path.exists(local_path_abs))
 
         # after checkout, the external is 'clean' aka at the correct version.
         tree = self.execute_checkout_with_status(cloned_repo_dir,
                                                  self.checkout_args)
-        self._check_sync_clean(tree[local_path],
+        self._check_sync_clean(tree[local_path_rel],
                                ExternalStatus.STATUS_OK,
                                ExternalStatus.STATUS_OK)
 
+        # Actually checked out the desired repo.
+        self.assertEqual('origin', GitRepository._remote_name_for_url(
+            # Which url to look up
+            self._generator.url_for_repo_path(SIMPLE_REPO),
+            # Which directory has the local checked-out repo.
+            dir=local_path_abs))
+
         # Actually checked out the desired tag.
-        (tag_found, tag_name) = GitRepository._git_current_tag(tag_full_path)
+        (tag_found, tag_name) = GitRepository._git_current_tag(local_path_abs)
         self.assertEqual(tag_name, 'tag1')
 
         # Check existence of some simp_tag files
@@ -677,22 +687,31 @@ def test_required_bybranch(self):
         # externals start out 'empty' aka not checked out.
         tree = self.execute_checkout_in_dir(cloned_repo_dir,
                                             self.status_args)
-        local_path = self._external_path(BRANCH_SECTION)
-        self._check_sync_clean(tree[local_path],
+        local_path_rel = self._external_path(BRANCH_SECTION)
+        self._check_sync_clean(tree[local_path_rel],
                                ExternalStatus.EMPTY,
                                ExternalStatus.DEFAULT)
-        branch_path = os.path.join(cloned_repo_dir, local_path)
-        self.assertFalse(os.path.exists(branch_path))
+        local_path_abs = os.path.join(cloned_repo_dir, local_path_rel)
+        self.assertFalse(os.path.exists(local_path_abs))
 
         # after checkout, the external is 'clean' aka at the correct version.
         tree = self.execute_checkout_with_status(cloned_repo_dir,
                                                  self.checkout_args)
-        self._check_sync_clean(tree[local_path],
+        self._check_sync_clean(tree[local_path_rel],
                                ExternalStatus.STATUS_OK,
                                ExternalStatus.STATUS_OK)
-        self.assertTrue(os.path.exists(branch_path))
+        self.assertTrue(os.path.exists(local_path_abs))
+
+        # Actually checked out the desired repo.
+        self.assertEqual('origin', GitRepository._remote_name_for_url(
+            # Which url to look up
+            self._generator.url_for_repo_path(SIMPLE_REPO),
+            # Which directory has the local checked-out repo.
+            dir=local_path_abs))
+
+        # Actually checked out the desired branch. 
         (branch_found, branch_name) = GitRepository._git_current_remote_branch(
-            branch_path)
+            local_path_abs)
         self.assertEquals(branch_name, 'origin/' + REMOTE_BRANCH_FEATURE2)
 
     def test_required_byhash(self):
@@ -706,20 +725,30 @@ def test_required_byhash(self):
         # externals start out 'empty' aka not checked out.
         tree = self.execute_checkout_in_dir(cloned_repo_dir,
                                             self.status_args)
-        local_path = self._external_path(HASH_SECTION)
-        self._check_sync_clean(tree[local_path],
+        local_path_rel = self._external_path(HASH_SECTION)
+        self._check_sync_clean(tree[local_path_rel],
                                ExternalStatus.EMPTY,
                                ExternalStatus.DEFAULT)
-        hash_path = os.path.join(cloned_repo_dir, local_path)
-        self.assertFalse(os.path.exists(hash_path))
+        local_path_abs = os.path.join(cloned_repo_dir, local_path_rel)
+        self.assertFalse(os.path.exists(local_path_abs))
 
         # after checkout, the externals are 'clean' aka at their correct version.
         tree = self.execute_checkout_with_status(cloned_repo_dir,
                                                  self.checkout_args)
-        self._check_sync_clean(tree[local_path],
+        self._check_sync_clean(tree[local_path_rel],
                                ExternalStatus.STATUS_OK,
                                ExternalStatus.STATUS_OK)
-        (hash_found, hash_name) = GitRepository._git_current_hash(hash_path)
+
+        # Actually checked out the desired repo.
+        self.assertEqual('origin', GitRepository._remote_name_for_url(
+            # Which url to look up
+            self._generator.url_for_repo_path(SIMPLE_REPO),
+            # Which directory has the local checked-out repo.
+            dir=local_path_abs))
+
+        # Actually checked out the desired hash.
+        (hash_found, hash_name) = GitRepository._git_current_hash(
+            local_path_abs)
         self.assertTrue(hash_name.startswith('60b1cc1a38d63'),
                         msg=hash_name)
 

test/test_unit_repository_git.py

@@ -433,7 +433,7 @@ def test_sync_branch_on_diff_hash(self):
         self.assertEqual(stat.clean_state, ExternalStatus.DEFAULT)
 
     def test_sync_branch_diff_remote(self):
-        """Test _determine_remote_name with a different remote
+        """Test _remote_name_for_url with a different remote
 
         """
         stat = ExternalStatus()
@@ -449,7 +449,7 @@ def test_sync_branch_diff_remote(self):
         # expected argument
 
     def test_sync_branch_diff_remote2(self):
-        """Test _determine_remote_name with a different remote
+        """Test _remote_name_for_url with a different remote
 
         """
         stat = ExternalStatus()