Merge ceb9f7a into 62a1052

package.json

@@ -63,6 +63,7 @@
     "node": ">=8.0.0"
   },
   "dependencies": {
+    "@types/safe-json-stringify": "^1.1.0",
     "axios": "^0.19.2",
     "dedent": "^0.7.0",
     "del": "^5.1.0",
@@ -73,6 +74,7 @@
     "lodash.uniq": "^4.5.0",
     "make-dir": "^3.0.2",
     "ora": "^4.0.3",
+    "safe-json-stringify": "^1.2.0",
     "strip-json-comments": "^3.0.1",
     "winston": "^3.2.1",
     "yargs": "^15.3.1"

src/__snapshots__/runWithOptions.test.ts.snap

@@ -39,7 +39,13 @@ Array [
     },
   ],
   Array [
-    "git fetch elastic mySourceBranch:mySourceBranch --force && git cherry-pick 2e63475c483f7844b0f2833bc57fdee32095bacb",
+    "git fetch elastic mySourceBranch:mySourceBranch --force",
+    Object {
+      "cwd": "/myHomeDir/.backport/repositories/elastic/kibana",
+    },
+  ],
+  Array [
+    "git cherry-pick 2e63475c483f7844b0f2833bc57fdee32095bacb",
     Object {
       "cwd": "/myHomeDir/.backport/repositories/elastic/kibana",
     },

src/runWithArgs.ts

@@ -23,9 +23,7 @@ export async function runWithArgs(args: string[]) {
           `Please check the logs for addtional details: ${getLogfilePath()}`
         )
       );
-      logger.info('Unknown error:');
-      logger.info(e);
-      logger.info(e.stack);
+      logger.info('Unknown error:', e);
     }
 
     // wait exiting until logs have been flushed to disc

src/runWithOptions.test.ts

@@ -142,7 +142,7 @@ describe('runWithOptions', () => {
   });
 
   it('exec should be called with correct args', () => {
-    expect(rpcExecMock).toHaveBeenCalledTimes(9);
+    expect(rpcExecMock).toHaveBeenCalledTimes(10);
     expect(rpcExecMock.mock.calls).toMatchSnapshot();
   });
 

src/services/child-process-promisified.ts

@@ -2,11 +2,16 @@ import child_process from 'child_process';
 import { promisify } from 'util';
 import { logger } from './logger';
 
