Fix tests

src/services/child-process-promisified.ts

@@ -3,17 +3,16 @@ import { promisify } from 'util';
 import { logger } from './logger';
 
 export async function exec(cmd: string, options: child_process.ExecOptions) {
-  logger.info(`exec: \`${cmd}\``);
   const execPromisified = promisify(child_process.exec);
   try {
     const res = await execPromisified(cmd, {
       maxBuffer: 100 * 1024 * 1024,
       ...options,
     });
-    logger.verbose(`exec response (success):`, res);
+    logger.verbose(`exec success '${cmd}':`, res);
     return res;
   } catch (e) {
-    logger.info(`exec response (error):`, e);
+    logger.info(`exec error '${cmd}':`, e);
     throw e;
   }
 }

src/services/git.test.ts

@@ -9,6 +9,11 @@ import {
   cherrypick,
   getFilesWithConflicts,
 } from '../services/git';
+import { ExecError } from '../test/ExecError';
+
+beforeEach(() => {
+  jest.clearAllMocks();
+});
 
 describe('getUnmergedFiles', () => {
   it('should split lines and remove empty', async () => {
@@ -54,7 +59,7 @@ describe('getFilesWithConflicts', () => {
         'conflicting-file.txt:1: leftover conflict marker\nconflicting-file.txt:3: leftover conflict marker\nconflicting-file.txt:5: leftover conflict marker\n',
       stderr: '',
     };
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
 
     const options = {
       repoOwner: 'elastic',
@@ -88,7 +93,7 @@ describe('createFeatureBranch', () => {
       stderr: "fatal: couldn't find remote ref 4.x\n",
     };
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     await expect(
       createFeatureBranch(options, baseBranch, featureBranch)
     ).rejects.toThrowErrorMatchingInlineSnapshot(
@@ -99,7 +104,7 @@ describe('createFeatureBranch', () => {
   it('should rethrow normal error', async () => {
     expect.assertions(1);
     const err = new Error('just a normal error');
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     expect.assertions(1);
 
     await expect(
@@ -125,13 +130,13 @@ describe('deleteRemote', () => {
       stderr: "fatal: No such remote: 'origin'\n",
     };
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     await expect(await deleteRemote(options, remoteName)).toBe(undefined);
   });
 
   it('should rethrow normal error', async () => {
     const err = new Error('just a normal error');
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     expect.assertions(1);
 
     await expect(
@@ -152,29 +157,88 @@ describe('cherrypick', () => {
     sha: 'abcd',
   };
 
-  it('should swallow cherrypick error', async () => {
+  it('should return `needsResolving: false` when no errors are encountered', async () => {
     jest
       .spyOn(childProcess, 'exec')
+
+      // mock git fetch
+      .mockResolvedValueOnce({ stderr: '', stdout: '' })
+
+      // mock cherry pick command
       .mockResolvedValueOnce({ stderr: '', stdout: '' });
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce({
-      killed: false,
-      code: 128,
-      signal: null,
-      cmd: 'git cherry-pick abcd',
-      stdout: '',
-      stderr: '',
+    expect(await cherrypick(options, commit)).toEqual({
+      needsResolving: false,
     });
-    await expect(await cherrypick(options, commit)).toBe(false);
   });
 
-  it('should re-throw other errors', async () => {
-    const err = new Error('non-cherrypick error');
-    jest
+  it('should use mainline option when specified', async () => {
+    const execSpy = jest
       .spyOn(childProcess, 'exec')
+
+      // mock git fetch
+      .mockResolvedValueOnce({ stderr: '', stdout: '' })
+
+      // mock cherry pick command
       .mockResolvedValueOnce({ stderr: '', stdout: '' });
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
+    await cherrypick({ ...options, mainline: 1 }, commit);
+
+    expect(execSpy.mock.calls[1][0]).toBe('git cherry-pick --mainline 1 abcd');
+  });
+
+  it('should return `needsResolving: true` upon cherrypick error', async () => {
+    jest
+      .spyOn(childProcess, 'exec')
+
+      // mock git fetch
+      .mockResolvedValueOnce({ stderr: '', stdout: '' })
+
+      // mock cherry pick command
+      .mockRejectedValueOnce(
+        new ExecError('some message', {
+          killed: false,
+          code: 128,
+          cmd: 'git cherry-pick abcd',
+          stdout: '',
+          stderr: '',
+        })
+      )
+
+      // mock getFilesWithConflicts
+      .mockRejectedValueOnce(
+        new ExecError('some error', {
+          code: 2,
+          cmd: 'git --no-pager diff --check',
+          stdout:
+            'conflicting-file.txt:1: leftover conflict marker\nconflicting-file.txt:3: leftover conflict marker\nconflicting-file.txt:5: leftover conflict marker\n',
+          stderr: '',
+        })
+      )
+
+      // mock getUnmergedFiles
+      .mockResolvedValueOnce({ stdout: '', stderr: '' });
+
+    expect(await cherrypick(options, commit)).toEqual({
+      needsResolving: true,
+    });
+  });
+
+  it('should re-throw non-cherrypick errors', async () => {
+    jest
+      .spyOn(childProcess, 'exec')
+
+      // mock git fetch
+      .mockResolvedValueOnce({ stderr: '', stdout: '' })
+
+      // mock cherry pick command
+      .mockRejectedValueOnce(new Error('non-cherrypick error'))
+
+      // getFilesWithConflicts
+      .mockResolvedValueOnce({ stdout: '', stderr: '' })
+
+      // getUnmergedFiles
+      .mockResolvedValueOnce({ stdout: '', stderr: '' });
 
     await expect(
       cherrypick(options, commit)
@@ -199,13 +263,13 @@ describe('cherrypickContinue', () => {
         'error: no cherry-pick or revert in progress\nfatal: cherry-pick failed\n',
     };
 
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     await expect(await cherrypickContinue(options)).toBe(undefined);
   });
 
   it('should re-throw other errors', async () => {
     const err = new Error('non-cherrypick error');
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
     expect.assertions(1);
 
     await expect(

src/services/git.ts

@@ -1,5 +1,6 @@
 import { resolve as pathResolve } from 'path';
 import del from 'del';
+import isEmpty from 'lodash.isempty';
 import uniq from 'lodash.uniq';
 import { BackportOptions } from '../options/options';
 import { CommitSelected } from '../types/Commit';
@@ -111,15 +112,24 @@ export async function cherrypick(
   const cmd = `git cherry-pick${mainline} ${commit.sha}`;
   try {
     await exec(cmd, { cwd: getRepoPath(options) });
-    return true;
+    return { needsResolving: false };
   } catch (e) {
-    // re-throw unknown errors
-    if (e.cmd !== cmd) {
-      throw e;
+    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 mainline:\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'
+      );
     }
 
-    // handle cherrypick-related error
-    return false;
+    const isCherryPickError = e.cmd === cmd;
+    const hasConflicts = !isEmpty(await getFilesWithConflicts(options));
+    const hasUnmergedFiles = !isEmpty(await getUnmergedFiles(options));
+
+    if (isCherryPickError && (hasConflicts || hasUnmergedFiles)) {
+      return { needsResolving: true };
+    }
+
+    // re-throw error if there are no conflicts to solve
+    throw e;
   }
 }
 

src/test/integration/__snapshots__/integration.test.ts.snap

@@ -430,47 +430,27 @@ Array [
     },
   ],
   Array [
-    "[INFO] exec: \`git remote rm origin\`",
-    undefined,
-  ],
-  Array [
-    "[VERBOSE] exec response (success):",
+    "[VERBOSE] exec success 'git remote rm origin':",
     "",
   ],
   Array [
-    "[INFO] exec: \`git remote rm sqren\`",
-    undefined,
-  ],
-  Array [
-    "[INFO] exec response (error):",
+    "[INFO] exec error 'git remote rm sqren':",
     [Error: Command failed: git remote rm sqren
 fatal: No such remote: 'sqren'
 ],
   ],
   Array [
-    "[INFO] exec: \`git remote add sqren https://myAccessToken@github.com/sqren/backport-demo.git\`",
-    undefined,
-  ],
-  Array [
-    "[VERBOSE] exec response (success):",
+    "[VERBOSE] exec success 'git remote add sqren https://myAccessToken@github.com/sqren/backport-demo.git':",
     "",
   ],
   Array [
-    "[INFO] exec: \`git remote rm elastic\`",
-    undefined,
-  ],
-  Array [
-    "[INFO] exec response (error):",
+    "[INFO] exec error 'git remote rm elastic':",
     [Error: Command failed: git remote rm elastic
 fatal: No such remote: 'elastic'
 ],
   ],
   Array [
-    "[INFO] exec: \`git remote add elastic https://myAccessToken@github.com/elastic/backport-demo.git\`",
-    undefined,
-  ],
-  Array [
-    "[VERBOSE] exec response (success):",
+    "[VERBOSE] exec success 'git remote add elastic https://myAccessToken@github.com/elastic/backport-demo.git':",
     "",
   ],
   Array [
@@ -482,28 +462,16 @@ fatal: No such remote: 'elastic'
     undefined,
   ],
   Array [
-    "[INFO] exec: \`git reset --hard && git clean -d --force && git fetch elastic 6.0 && git checkout -B backport/6.0/pr-85 elastic/6.0 --no-track\`",
-    undefined,
-  ],
-  Array [
-    "[VERBOSE] exec response (success):",
+    "[VERBOSE] exec success 'git reset --hard && git clean -d --force && git fetch elastic 6.0 && git checkout -B backport/6.0/pr-85 elastic/6.0 --no-track':",
     "HEAD is now at <COMMIT HASH> Add 👻
 ",
   ],
   Array [
-    "[INFO] exec: \`git fetch elastic master:master --force\`",
-    undefined,
-  ],
-  Array [
-    "[VERBOSE] exec response (success):",
+    "[VERBOSE] exec success 'git fetch elastic master:master --force':",
     "",
   ],
   Array [
-    "[INFO] exec: \`git cherry-pick f3b618b9421fdecdb36862f907afbdd6344b361d\`",
-    undefined,
-  ],
-  Array [
-    "[VERBOSE] exec response (success):",
+    "[VERBOSE] exec success 'git cherry-pick f3b618b9421fdecdb36862f907afbdd6344b361d':",
     "[backport/6.0/pr-85 <COMMIT HASH>] Add witch (#85)
  Date: Thu Apr 11 00:46:08 2019 +0200
  1 file changed, 1 insertion(+), 1 deletion(-)
@@ -514,19 +482,11 @@ fatal: No such remote: 'elastic'
     undefined,
   ],
   Array [
-    "[INFO] exec: \`git push sqren backport/6.0/pr-85:backport/6.0/pr-85 --force\`",
-    undefined,
-  ],
-  Array [
-    "[VERBOSE] exec response (success):",
+    "[VERBOSE] exec success 'git push sqren backport/6.0/pr-85:backport/6.0/pr-85 --force':",
     "",
   ],
   Array [
-    "[INFO] exec: \`git checkout master && git branch -D backport/6.0/pr-85\`",
-    undefined,
-  ],
-  Array [
-    "[VERBOSE] exec response (success):",
+    "[VERBOSE] exec success 'git checkout master && git branch -D backport/6.0/pr-85':",
     "Deleted branch backport/6.0/pr-85 (was <COMMIT HASH>).
 ",
   ],

src/ui/__snapshots__/cherrypickAndCreatePullRequest.test.ts.snap

@@ -26,6 +26,12 @@ Array [
       "cwd": "/myHomeDir/.backport/repositories/elastic/kibana",
     },
   ],
+  Array [
+    "git --no-pager diff --name-only --diff-filter=U",
+    Object {
+      "cwd": "/myHomeDir/.backport/repositories/elastic/kibana",
+    },
+  ],
   Array [
     "git --no-pager diff --check",
     Object {

src/ui/cherrypickAndCreatePullRequest.test.ts

@@ -237,14 +237,6 @@ describe('cherrypickAndCreatePullRequest', () => {
 
         You do not need to \`git add\` or \`git commit\` the files - simply fix the conflicts.
 
-        Press ENTER to continue",
-          ],
-          Array [
-            "The following files have conflicts:
-         - /myHomeDir/.backport/repositories/elastic/kibana/conflicting-file.txt
-
-        You do not need to \`git add\` or \`git commit\` the files - simply fix the conflicts.
-
         Press ENTER to continue",
           ],
           Array [
@@ -276,9 +268,6 @@ describe('cherrypickAndCreatePullRequest', () => {
           Array [
             "",
           ],
-          Array [
-            "",
-          ],
         ]
       `);
       expect((ora as any).mock.calls).toMatchInlineSnapshot(`
@@ -327,7 +316,7 @@ function setupExecSpy() {
         throw new ExecError('cherrypick failed', { cmd });
       }
 
-      // filesWithConflicts
+      // getFilesWithConflicts
       if (cmd === 'git --no-pager diff --check') {
         conflictCheckCounts++;
         if (conflictCheckCounts >= 4) {

src/ui/cherrypickAndCreatePullRequest.ts

@@ -101,8 +101,8 @@ async function waitForCherrypick(
   ).start();
 
   try {
-    const didCherrypick = await cherrypick(options, commit);
-    if (didCherrypick) {
+    const { needsResolving } = await cherrypick(options, commit);
+    if (!needsResolving) {
       cherrypickSpinner.succeed();
       return;
     }