Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --dry-run option #182

Merged
merged 4 commits into from
Apr 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@
/dist
/yarn-error.log
.DS_Store
/test/integration/mock-environment
/src/test/integration/mock-environment

3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true
}
},
"typescript.tsdk": "node_modules/typescript/lib"
}
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,23 +98,24 @@ The above commands will start an interactive prompt. You can use the `arrow keys
| --accesstoken | Github access token | | string |
| --all | Show commits from other than me | false | boolean |
| --author | Filter commits by author | _Current user_ | string |
| --branch | Branch to backport to | | string |
| --branch | Target branch to backport to | | string |
| --commits-count | Number of commits to choose from | 10 | number |
| --dry-run | Perform backport without pushing to Github | false | boolean |
| --editor | Editor (eg. `code`) to open and solve conflicts | | string |
| --fork | Create backports in fork (true) or origin repo (false) | true | boolean |
| --git-hostname | Hostname for Git remotes | github.com | string |
| --github-api-base-url-v3 | Base url for Github's Rest (v3) API | https://api.github.com | string |
| --github-api-base-url-v4 | Base url for Github's GraphQL (v4) API | https://api.github.com/graphql | string |
| --labels | Pull request labels | | string |
| --mainline | Parent id of merge commit | 1 (when no parent id given) | number |
| --mainline | Parent id of merge commit | 1 | number |
| --multiple | Select multiple commits/branches | false | boolean |
| --path | Only list commits touching files under a specific path | | string |
| --pr-description | Pull request description suffix | | string |
| --pr-title | Pull request title pattern | | string |
| --pr | Pull request to backport | | number |
| --reset-author | Set yourself as commit author | | boolean |
| --sha | Sha of commit to backport | | string |
| --sourceBranch | Backport commits from a non-default branch | | string |
| --sourceBranch | The branch to source commits from | | string |
| --upstream | Name of organization and repository | | string |
| --username | Github username | | string |
| --help | Show help | | |
Expand Down
10 changes: 5 additions & 5 deletions src/__snapshots__/runWithOptions.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ Array [
"choices": Array [
Object {
"name": "1. Add 👻 (2e63475c) ",
"short": "Add 👻 (2e63475c)",
"short": "2e63475c",
"value": Object {
"existingBackports": Array [],
"formattedMessage": "Add 👻 (2e63475c)",
Expand All @@ -98,7 +98,7 @@ Array [
},
Object {
"name": "2. Add witch (#85) ",
"short": "Add witch (#85)",
"short": "#85 (f3b618b9)",
"value": Object {
"existingBackports": Array [],
"formattedMessage": "Add witch (#85)",
Expand All @@ -110,7 +110,7 @@ Array [
},
Object {
"name": "3. Add SF mention (#80) 6.3",
"short": "Add SF mention (#80)",
"short": "#80 (79cf1845)",
"value": Object {
"existingBackports": Array [
Object {
Expand All @@ -127,7 +127,7 @@ Array [
},
Object {
"name": "4. Add backport config (3827bbba) ",
"short": "Add backport config (3827bbba)",
"short": "3827bbba",
"value": Object {
"existingBackports": Array [],
"formattedMessage": "Add backport config (3827bbba)",
Expand All @@ -139,7 +139,7 @@ Array [
},
Object {
"name": "5. Initial commit (5ea0da55) ",
"short": "Initial commit (5ea0da55)",
"short": "5ea0da55",
"value": Object {
"existingBackports": Array [],
"formattedMessage": "Initial commit (5ea0da55)",
Expand Down
1 change: 1 addition & 0 deletions src/options/cliArgs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ describe('getOptionsFromCliArgs', () => {
githubApiBaseUrlV3: 'https://api.github.com',
githubApiBaseUrlV4: 'https://api.github.com/graphql',
backportCreatedLabels: [],
dryRun: false,
targetBranches: ['6.0', '6.1'],
targetBranchChoices: [],
fork: true,
Expand Down
5 changes: 5 additions & 0 deletions src/options/cliArgs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ export function getOptionsFromCliArgs(
alias: 'count',
type: 'number',
})
.option('dryRun', {
default: false,
description: 'Perform backport without pushing to Github',
type: 'boolean',
})
.option('editor', {
default: configOptions.editor,
description: 'Editor to be opened during conflict resolution',
Expand Down
2 changes: 1 addition & 1 deletion src/options/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export async function getOptionsFromConfigFiles() {

return {
// defaults
backportCreatedLabels: [] as string[],
backportCreatedLabels: [] as string[] | never[],
fork: true,
multiple: false,
multipleCommits: false,
Expand Down
2 changes: 2 additions & 0 deletions src/options/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ describe('getOptions', () => {
githubApiBaseUrlV4: 'https://api.github.com/graphql',
author: 'sqren',
backportCreatedLabels: [],
dryRun: false,
targetBranchChoices: [
{ checked: false, name: '6.0' },
{ checked: false, name: '5.9' },
Expand Down Expand Up @@ -109,6 +110,7 @@ describe('validateRequiredOptions', () => {
githubApiBaseUrlV4: 'https://api.github.com/graphql',
author: undefined,
backportCreatedLabels: [],
dryRun: false,
targetBranchChoices: [],
targetBranches: ['branchA'],
commitsCount: 10,
Expand Down
1 change: 1 addition & 0 deletions src/runWithOptions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('runWithOptions', () => {
githubApiBaseUrlV4: 'https://api.github.com/graphql',
author: 'sqren',
backportCreatedLabels: [],
dryRun: false,
targetBranches: [],
targetBranchChoices: [
{ name: '6.x' },
Expand Down
28 changes: 5 additions & 23 deletions src/runWithOptions.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
import chalk from 'chalk';
import { BackportOptions } from './options/options';
import { HandledError } from './services/HandledError';
import { addLabelsToPullRequest } from './services/github/v3/addLabelsToPullRequest';
import { logger, consoleLog } from './services/logger';
import { sequentially } from './services/sequentially';
import { cherrypickAndCreatePullRequest } from './ui/cherrypickAndCreatePullRequest';
import { getTargetBranches } from './ui/getBranches';
import { getCommits } from './ui/getCommits';
import { maybeSetupRepo } from './ui/maybeSetupRepo';
import { withSpinner } from './ui/withSpinner';

export async function runWithOptions(options: BackportOptions) {
if (options.dryRun) {
consoleLog(chalk.red('Dry run: Nothing will be pushed to Github\n'));
}

const commits = await getCommits(options);
const targetBranches = await getTargetBranches(options, commits);

await maybeSetupRepo(options);

let backportSucceeded = false; // minimum 1 backport PR was successfully created
await sequentially(targetBranches, async (targetBranch) => {
logger.info(`Backporting ${JSON.stringify(commits)} to ${targetBranch}`);
try {
Expand All @@ -24,7 +26,6 @@ export async function runWithOptions(options: BackportOptions) {
commits,
targetBranch,
});
backportSucceeded = true;
} catch (e) {
if (e instanceof HandledError) {
consoleLog(e.message);
Expand All @@ -33,23 +34,4 @@ export async function runWithOptions(options: BackportOptions) {
}
}
});

if (backportSucceeded && options.backportCreatedLabels.length > 0) {
await Promise.all(
commits.map(async ({ pullNumber }) => {
if (pullNumber) {
return withSpinner(
{ text: `Adding labels to #${pullNumber}` },
() => {
return addLabelsToPullRequest(
options,
pullNumber,
options.backportCreatedLabels
);
}
);
}
})
);
}
}
28 changes: 27 additions & 1 deletion src/services/git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ describe('cherrypick', () => {
);

await expect(cherrypick(options, commit)).rejects
.toThrowError(`Failed to cherrypick because the selected commit was a merge. Please try again by specifying the parent with the \`mainline\` argument:
.toThrowError(`Cherrypick failed because the selected commit was a merge commit. Please try again by specifying the parent with the \`mainline\` argument:

> backport --mainline

Expand All @@ -257,6 +257,32 @@ or:
Or refer to the git documentation for more information: https://git-scm.com/docs/git-cherry-pick#Documentation/git-cherry-pick.txt---mainlineparent-number`);
});

