Find related backports by looking at mergedCommit.oid (#173)

 - Find related backports by looking at `mergedCommit.oid`
 - Improve error reporting by writing the error.message in console and the rest in logs
 - Change `setInterval` to recursive `setTimeout` to avoid flooding with requests (mostly relevant for huge git repos)

src/runWithArgs.ts

@@ -3,6 +3,7 @@ import { runWithOptions } from './runWithOptions';
 import { HandledError } from './services/HandledError';
 import { initLogger } from './services/logger';
 import { getLogfilePath } from './services/env';
+import chalk from 'chalk';
 
 export async function runWithArgs(args: string[]) {
   const logger = initLogger();
@@ -14,8 +15,13 @@ export async function runWithArgs(args: string[]) {
     if (e instanceof HandledError) {
       console.error(e.message);
     } else {
+      console.error('\n');
+      console.error(chalk.bold('⚠️  An unknown error occurred  ⚠️'));
+      console.error(e.message);
       console.error(
-        `An unknown error occurred. Please check the logs for addtional details: ${getLogfilePath()}`
+        chalk.italic(
+          `Please check the logs for addtional details: ${getLogfilePath()}`
+        )
       );
       logger.info('Unknown error:');
       logger.info(e);

src/runWithOptions.ts

@@ -25,7 +25,6 @@ export async function runWithOptions(options: BackportOptions) {
       if (e instanceof HandledError) {
         console.error(e.message);
       } else {
-        console.error(e);
         throw e;
       }
     }

src/services/git.ts

@@ -77,7 +77,7 @@ export async function deleteRemote(
   try {
     await exec(`git remote rm ${remoteName}`, { cwd: getRepoPath(options) });
   } catch (e) {
-    const isExecError = e.code === 128;
+    const isExecError = e.cmd && e.code === 128;
     // re-throw if error is not an exec related error
     if (!isExecError) {
       throw e;
@@ -111,7 +111,7 @@ export async function cherrypickContinue(options: BackportOptions) {
       cwd: getRepoPath(options),
     });
   } catch (e) {
-    const isCherrypickError = e.code === 128;
+    const isCherrypickError = e.cmd && e.code === 128;
     if (!isCherrypickError) {
       throw e;
     }
@@ -131,7 +131,7 @@ export async function getFilesWithConflicts(options: BackportOptions) {
 
     return [];
   } catch (e) {
-    const isConflictError = e.code === 2;
+    const isConflictError = e.cmd && e.code === 2;
     if (isConflictError) {
       const files = (e.stdout as string)
         .split('\n')

src/services/github/__snapshots__/fetchCommitsByAuthor.test.ts.snap

@@ -60,6 +60,9 @@ Array [
                             name
                           }
                           number
+                          mergeCommit {
+                            oid
+                          }
                           timelineItems(
                             last: 20
                             itemTypes: CROSS_REFERENCED_EVENT

src/services/github/fetchCommitsByAuthor.test.ts

@@ -4,7 +4,7 @@ import {
   getExistingBackportPRs,
 } from './fetchCommitsByAuthor';
 import { commitsWithPullRequestsMock } from './mocks/commitsByAuthorMock';
-import { getExistingBackportPRsMock } from './mocks/getExistingBackportPRsMock';
+import { getCommitsByAuthorMock } from './mocks/getCommitsByAuthorMock';
 import { getDefaultOptions } from '../../test/getDefaultOptions';
 import axios from 'axios';
 import { getPullRequestEdgeMock } from './mocks/getPullRequestEdgeMock';
@@ -185,11 +185,11 @@ async function getExistingBackportsByRepoName(
   repoName1: string,
   repoName2: string
 ) {
-  const existingPrsMock = getExistingBackportPRsMock(repoName1);
+  const commitsMock = getCommitsByAuthorMock(repoName1);
 
   spyOn(axios, 'post').and.returnValues(
     { data: { data: currentUserMock } },
-    { data: { data: existingPrsMock } }
+    { data: { data: commitsMock } }
   );
 
   const options = getDefaultOptions({

src/services/github/fetchCommitsByAuthor.ts

@@ -53,6 +53,9 @@ export async function fetchCommitsByAuthor(
                             name
                           }
                           number
+                          mergeCommit {
+                            oid
+                          }
                           timelineItems(
                             last: 20
                             itemTypes: CROSS_REFERENCED_EVENT
@@ -124,7 +127,8 @@ export async function fetchCommitsByAuthor(
 
     const associatedPullRequest = getAssociatedPullRequest(
       pullRequestEdge,
-      options
+      options,
+      sha
     );
 
     const existingBackports = getExistingBackportPRs(
@@ -161,11 +165,13 @@ function getPullNumberFromMessage(firstMessageLine: string) {
 
 function getAssociatedPullRequest(
   pullRequestEdge: PullRequestEdge | undefined,
-  options: BackportOptions
+  options: BackportOptions,
+  sha: string
 ) {
   const isAssociated =
     pullRequestEdge?.node.repository.name === options.repoName &&
-    pullRequestEdge?.node.repository.owner.login === options.repoOwner;
+    pullRequestEdge?.node.repository.owner.login === options.repoOwner &&
+    pullRequestEdge?.node.mergeCommit.oid === sha;
 
   if (isAssociated) {
     return pullRequestEdge;
@@ -250,6 +256,9 @@ interface HistoryEdge {
 export interface PullRequestEdge {
   node: {
     number: number;
+    mergeCommit: {
+      oid: string;
+    };
     repository: {
       owner: {
         login: string;

src/services/github/mocks/commitsByAuthorMock.ts

@@ -23,6 +23,9 @@ export const commitsWithPullRequestsMock: DataResponse = {
                   edges: [
                     {
                       node: {
+                        mergeCommit: {
+                          oid: 'f3b618b9421fdecdb36862f907afbdd6344b361d',
+                        },
                         repository: {
                           name: 'kibana',
                           owner: {
@@ -48,6 +51,9 @@ export const commitsWithPullRequestsMock: DataResponse = {
                   edges: [
                     {
                       node: {
+                        mergeCommit: {
+                          oid: '79cf18453ec32a4677009dcbab1c9c8c73fc14fe',
+                        },
                         repository: {
                           name: 'kibana',
                           owner: {

src/services/github/mocks/getExistingBackportPRsMock.ts → src/services/github/mocks/getCommitsByAuthorMock.ts

@@ -1,6 +1,6 @@
 import { DataResponse } from '../fetchCommitsByAuthor';
 
-export const getExistingBackportPRsMock = (repoName: string): DataResponse => ({
+export const getCommitsByAuthorMock = (repoName: string): DataResponse => ({
   repository: {
     ref: {
       target: {
@@ -15,6 +15,9 @@ export const getExistingBackportPRsMock = (repoName: string): DataResponse => ({
                   edges: [
                     {
                       node: {
+                        mergeCommit: {
+                          oid: '79cf18453ec32a4677009dcbab1c9c8c73fc14fe',
+                        },
                         repository: {
                           name: repoName,
                           owner: {

src/services/github/mocks/getPullRequestEdgeMock.ts

@@ -12,6 +12,9 @@ export function getPullRequestEdgeMock({
 }): PullRequestEdge {
   return {
     node: {
+      mergeCommit: {
+        oid: 'f3b618b9421fdecdb36862f907afbdd6344b361d',
+      },
       repository: {
         name: 'kibana',
         owner: {

src/services/logger.ts

@@ -38,7 +38,17 @@ export function initLogger() {
 }
 
 function formatMessage(message: string | Record<any, any>) {
-  return isString(message) ? message : JSON.stringify(message, null, 2);
+  if (message == null) {
+    return '';
+  }
+
+  if (isString(message)) {
+    return message;
+  }
+
+  if (typeof message === 'object') {
+    return JSON.stringify(message, Object.getOwnPropertyNames(message), 2);
+  }
 }
 
 // log levels:

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

@@ -49,6 +49,9 @@ Object {
                             name
                           }
                           number
+                          mergeCommit {
+                            oid
+                          }
                           timelineItems(
                             last: 20
                             itemTypes: CROSS_REFERENCED_EVENT
@@ -178,6 +181,9 @@ Object {
                             name
                           }
                           number
+                          mergeCommit {
+                            oid
+                          }
                           timelineItems(
                             last: 20
                             itemTypes: CROSS_REFERENCED_EVENT

src/ui/cherrypickAndCreatePullRequest.test.ts

@@ -200,7 +200,7 @@ describe('cherrypickAndCreatePullRequest', () => {
         sourceBranch: 'myDefaultRepoBaseBranch',
       } as BackportOptions;
 
-      const res = await runTimers(() =>
+      const res = await runTimersUntilResolved(() =>
         cherrypickAndCreatePullRequest({
           options,
           commits: [
@@ -291,7 +291,9 @@ function setupExecSpy() {
         if (conflictCheckCounts >= 4) {
           return {};
         }
-        const e = new Error('cherrypick failed');
+        const e = new Error('Not all conflicts resolved');
+        // @ts-ignore
+        e.cmd = cmd;
         // @ts-ignore
         e.code = 2;
         // @ts-ignore
@@ -330,13 +332,23 @@ function setupExecSpy() {
     }) as typeof childProcess.exec);
 }
 
-async function runTimers(fn: () => Promise<any>) {
+/*
+ * Run timers (setInterval/setTimeout) every tick continuously until the promise has been resolved
+ */
+async function runTimersUntilResolved(fn: () => Promise<any>) {
   jest.useFakeTimers();
 
+  let isResolved = false;
   const p = fn();
-  await new Promise((resolve) => setImmediate(resolve));
-  jest.advanceTimersByTime(1000);
-  jest.runOnlyPendingTimers();
+  p.finally(() => (isResolved = true));
+
+  while (isResolved === false) {
+    // tick
+    await new Promise((resolve) => setImmediate(resolve));
+
+    // run timers
+    jest.runAllTimers();
+  }
 
   return p;
 }

src/ui/cherrypickAndCreatePullRequest.ts

@@ -96,19 +96,24 @@ async function waitForCherrypick(
   options: BackportOptions,
   commit: CommitSelected
 ) {
-  const spinner = ora(
+  const cherrypickSpinner = ora(
     `Cherry-picking commit ${getShortSha(commit.sha)}`
   ).start();
   try {
     await cherrypick(options, commit);
-    spinner.succeed();
+    cherrypickSpinner.succeed();
   } catch (e) {
+    cherrypickSpinner.fail();
+
     const filesWithConflicts = await getFilesWithConflicts(options);
     if (isEmpty(filesWithConflicts)) {
       throw e;
     }
 
-    spinner.fail();
+    /*
+     * Conflict resolution phase starts here...
+     */
+
     if (options.editor) {
       const repoPath = getRepoPath(options);
       await exec(`${options.editor} ${repoPath}`);
@@ -120,13 +125,23 @@ async function waitForCherrypick(
     // list unstaged files and require user to confirm adding+comitting them
     await waitForEnterAndListUnstaged(options);
 
-    // add unstaged files
-    await addUnstagedFiles(options);
+    /*
+     * Conflicts resolved and cherrypicking can be continued...
+     */
+
+    const stagingSpinner = ora(`Staging and committing files`).start();
+    try {
+      // add unstaged files
+      await addUnstagedFiles(options);
 
-    // continue cherrypick (similar to commit)
-    await cherrypickContinue(options);
+      // continue cherrypick (similar to `git commit`)
+      await cherrypickContinue(options);
+    } catch (e) {
+      stagingSpinner.fail();
+      throw e;
+    }
 
-    ora(`Staging and committing files`).start().succeed();
+    stagingSpinner.succeed();
   }
 }
 
@@ -138,19 +153,18 @@ async function waitForConflictsToBeResolved(options: BackportOptions) {
     const checkForConflicts = async () => {
       const filesWithConflicts = await getFilesWithConflicts(options);
       if (isEmpty(filesWithConflicts)) {
-        clearInterval(intervalId);
         resolve();
         spinner.succeed(spinnerText);
       } else {
         spinner.text = dedent(`${spinnerText}
 
-        Resolve the conflicts the following files and then return here. You do not need to \`git add\` or \`git commit\`:
+        Resolve the conflicts in the following files and then return here. You do not need to \`git add\` or \`git commit\`:
         ${filesWithConflicts.join('\n')}`);
+        setTimeout(checkForConflicts, 1000);
       }
     };
 
     checkForConflicts();
-    const intervalId = setInterval(checkForConflicts, 1000);
   });
 }