diff --git a/package.json b/package.json index 750a46de21c0ac0b8a2eb1a5fc2a7e7b58fb3519..6369c0df29642115f18e73d4d7a062228a30fe87 100644 --- a/package.json +++ b/package.json @@ -292,7 +292,7 @@ "view/item/context": [ { "command": "gl.checkoutMrBranch", - "when": "view =~ /issuesAndMrs/ && viewItem == mr-item-from-branch" + "when": "view =~ /issuesAndMrs/ && viewItem == mr-item-from-same-project" } ], "comments/comment/title": [ diff --git a/src/__mocks__/vscode.js b/src/__mocks__/vscode.js index de1bb6402feeed12d9066a4c010011048eec5b7d..5e7d203605aa56fd4ad16c0a0ad592b299ebf3b9 100644 --- a/src/__mocks__/vscode.js +++ b/src/__mocks__/vscode.js @@ -17,6 +17,7 @@ module.exports = { createCommentController: jest.fn(), }, window: { + showInformationMessage: jest.fn(), showWarningMessage: jest.fn(), showErrorMessage: jest.fn(), createStatusBarItem: jest.fn(), diff --git a/src/command_names.ts b/src/command_names.ts index 581d60495ba7218b0272ebf324c7460ddd250a7a..7e95b23b57a1e9b19090060b99530b3a2b4a5259 100644 --- a/src/command_names.ts +++ b/src/command_names.ts @@ -45,4 +45,5 @@ export const PROGRAMMATIC_COMMANDS = { export const VS_COMMANDS = { DIFF: 'vscode.diff', OPEN: 'vscode.open', + GIT_SHOW_OUTPUT: 'git.showOutput', }; diff --git a/src/commands/checkout_mr_branch.test.ts b/src/commands/checkout_mr_branch.test.ts index 8ada818136334d9d6c3fba89673aa7869c73449a..b8a30248750d24ba0595095ebe6fd745e37979b5 100644 --- a/src/commands/checkout_mr_branch.test.ts +++ b/src/commands/checkout_mr_branch.test.ts @@ -1,100 +1,96 @@ import * as vscode from 'vscode'; -import * as sidebar from '../sidebar.js'; -import { GitExtension, Repository } from '../api/git'; import { MrItemModel } from '../data_providers/items/mr_item_model'; -import { anotherWorkspace, mr, workspace } from '../test_utils/entities'; -import { createFakeRepository, FakeGitExtension } from '../test_utils/fake_git_extension'; import { checkoutMrBranch } from './checkout_mr_branch'; -import { gitExtensionWrapper } from '../git/git_extension_wrapper'; +import { WrappedRepository } from '../git/wrapped_repository'; +import { mr } from '../test_utils/entities'; +import { GitErrorCodes } from '../api/git'; + +describe('checkout MR branch', () => { + let mrItemModel: MrItemModel; + + let wrappedRepository: WrappedRepository; + + beforeEach(() => { + const mockRepository: Partial = { + fetch: jest.fn().mockResolvedValue(undefined), + checkout: jest.fn().mockResolvedValue(undefined), + lastCommitSha: mr.sha, + }; + wrappedRepository = mockRepository as WrappedRepository; + }); -jest.mock('../sidebar.js'); -vscode.window.showInformationMessage = jest.fn(); -vscode.window.showErrorMessage = jest.fn(); + afterEach(() => { + jest.resetAllMocks(); + }); -describe('MR branch commands', () => { - describe('Checkout branch by Merge request', () => { - let commandData: MrItemModel; + describe('with branch from the same project', () => { + beforeEach(() => { + const mrFromTheSameProject = { + ...mr, + source_project_id: 123, + target_project_id: 123, + source_branch_name: 'feature-a', + }; + mrItemModel = new MrItemModel(mrFromTheSameProject, wrappedRepository); + }); - let fakeExtension: FakeGitExtension; + it('checks out the local branch', async () => { + await checkoutMrBranch(mrItemModel); - let firstWorkspace: GitLabWorkspace; - let secondWorkspace: GitLabWorkspace; + expect(wrappedRepository.fetch).toBeCalled(); + expect(wrappedRepository.checkout).toBeCalledWith('feature-a'); + }); - let firstRepository: Repository; - let secondRepository: Repository; + it('shows a success message', async () => { + await checkoutMrBranch(mrItemModel); - beforeEach(() => { - firstWorkspace = { ...workspace }; - secondWorkspace = { ...anotherWorkspace }; + expect(vscode.window.showInformationMessage).toBeCalledWith('Branch changed to feature-a'); }); - afterEach(() => { - (vscode.window.showInformationMessage as jest.Mock).mockReset(); - (vscode.window.showWarningMessage as jest.Mock).mockReset(); + it('rejects with an error if error occurred', async () => { + (wrappedRepository.checkout as jest.Mock).mockRejectedValue(new Error('error')); + + await expect(checkoutMrBranch(mrItemModel)).rejects.toEqual(new Error('error')); }); - describe('If merge request from local branch', () => { - describe('Basic functionality', () => { - beforeEach(() => { - firstRepository = createFakeRepository(firstWorkspace.uri); - secondRepository = createFakeRepository(secondWorkspace.uri); - - fakeExtension = new FakeGitExtension(firstRepository, secondRepository); - jest - .spyOn(gitExtensionWrapper, 'Extension', 'get') - .mockReturnValue((fakeExtension as unknown) as GitExtension); - - commandData = new MrItemModel(mr, firstWorkspace); - - checkoutByMRFromLocalBranch(commandData); - }); - - it('(local-branch) Branch fetch message', () => { - expect((vscode.window.showInformationMessage as jest.Mock).mock.calls[0]).toEqual([ - 'Fetching branches...', - ]); - }); - - it('(local-branch) Was checkout', async () => { - await expect(firstRepository.checkout).toBeCalled(); - }); - - it('(local-branch) Was fetching before checkout', async () => { - await expect(firstRepository.checkout).toBeCalled(); - expect(firstRepository.fetch).toBeCalled(); - }); - - it('(local-branch) There were no error messages', () => { - expect(vscode.window.showErrorMessage).not.toBeCalled(); - }); - - it('(local-branch) Sidebar was refreshed', () => { - expect(sidebar.refresh).toBeCalled(); - }); - - it('(local-branch) Message about success', () => { - const callsCount = (vscode.window.showInformationMessage as jest.Mock).mock.calls.length; - expect( - (vscode.window.showInformationMessage as jest.Mock).mock.calls[callsCount - 1], - ).toEqual([`Branch successfully changed to ${mr.source_branch}`]); - }); + it('handles errors from the Git Extension', async () => { + (wrappedRepository.checkout as jest.Mock).mockRejectedValue({ + gitErrorCode: GitErrorCodes.DirtyWorkTree, + stderr: 'Git standard output', }); - describe('Multi-root Workspaces', () => { - beforeEach(() => { - commandData = new MrItemModel(mr, secondWorkspace); - - checkoutByMRFromLocalBranch(commandData); - }); - - it('(multi-root) The branch was checkout from right repository', async () => { - await expect(secondRepository.checkout).toBeCalled(); - }); - - it('(multi-root) The branch from second repository was not checkout', async () => { - await expect(secondRepository.checkout).toBeCalled(); - expect(firstRepository.fetch).not.toBeCalled(); - expect(firstRepository.checkout).not.toBeCalled(); - }); + + await checkoutMrBranch(mrItemModel); + + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + 'Checkout failed: Git standard output', + 'See Git Log', + ); + }); + + it('warns user that their local branch is not in sync', async () => { + (wrappedRepository as any).lastCommitSha = 'abdef'; // simulate local sha being different from mr.sha + + await checkoutMrBranch(mrItemModel); + + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith( + "Branch changed to feature-a, but it's out of sync with the remote branch. Synchronize it by pushing or pulling.", + ); + }); + }); + + describe('with branch from a forked project', () => { + beforeEach(() => { + const mrFromAFork = { + ...mr, + source_project_id: 123, + target_project_id: 456, + source_branch_name: 'feature-a', + }; + mrItemModel = new MrItemModel(mrFromAFork, wrappedRepository); + }); + it('throws an error', async () => { + await expect(checkoutMrBranch(mrItemModel)).rejects.toMatchObject({ + message: 'this command is only available for same-project MRs', }); }); }); diff --git a/src/commands/checkout_mr_branch.ts b/src/commands/checkout_mr_branch.ts index a1d9d79f57e0a53af7fa36b0a17f175eadfa0005..b578f767e316cb161b94a504a218b31128345e98 100644 --- a/src/commands/checkout_mr_branch.ts +++ b/src/commands/checkout_mr_branch.ts @@ -1,56 +1,42 @@ import * as vscode from 'vscode'; import * as assert from 'assert'; -import * as sidebar from '../sidebar.js'; import { MrItemModel } from '../data_providers/items/mr_item_model'; -import { gitExtensionWrapper } from '../git/git_extension_wrapper'; -import { UserFriendlyError } from '../errors/user_friendly_error'; -import { handleError } from '../log'; +import { VS_COMMANDS } from '../command_names'; +import { doNotAwait } from '../utils/do_not_await'; +const handleGitError = async (e: { stderr: string }) => { + const SEE_GIT_LOG = 'See Git Log'; + const choice = await vscode.window.showErrorMessage(`Checkout failed: ${e.stderr}`, SEE_GIT_LOG); + if (choice === SEE_GIT_LOG) { + await vscode.commands.executeCommand(VS_COMMANDS.GIT_SHOW_OUTPUT); + } +}; /** - * Command will checkout source branch by merge request in current git. Merge request must be from local branch. - * @param data item of view from the left sidebar + * Command will checkout source branch for merge request. Merge request must be from local branch. */ -export const checkoutMrBranch = async (data: MrItemModel): Promise => { - // todo: Change data.workspace to data.repository (issue #345) - assert(data.mr.target_project_id === data.mr.source_project_id); - const { showInformationMessage } = vscode.window; - const sourceBranchName = data.mr.source_branch as string; - let branchNameForCheckout: string; +export const checkoutMrBranch = async (mrItemModel: MrItemModel): Promise => { + const { mr } = mrItemModel; + assert( + mr.target_project_id === mr.source_project_id, + 'this command is only available for same-project MRs', + ); try { - const repos = gitExtensionWrapper.getRepositoriesByWorkspace(data.workspace); - if (repos.length > 1) { - throw new Error( - `You have more then one repos in one workspace. Extension can't work with this case yet. But we will fix it on soon.`, + const { repository } = mrItemModel; + doNotAwait(vscode.window.showInformationMessage('Fetching branches...')); + await repository.fetch(); + await repository.checkout(mr.source_branch); + if (repository.lastCommitSha !== mr.sha) { + await vscode.window.showWarningMessage( + `Branch changed to ${mr.source_branch}, but it's out of sync with the remote branch. Synchronize it by pushing or pulling.`, ); + return; } - const repo = repos[0]; - showInformationMessage('Fetching branches...'); - - // merge from local branch - branchNameForCheckout = sourceBranchName; - await repo.fetch(); - await repo.checkout(sourceBranchName); - - assert( - repo.state.HEAD, - "We can't read repository HEAD. We suspect that your `git head` command fails and we can't continue till it succeeds", - ); - - const currentBranchName = repo.state.HEAD.name; - if (currentBranchName !== branchNameForCheckout) { - throw new Error( - `The branch name after the checkout (${currentBranchName}) is not the branch that the extension tried to check out (${branchNameForCheckout}). This is an unexpected error, please inspect your repository before making any further changes.`, - ); - } - - sidebar.refresh(); - showInformationMessage(`Branch successfully changed to ${sourceBranchName}`); + await vscode.window.showInformationMessage(`Branch changed to ${mr.source_branch}`); } catch (e) { - handleError( - new UserFriendlyError( - e.stderr || `${(e as Error).message}` || `Couldn't checkout branch ${sourceBranchName}`, - e, - ), - ); + if (e.gitErrorCode) { + await handleGitError(e); + return; + } + throw e; } }; diff --git a/src/git/wrapped_repository.ts b/src/git/wrapped_repository.ts index 40c0b388aebd459af89c11d3769cd2de72785d17..c2cfa7fbad61fb5088b5a4815467ddc9c106335d 100644 --- a/src/git/wrapped_repository.ts +++ b/src/git/wrapped_repository.ts @@ -79,6 +79,25 @@ export class WrappedRepository { return preferredRemote || branchRemote || firstRemote || 'origin'; } + async fetch(): Promise { + await this.rawRepository.fetch(); + } + + async checkout(branchName: string): Promise { + await this.rawRepository.checkout(branchName); + + assert( + this.rawRepository.state.HEAD, + "We can't read repository HEAD. We suspect that your `git head` command fails and we can't continue till it succeeds", + ); + + const currentBranchName = this.rawRepository.state.HEAD.name; + assert( + currentBranchName === branchName, + `The branch name after the checkout (${currentBranchName}) is not the branch that the extension tried to check out (${branchName}). Inspect your repository before making any more changes.`, + ); + } + getRemoteByName(remoteName: string): GitRemote { const remoteUrl = this.rawRepository.state.remotes.find(r => r.name === remoteName)?.fetchUrl; assert(remoteUrl, `could not find any URL for git remote with name '${this.remoteName}'`); diff --git a/src/test_utils/entities.ts b/src/test_utils/entities.ts index c87c89e64245c3da020ed431d0cd4055d3322b93..ca9f76f673cf012cb1fc5c86dfab9b0199582727 100644 --- a/src/test_utils/entities.ts +++ b/src/test_utils/entities.ts @@ -33,7 +33,6 @@ export const mr: RestMr = { source_project_id: 9999, target_project_id: 9999, source_branch: 'feature-a', - target_branch: 'main', }; export const diffFile: RestDiffFile = { diff --git a/src/types.d.ts b/src/types.d.ts index b2ff7056d5256e644d2644987d51af648a645f6f..a129ba16611cba7a2a413bb7b6d6f5dcc680a424 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -25,7 +25,6 @@ interface RestMr extends RestIssuable { sha: string; source_project_id: number; target_project_id: number; - target_branch: string; source_branch: string; } diff --git a/src/utils/do_not_await.ts b/src/utils/do_not_await.ts new file mode 100644 index 0000000000000000000000000000000000000000..cd9a9683bd8fcc6280dfd28274a91f1c53da7f18 --- /dev/null +++ b/src/utils/do_not_await.ts @@ -0,0 +1,6 @@ +/** doNotAwait is used to circumvent the otherwise invaluable + * @typescript-eslint/no-floating-promises rule. This util is meant + * for informative messages that would otherwise block execution */ +export const doNotAwait = (promise: Thenable): void => { + // not waiting for the promise +};