it('it should gracefully handle empty commits', async () => {
jest
.spyOn(childProcess, 'exec')

// mock git fetch
.mockResolvedValueOnce({ stderr: '', stdout: '' })

// mock cherry pick command
.mockRejectedValueOnce(
new ExecError({
killed: false,
code: 1,
signal: null,
cmd: 'git cherry-pick fe6b13b83cc010f722548cd5a0a8c2d5341a20dd',
stdout:
'On branch backport/7.x/pr-58692\nYou are currently cherry-picking commit fe6b13b83cc.\n\nnothing to commit, working tree clean\n',
stderr:
"The previous cherry-pick is now empty, possibly due to conflict resolution.\nIf you wish to commit it anyway, use:\n\n git commit --allow-empty\n\nOtherwise, please use 'git cherry-pick --skip'\n",
})
);

await expect(cherrypick(options, commit)).rejects.toThrowError(
`Cherrypick failed because the selected commit (abcd) is empty. This is most likely caused by attemping to backporting a commit that was already backported`
);
});

it('should re-throw non-cherrypick errors', async () => {
jest
.spyOn(childProcess, 'exec')
Expand Down
53 changes: 42 additions & 11 deletions src/services/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ import { resolve as pathResolve } from 'path';
import del from 'del';
import isEmpty from 'lodash.isempty';
import uniq from 'lodash.uniq';
import ora from 'ora';
import { BackportOptions } from '../options/options';
import { CommitSelected } from '../types/Commit';
import { HandledError } from './HandledError';
import { execAsCallback, exec } from './child-process-promisified';
import { getRepoOwnerPath, getRepoPath } from './env';
import { stat } from './fs-promisified';
import { getShortSha } from './github/commitFormatters';
import { logger } from './logger';

async function folderExists(path: string): Promise<boolean> {
Expand Down Expand Up @@ -114,9 +116,18 @@ export async function cherrypick(
await exec(cmd, { cwd: getRepoPath(options) });
return { needsResolving: false };
} catch (e) {
// missing `mainline` option
if (e.message.includes('is a merge but no -m option was given')) {
throw new HandledError(
'Failed to cherrypick because the selected commit was a merge. Please try again by specifying the parent with the `mainline` argument:\n\n> backport --mainline\n\nor:\n\n> backport --mainline <parent-number>\n\nOr refer to the git documentation for more information: https://git-scm.com/docs/git-cherry-pick#Documentation/git-cherry-pick.txt---mainlineparent-number'
'Cherrypick failed because the selected commit was a merge commit. Please try again by specifying the parent with the `mainline` argument:\n\n> backport --mainline\n\nor:\n\n> backport --mainline <parent-number>\n\nOr refer to the git documentation for more information: https://git-scm.com/docs/git-cherry-pick#Documentation/git-cherry-pick.txt---mainlineparent-number'
);
}

// commit was already backported
if (e.message.includes('The previous cherry-pick is now empty')) {
const shortSha = getShortSha(commit.sha);
throw new HandledError(
`Cherrypick failed because the selected commit (${shortSha}) is empty. This is most likely caused by attemping to backporting a commit that was already backported`
);
}

Expand Down Expand Up @@ -163,7 +174,10 @@ export async function getFilesWithConflicts(options: BackportOptions) {
if (isConflictError) {
const files = (e.stdout as string)
.split('\n')
.filter((line: string) => !!line.trim())
.filter(
(line: string) =>
!!line.trim() && !line.startsWith('+') && !line.startsWith('-')
)
.map((line: string) => {
const posSeparator = line.indexOf(':');
const filename = line.slice(0, posSeparator).trim();
Expand Down Expand Up @@ -239,13 +253,30 @@ export function getRemoteName(options: BackportOptions) {
return options.fork ? options.username : options.repoOwner;
}

export function pushFeatureBranch(
options: BackportOptions,
featureBranch: string
) {
const remoteName = getRemoteName(options);
return exec(
`git push ${remoteName} ${featureBranch}:${featureBranch} --force`,
{ cwd: getRepoPath(options) }
);
export function pushFeatureBranch({
options,
featureBranch,
headBranchName,
}: {
options: BackportOptions;
featureBranch: string;
headBranchName: string;
}) {
const spinner = ora(`Pushing branch "${headBranchName}"`).start();

if (options.dryRun) {
spinner.succeed(`Dry run: Pushing branch "${headBranchName}"`);
return exec('true', { cwd: getRepoPath(options) });
}

try {
const remoteName = getRemoteName(options);
return exec(
`git push ${remoteName} ${featureBranch}:${featureBranch} --force`,
{ cwd: getRepoPath(options) }
);
} catch (e) {
spinner.fail();
throw e;
}
}
38 changes: 27 additions & 11 deletions src/services/github/v3/addLabelsToPullRequest.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import ora from 'ora';
import { BackportOptions } from '../../../options/options';
import { logger } from '../../logger';
import { apiRequestV3 } from './apiRequestV3';
Expand All @@ -9,19 +10,34 @@ export async function addLabelsToPullRequest(
repoOwner,
accessToken,
username,
dryRun,
}: BackportOptions,
pullNumber: number,
labels: string[]
) {
logger.info(`Adding label "${labels}" to #${pullNumber}`);
): Promise<void> {
const text = `Adding labels to #${pullNumber}: ${labels.join(', ')}`;
logger.info(text);
const spinner = ora(text).start();

return apiRequestV3({
method: 'post',
url: `${githubApiBaseUrlV3}/repos/${repoOwner}/${repoName}/issues/${pullNumber}/labels`,
data: labels,
auth: {
username: username,
password: accessToken,
},
});
if (dryRun) {
spinner.succeed(`Dry run: ${text}`);
return;
}

try {
await apiRequestV3({
method: 'post',
url: `${githubApiBaseUrlV3}/repos/${repoOwner}/${repoName}/issues/${pullNumber}/labels`,
data: labels,
auth: {
username: username,
password: accessToken,
},
});
spinner.succeed();
return;
} catch (e) {
spinner.fail();
throw e;
}
}
Loading