Merge 4a66b41 into 83c0cf2

.gitignore

@@ -4,4 +4,5 @@
 /dist
 /yarn-error.log
 .DS_Store
-/test/integration/mock-environment
+/src/test/integration/mock-environment
+

.vscode/settings.json

@@ -7,5 +7,6 @@
   "editor.formatOnSave": true,
   "editor.codeActionsOnSave": {
     "source.fixAll.eslint": true
-  }
+  },
+  "typescript.tsdk": "node_modules/typescript/lib"
 }

README.md

@@ -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                                              |                                |         |

src/__snapshots__/runWithOptions.test.ts.snap

@@ -86,7 +86,7 @@ Array [
         "choices": Array [
           Object {
             "name": "1. Add 👻 (2e63475c) ",
-            "short": "Add 👻 (2e63475c)",
+            "short": "2e63475c",
             "value": Object {
               "existingBackports": Array [],
               "formattedMessage": "Add 👻 (2e63475c)",
@@ -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)",
@@ -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 {
@@ -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)",
@@ -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)",

src/options/cliArgs.test.ts

@@ -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,

src/options/cliArgs.ts

@@ -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',

src/options/config/config.ts

@@ -20,7 +20,7 @@ export async function getOptionsFromConfigFiles() {
 
   return {
     // defaults
-    backportCreatedLabels: [] as string[],
+    backportCreatedLabels: [] as string[] | never[],
     fork: true,
     multiple: false,
     multipleCommits: false,

src/options/options.test.ts

@@ -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' },
@@ -109,6 +110,7 @@ describe('validateRequiredOptions', () => {
     githubApiBaseUrlV4: 'https://api.github.com/graphql',
     author: undefined,
     backportCreatedLabels: [],
+    dryRun: false,
     targetBranchChoices: [],
     targetBranches: ['branchA'],
     commitsCount: 10,

src/runWithOptions.test.ts

@@ -27,6 +27,7 @@ describe('runWithOptions', () => {
       githubApiBaseUrlV4: 'https://api.github.com/graphql',
       author: 'sqren',
       backportCreatedLabels: [],
+      dryRun: false,
       targetBranches: [],
       targetBranchChoices: [
         { name: '6.x' },

src/runWithOptions.ts

@@ -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 {
@@ -24,7 +26,6 @@ export async function runWithOptions(options: BackportOptions) {
         commits,
         targetBranch,
       });
-      backportSucceeded = true;
     } catch (e) {
       if (e instanceof HandledError) {
         consoleLog(e.message);
@@ -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
-              );
-            }
-          );
-        }
-      })
-    );
-  }
 }

src/services/git.test.ts

@@ -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
 
@@ -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')

src/services/git.ts

@@ -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> {
@@ -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`
       );
     }
 
@@ -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();
@@ -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;
+  }
 }

src/services/github/v3/addLabelsToPullRequest.ts

@@ -1,3 +1,4 @@
+import ora from 'ora';
 import { BackportOptions } from '../../../options/options';
 import { logger } from '../../logger';
 import { apiRequestV3 } from './apiRequestV3';
@@ -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;
+  }
 }