Fix remote checkout (#385)

* Fix remote checkout: no longer dettached (using the --track option)

* Small fixes in checkout_branch(...)

* add tests for _get_branch_reference() and checkout_branch() methods + documentation

* Fix typoes + add more tests for mock__get_branch_reference + use .format() for string formatting. All tests have passed.

Author: nibheis

jupyterlab_git/git.py

@@ -609,25 +609,50 @@ def checkout_new_branch(self, branchname, current_path):
                 "message": my_error.decode("utf-8"),
             }
 
+    def _get_branch_reference(self, branchname, current_path):
+        """
+        Execute git rev-parse --symbolic-full-name <branch-name> and return the result (or None).
+        """
+        p = subprocess.Popen(
+            ["git", "rev-parse", "--symbolic-full-name", branchname],
+            stdout=PIPE,
+            stderr=PIPE,
+            cwd=os.path.join(self.root_dir, current_path),
+        )
+        my_output, my_error = p.communicate()
+        if p.returncode == 0:
+            return my_output.decode("utf-8").strip("\n")
+        else:
+            return None
+
     def checkout_branch(self, branchname, current_path):
         """
         Execute git checkout <branch-name> command & return the result.
+        Use the --track parameter for a remote branch.
         """
-        p = Popen(
-            ["git", "checkout", branchname],
+        reference_name = self._get_branch_reference(branchname, current_path)
+        if reference_name is None:
+            is_remote_branch = False
+        else:
+            is_remote_branch = self._is_remote_branch(reference_name)
+
+        if is_remote_branch:
+            cmd = ["git", "checkout", "--track", branchname]
+        else:
+            cmd = ["git", "checkout", branchname]
+
+        p = subprocess.Popen(
+            cmd,
             stdout=PIPE,
             stderr=PIPE,
             cwd=os.path.join(self.root_dir, current_path),
         )
+
         my_output, my_error = p.communicate()
         if p.returncode == 0:
-            return {"code": p.returncode, "message": my_output.decode("utf-8")}
+            return { "code": 0, "message": my_output.decode("utf-8") }
         else:
-            return {
-                "code": p.returncode,
-                "command": "git checkout " + branchname,
-                "message": my_error.decode("utf-8"),
-            }
+            return { "code": p.returncode, "message": my_error.decode("utf-8"), "command":  " ".join(cmd) }
 
     def checkout(self, filename, top_repo_path):
         """

jupyterlab_git/tests/test_branch.py

@@ -98,6 +98,254 @@ def test_get_current_branch_success(mock_subproc_popen):
     ])
     assert 'feature-foo' == actual_response
 
+@patch('subprocess.Popen')
+@patch.object(Git, '_get_branch_reference', return_value=None)
+def test_checkout_branch_noref_success(mock__get_branch_reference, mock_subproc_popen):
+    branch='test-branch'
+    curr_path='test_curr_path'
+    stdout_message='checkout output from git'
+    stderr_message=''
+    rc=0
+
+    # Given
+    process_mock = Mock()
+    attrs = {
+        'communicate.return_value': (stdout_message.encode('utf-8'), stderr_message.encode('utf-8')),
+        'returncode': rc
+    }
+    process_mock.configure_mock(**attrs)
+    mock_subproc_popen.return_value = process_mock
+
+    # When
+    actual_response = Git(root_dir='/bin').checkout_branch(branchname=branch, current_path=curr_path)
+
+    # Then
+    mock__get_branch_reference.assert_has_calls([ call(branch, curr_path) ])
+
+    cmd=['git', 'checkout', branch]
+    mock_subproc_popen.assert_has_calls([
+        call(cmd, stdout=PIPE, stderr=PIPE, cwd='/bin/{}'.format(curr_path)),
+        call().communicate()
+    ])
+
+    assert { "code": rc, "message": stdout_message } == actual_response
+
+
+@patch('subprocess.Popen')
+@patch.object(Git, '_get_branch_reference', return_value=None)
+def test_checkout_branch_noref_failure(mock__get_branch_reference, mock_subproc_popen):
+    branch='test-branch'
+    curr_path='test_curr_path'
+    stdout_message=''
+    stderr_message="error: pathspec '{}' did not match any file(s) known to git".format(branch)
+    rc=1
+
+    # Given
+    process_mock = Mock()
+    attrs = { 'communicate.return_value': (stdout_message.encode('utf-8'), stderr_message.encode('utf-8')), 'returncode': rc }
+    process_mock.configure_mock(**attrs)
+    mock_subproc_popen.return_value = process_mock
+
+    # When
+    actual_response = Git(root_dir='/bin').checkout_branch(branchname=branch, current_path=curr_path)
+
+    # Then
+    mock__get_branch_reference.assert_has_calls([ call(branch, curr_path) ])
+
+    cmd=['git', 'checkout', branch]
+    mock_subproc_popen.assert_has_calls([
+        call(cmd, stdout=PIPE, stderr=PIPE, cwd='/bin/{}'.format(curr_path)),
+        call().communicate()
+    ])
+
+    assert { "code": rc, "message": stderr_message,  "command": ' '.join(cmd) } == actual_response
+
+
+@patch('subprocess.Popen')
+@patch.object(Git, '_get_branch_reference', return_value="refs/remotes/remote_branch")
+def test_checkout_branch_remoteref_success(mock__get_branch_reference, mock_subproc_popen):
+    branch='test-branch'
+    curr_path='test_curr_path'
+    stdout_message='checkout output from git'
+    stderr_message=''
+    rc=0
+
+    # Given
+    process_mock = Mock()
+    attrs = {
+        'communicate.return_value': (stdout_message.encode('utf-8'), stderr_message.encode('utf-8')),
+        'returncode': rc
+    }
+    process_mock.configure_mock(**attrs)
+    mock_subproc_popen.return_value = process_mock
+
+    # When
+    actual_response = Git(root_dir='/bin').checkout_branch(branchname=branch, current_path=curr_path)
+
+    # Then
+    mock__get_branch_reference.assert_has_calls([ call(branch, curr_path) ])
+
+    cmd=['git', 'checkout', '--track', branch]
+    mock_subproc_popen.assert_has_calls([
+        call(cmd, stdout=PIPE, stderr=PIPE, cwd='/bin/{}'.format(curr_path)),
+        call().communicate()
+    ])
+    assert { "code": rc, "message": stdout_message } == actual_response
+
+
+@patch('subprocess.Popen')
+@patch.object(Git, '_get_branch_reference', return_value="refs/heads/local_branch")
+def test_checkout_branch_headsref_failure(mock__get_branch_reference, mock_subproc_popen):
+    branch='test-branch'
+    curr_path='test_curr_path'
+    stdout_message=''
+    stderr_message="error: pathspec '{}' did not match any file(s) known to git".format(branch)
+    rc=1
+
+    # Given
+    process_mock = Mock()
+    attrs = { 'communicate.return_value': (stdout_message.encode('utf-8'), stderr_message.encode('utf-8')), 'returncode': rc }
+    process_mock.configure_mock(**attrs)
+    mock_subproc_popen.return_value = process_mock
+
+    # When
+    actual_response = Git(root_dir='/bin').checkout_branch(branchname=branch, current_path=curr_path)
+
+    # Then
+    mock__get_branch_reference.assert_has_calls([ call(branch, curr_path) ])
+
+    cmd=['git', 'checkout', branch]
+    mock_subproc_popen.assert_has_calls([
+        call(cmd, stdout=PIPE, stderr=PIPE, cwd='/bin/{}'.format(curr_path)),
+        call().communicate()
+    ])
+    assert { "code": rc, "message": stderr_message,  "command": ' '.join(cmd) } == actual_response
+
+
+@patch('subprocess.Popen')
+@patch.object(Git, '_get_branch_reference', return_value="refs/heads/local_branch")
+def test_checkout_branch_headsref_success(mock__get_branch_reference, mock_subproc_popen):
+    branch='test-branch'
+    stdout_message='checkout output from git'
+    stderr_message=''
+    rc=0
+
+    # Given
+    process_mock = Mock()
+    attrs = {
+        'communicate.return_value': (stdout_message.encode('utf-8'), stderr_message.encode('utf-8')),
+        'returncode': rc
+    }
+    process_mock.configure_mock(**attrs)
+    mock_subproc_popen.return_value = process_mock
+
+    # When
+    actual_response = Git(root_dir='/bin').checkout_branch(
+        branchname=branch,
+        current_path='test_curr_path')
+
+    # Then
+    cmd=['git', 'checkout', branch]
+    mock_subproc_popen.assert_has_calls([
+        call(cmd, stdout=PIPE, stderr=PIPE, cwd='/bin/test_curr_path'),
+        call().communicate()
+    ])
+    assert { "code": rc, "message": stdout_message } == actual_response
+
+
+@patch('subprocess.Popen')
+@patch.object(Git, '_get_branch_reference', return_value="refs/remotes/remote_branch")
+def test_checkout_branch_remoteref_failure(mock__get_branch_reference, mock_subproc_popen):
+    branch='test-branch'
+    stdout_message=''
+    stderr_message="error: pathspec '{}' did not match any file(s) known to git".format(branch)
+    rc=1
+
+    # Given
+    process_mock = Mock()
+    attrs = { 'communicate.return_value': (stdout_message.encode('utf-8'), stderr_message.encode('utf-8')), 'returncode': rc }
+    process_mock.configure_mock(**attrs)
+    mock_subproc_popen.return_value = process_mock
+
+    # When
+    actual_response = Git(root_dir='/bin').checkout_branch(branchname=branch, current_path='test_curr_path')
+
+    # Then
+    cmd=['git', 'checkout', '--track', branch]
+    mock_subproc_popen.assert_has_calls([
+        call(cmd, stdout=PIPE, stderr=PIPE, cwd='/bin/test_curr_path'),
+        call().communicate()
+    ])
+    assert { "code": rc, "message": stderr_message,  "command": ' '.join(cmd) } == actual_response
+
+
+
+@patch('subprocess.Popen')
+def test_get_branch_reference_success(mock_subproc_popen):
+    actual_response = 0
+    branch='test-branch'
+    reference = 'refs/remotes/origin/test_branch'
+    # Given
+    process_mock = Mock()
+    attrs = {
+        'communicate.return_value': (reference.encode('utf-8'), ''.encode('utf-8')),
+        'returncode': 0
+    }
+    process_mock.configure_mock(**attrs)
+    mock_subproc_popen.return_value = process_mock
+ 
+    # When
+    actual_response = Git(root_dir='/bin')._get_branch_reference(
+        branchname=branch,
+        current_path='test_curr_path')
+ 
+    # Then
+    mock_subproc_popen.assert_has_calls([
+        call(
+            ['git', 'rev-parse', '--symbolic-full-name', branch],
+            stdout=PIPE,
+            stderr=PIPE,
+            cwd='/bin/test_curr_path'
+        ),
+        call().communicate()
+    ])
+    assert actual_response == reference
+
+
+@patch('subprocess.Popen')
+def test_get_branch_reference_failure(mock_subproc_popen):
+    actual_response = 0
+    branch='test-branch'
+    reference = 'test-branch'
+    # Given
+    process_mock = Mock()
+    attrs = {
+        'communicate.return_value': (
+            reference.encode('utf-8'),
+            "fatal: ambiguous argument '{}': unknown revision or path not in the working tree.".format(branch).encode('utf-8')
+        ),
+        'returncode': 128
+    }
+    process_mock.configure_mock(**attrs)
+    mock_subproc_popen.return_value = process_mock
+ 
+    # When
+    actual_response = Git(root_dir='/bin')._get_branch_reference(
+        branchname=branch,
+        current_path='test_curr_path')
+ 
+    # Then
+    mock_subproc_popen.assert_has_calls([
+        call(
+            ['git', 'rev-parse', '--symbolic-full-name', branch],
+            stdout=PIPE,
+            stderr=PIPE,
+            cwd='/bin/test_curr_path'
+        ),
+        call().communicate()
+    ])
+    assert actual_response is None
+
 
 @patch('subprocess.Popen')
 def test_get_current_branch_failure(mock_subproc_popen):