-export const exec = (cmd: string, options: child_process.ExecOptions = {}) => {
-  logger.info(`exec: ${cmd}`);
+export async function exec(cmd: string, options: child_process.ExecOptions) {
+  logger.info(`exec cmd: ${cmd}`);
   const execPromisified = promisify(child_process.exec);
-  return execPromisified(cmd, { maxBuffer: 100 * 1024 * 1024, ...options });
-};
+  const res = await execPromisified(cmd, {
+    maxBuffer: 100 * 1024 * 1024,
+    ...options,
+  });
+  logger.verbose(`exec result`, res);
+  return res;
+}
 
 export const execAsCallback = (
   ...args: Parameters<typeof child_process.exec>

src/services/git.test.ts

@@ -1,57 +1,44 @@
 import { BackportOptions } from '../options/options';
 import {
   addRemote,
-  getUnstagedFiles,
-  getFilesWithConflicts,
+  getUnmergedFiles,
   cherrypickContinue,
   deleteRemote,
   createFeatureBranch,
+  cherrypick,
 } from '../services/git';
 import * as childProcess from '../services/child-process-promisified';
-import { PromiseReturnType } from '../types/PromiseReturnType';
 
-type ExecReturnType = PromiseReturnType<typeof childProcess.exec>;
-
-describe('getUnstagedFiles', () => {
-  it('should split by linebreak and remove empty items', async () => {
-    const stdout = `add 'conflicting-file.txt'\nadd 'another-conflicting-file.js'\n`;
-    jest
-      .spyOn(childProcess, 'exec')
-      .mockResolvedValue({ stdout } as ExecReturnType);
+describe('getUnmergedFiles', () => {
+  it('should split lines and remove empty', async () => {
+    jest.spyOn(childProcess, 'exec').mockResolvedValue({
+      stdout: 'conflicting-file.txt\nconflicting-file2.txt\n',
+      stderr: '',
+    });
 
     const options = {
       repoOwner: 'elastic',
       repoName: 'kibana',
     } as BackportOptions;
 
-    expect(await getUnstagedFiles(options)).toEqual([
+    await expect(await getUnmergedFiles(options)).toEqual([
       ' - /myHomeDir/.backport/repositories/elastic/kibana/conflicting-file.txt',
-      ' - /myHomeDir/.backport/repositories/elastic/kibana/another-conflicting-file.js',
+      ' - /myHomeDir/.backport/repositories/elastic/kibana/conflicting-file2.txt',
     ]);
   });
-});
 
-describe('getFilesWithConflicts', () => {
-  it('should split by linebreak and remove empty and duplicate items', async () => {
-    const err = {
-      killed: false,
-      code: 2,
-      signal: null,
-      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',
+  it('should not error on empty', async () => {
+    jest.spyOn(childProcess, 'exec').mockResolvedValue({
+      stdout: '',
       stderr: '',
-    };
-    jest.spyOn(childProcess, 'exec').mockRejectedValue(err as ExecReturnType);
+    });
 
     const options = {
       repoOwner: 'elastic',
       repoName: 'kibana',
     } as BackportOptions;
 
-    expect(await getFilesWithConflicts(options)).toEqual([
-      ' - /myHomeDir/.backport/repositories/elastic/kibana/conflicting-file.txt',
-    ]);
+    await expect(await getUnmergedFiles(options)).toEqual([]);
   });
 });
 
@@ -64,7 +51,7 @@ describe('createFeatureBranch', () => {
   const baseBranch = '4.x';
   const featureBranch = 'backport/4.x/commit-72f94e76';
 
-  it('should throw HandledError', () => {
+  it('should throw HandledError', async () => {
     expect.assertions(1);
     const err = {
       killed: false,
@@ -77,20 +64,20 @@ describe('createFeatureBranch', () => {
     };
 
     jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
-    expect(
+    await expect(
       createFeatureBranch(options, baseBranch, featureBranch)
     ).rejects.toThrowErrorMatchingInlineSnapshot(
       `"The branch \\"4.x\\" is invalid or doesn't exist"`
     );
   });
 
-  it('should rethrow normal error', () => {
+  it('should rethrow normal error', async () => {
     expect.assertions(1);
     const err = new Error('just a normal error');
     jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
     expect.assertions(1);
 
-    expect(
+    await expect(
       createFeatureBranch(options, baseBranch, featureBranch)
     ).rejects.toThrowErrorMatchingInlineSnapshot(`"just a normal error"`);
   });
@@ -114,20 +101,62 @@ describe('deleteRemote', () => {
     };
 
     jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
-    expect(await deleteRemote(options, remoteName)).toBe(undefined);
+    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);
     expect.assertions(1);
 
-    expect(
+    await expect(
       deleteRemote(options, remoteName)
     ).rejects.toThrowErrorMatchingInlineSnapshot(`"just a normal error"`);
   });
 });
 
+describe('cherrypick', () => {
+  const options = {
+    repoOwner: 'elastic',
+    repoName: 'kibana',
+  } as BackportOptions;
+
+  const commit = {
+    branch: '7.x',
+    formattedMessage: '',
+    sha: 'abcd',
+  };
+
+  it('should swallow cherrypick error', async () => {
+    jest
+      .spyOn(childProcess, 'exec')
+      .mockResolvedValueOnce({ stderr: '', stdout: '' });
+
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce({
+      killed: false,
+      code: 128,
+      signal: null,
+      cmd: 'git cherry-pick abcd',
+      stdout: '',
+      stderr: '',
+    });
+    await expect(await cherrypick(options, commit)).toBe(false);
+  });
+
+  it('should re-throw other errors', async () => {
+    const err = new Error('non-cherrypick error');
+    jest
+      .spyOn(childProcess, 'exec')
+      .mockResolvedValueOnce({ stderr: '', stdout: '' });
+
+    jest.spyOn(childProcess, 'exec').mockRejectedValueOnce(err);
+
+    await expect(
+      cherrypick(options, commit)
+    ).rejects.toThrowErrorMatchingInlineSnapshot(`"non-cherrypick error"`);
+  });
+});
+
 describe('cherrypickContinue', () => {
   const options = {
     repoOwner: 'elastic',
@@ -146,15 +175,15 @@ describe('cherrypickContinue', () => {
     };
 
     jest.spyOn(childProcess, 'exec').mockRejectedValue(err);
-    expect(await cherrypickContinue(options)).toBe(undefined);
+    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);
     expect.assertions(1);
 
-    expect(
+    await expect(
       cherrypickContinue(options)
     ).rejects.toThrowErrorMatchingInlineSnapshot(`"non-cherrypick error"`);
   });
@@ -171,7 +200,7 @@ describe('addRemote', () => {
   it('add correct origin remote', async () => {
     const spy = jest
       .spyOn(childProcess, 'exec')
-      .mockResolvedValue({} as ExecReturnType);
+      .mockResolvedValue({ stderr: '', stdout: '' });
     await addRemote(options, 'elastic');
 
     return expect(
@@ -185,7 +214,7 @@ describe('addRemote', () => {
   it('add correct user remote', async () => {
     const spy = jest
       .spyOn(childProcess, 'exec')
-      .mockResolvedValue({} as ExecReturnType);
+      .mockResolvedValue({ stderr: '', stdout: '' });
     await addRemote(options, 'sqren');
 
     return expect(
@@ -199,7 +228,7 @@ describe('addRemote', () => {
   it('allows custom github url', async () => {
     const spy = jest
       .spyOn(childProcess, 'exec')
-      .mockResolvedValue({} as ExecReturnType);
+      .mockResolvedValue({ stderr: '', stdout: '' });
     await addRemote(
       { ...options, gitHostname: 'github.my-company.com' },
       'sqren'