Skip to content

Commit bd9552e

Browse files
committed
Add comprehensive fix for edge case and additional test coverage
1 parent a07e38f commit bd9552e

2 files changed

Lines changed: 48 additions & 6 deletions

File tree

src/github/pullRequestGitHelper.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,6 @@ export class PullRequestGitHelper {
100100

101101
try {
102102
branch = await repository.getBranch(branchName);
103-
// Make sure we aren't already on this branch
104-
if (repository.state.HEAD?.name === branch.name) {
105-
Logger.appendLine(`Tried to checkout ${branchName}, but branch is already checked out.`, PullRequestGitHelper.ID);
106-
return;
107-
}
108-
109103
// Check if local branch is pointing to the same commit as the remote
110104
if (branch.commit !== trackedBranch.commit) {
111105
Logger.debug(`Local branch ${branchName} commit ${branch.commit} differs from remote commit ${trackedBranch.commit}. Updating local branch.`, PullRequestGitHelper.ID);
@@ -117,6 +111,12 @@ export class PullRequestGitHelper {
117111
branch = await repository.getBranch(branchName);
118112
}
119113

114+
// Make sure we aren't already on this branch
115+
if (repository.state.HEAD?.name === branch.name) {
116+
Logger.appendLine(`Tried to checkout ${branchName}, but branch is already checked out.`, PullRequestGitHelper.ID);
117+
return;
118+
}
119+
120120
Logger.debug(`Checkout ${branchName}`, PullRequestGitHelper.ID);
121121
progress.report({ message: vscode.l10n.t('Checking out {0}', branchName) });
122122
await repository.checkout(branchName);

src/test/github/pullRequestGitHelper.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,48 @@ describe('PullRequestGitHelper', function () {
8585
assert.strictEqual(updatedBranch.commit, 'remote-commit-hash');
8686
assert.strictEqual(repository.state.HEAD?.name, 'my-branch');
8787
});
88+
89+
it('updates branch even when currently checked out if commit differs from remote', async function () {
90+
const url = 'git@github.com:owner/name.git';
91+
const remote = new GitHubRemote('origin', url, new Protocol(url), GitHubServerType.GitHubDotCom);
92+
const gitHubRepository = new MockGitHubRepository(remote, credentialStore, telemetry, sinon);
93+
94+
const prItem = convertRESTPullRequestToRawPullRequest(
95+
new PullRequestBuilder()
96+
.number(100)
97+
.user(u => u.login('me'))
98+
.base(b => {
99+
(b.repo)(r => (<RepositoryBuilder>r).clone_url('git@github.com:owner/name.git'));
100+
})
101+
.head(h => {
102+
h.repo(r => (<RepositoryBuilder>r).clone_url('git@github.com:owner/name.git'));
103+
h.ref('my-branch');
104+
})
105+
.build(),
106+
gitHubRepository,
107+
);
108+
109+
const pullRequest = new PullRequestModel(credentialStore, telemetry, gitHubRepository, remote, prItem);
110+
111+
// Setup: local branch exists with different commit than remote AND is currently checked out
112+
await repository.createBranch('my-branch', true, 'local-commit-hash'); // checkout = true
113+
114+
// Setup: remote branch has different commit
115+
await repository.createBranch('refs/remotes/origin/my-branch', false, 'remote-commit-hash');
116+
117+
const remotes = [remote];
118+
119+
// Expect fetch to be called
120+
repository.expectFetch('origin', 'my-branch');
121+
122+
await PullRequestGitHelper.fetchAndCheckout(repository, remotes, pullRequest, { report: () => undefined });
123+
124+
// Verify that the local branch was updated to point to the remote commit, even though we were already on it
125+
const updatedBranch = await repository.getBranch('my-branch');
126+
assert.strictEqual(updatedBranch.commit, 'remote-commit-hash');
127+
// Since we were already on the branch, HEAD should remain unchanged
128+
assert.strictEqual(repository.state.HEAD?.name, 'my-branch');
129+
});
88130
});
89131

90132
describe('checkoutFromFork', function () {

0 commit comments

Comments
 (0)