From 8a87bcd91824acf7fd9ee801ec439d303fc84755 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Fri, 10 Nov 2017 13:07:06 +0100
Subject: [PATCH] feat: rebase onboarding branch (#1139)

Previously, Renovate's "Configure Renovate" onboarding branch would never get rebased after it was created. Now, it will be rebased every time the base branch is updated, unless the list of modified files is more than `renovate.json` alone.

Closes #1111
---
 lib/platform/github/index.js                  | 12 +++++
 lib/platform/gitlab/index.js                  |  6 +++
 .../repository/onboarding/branch/index.js     |  5 ++-
 .../repository/onboarding/branch/rebase.js    | 42 +++++++++++++++++
 .../platform/__snapshots__/index.spec.js.snap |  2 +
 .../github/__snapshots__/index.spec.js.snap   |  7 +++
 test/platform/github/index.spec.js            | 17 +++++++
 test/platform/gitlab/index.spec.js            |  6 +++
 .../onboarding/branch/index.spec.js           | 13 +++++-
 .../onboarding/branch/rebase.spec.js          | 45 +++++++++++++++++++
 10 files changed, 153 insertions(+), 2 deletions(-)
 create mode 100644 lib/workers/repository/onboarding/branch/rebase.js
 create mode 100644 test/workers/repository/onboarding/branch/rebase.spec.js

diff --git a/lib/platform/github/index.js b/lib/platform/github/index.js
index 068dc6fa0a..7a611dbc8d 100644
--- a/lib/platform/github/index.js
+++ b/lib/platform/github/index.js
@@ -30,6 +30,7 @@ module.exports = {
   findPr,
   createPr,
   getPr,
+  getPrFiles,
   updatePr,
   mergePr,
   // file
@@ -579,6 +580,17 @@ async function getPr(prNo) {
   return pr;
 }
 
+// Return a list of all modified files in a PR
+async function getPrFiles(prNo) {
+  logger.debug({ prNo }, 'getPrFiles');
+  if (!prNo) {
+    return [];
+  }
+  const files = (await get(`repos/${config.repoName}/pulls/${prNo}/files`))
+    .body;
+  return files.map(f => f.filename);
+}
+
 async function updatePr(prNo, title, body) {
   logger.debug(`updatePr(${prNo}, ${title}, body)`);
   const patchBody = { title };
diff --git a/lib/platform/gitlab/index.js b/lib/platform/gitlab/index.js
index f4a445f916..dc5b4f2985 100644
--- a/lib/platform/gitlab/index.js
+++ b/lib/platform/gitlab/index.js
@@ -31,6 +31,7 @@ module.exports = {
   findPr,
   createPr,
   getPr,
+  getPrFiles,
   updatePr,
   mergePr,
   // file
@@ -404,6 +405,11 @@ async function getPr(iid) {
   return pr;
 }
 
+function getPrFiles() {
+  // TODO
+  return [];
+}
+
 async function updatePr(iid, title, description) {
   await get.put(`projects/${config.repoName}/merge_requests/${iid}`, {
     body: {
diff --git a/lib/workers/repository/onboarding/branch/index.js b/lib/workers/repository/onboarding/branch/index.js
index 6c820dc832..2967938c61 100644
--- a/lib/workers/repository/onboarding/branch/index.js
+++ b/lib/workers/repository/onboarding/branch/index.js
@@ -1,5 +1,6 @@
 const { detectPackageFiles } = require('../../../../manager');
 const { createOnboardingBranch } = require('./create');
+const { rebaseOnboardingBranch } = require('./rebase');
 const { isOnboarded, onboardingPrExists } = require('./check');
 
 async function checkOnboardingBranch(config) {
@@ -14,7 +15,9 @@ async function checkOnboardingBranch(config) {
     throw new Error('fork');
   }
   logger.info('Repo is not onboarded');
-  if (!await onboardingPrExists(config)) {
+  if (await onboardingPrExists(config)) {
+    await rebaseOnboardingBranch(config);
+  } else {
     if ((await detectPackageFiles(config)).length === 0) {
       throw new Error('no-package-files');
     }
diff --git a/lib/workers/repository/onboarding/branch/rebase.js b/lib/workers/repository/onboarding/branch/rebase.js
new file mode 100644
index 0000000000..64ec98581b
--- /dev/null
+++ b/lib/workers/repository/onboarding/branch/rebase.js
@@ -0,0 +1,42 @@
+async function rebaseOnboardingBranch(config) {
+  logger.debug('Checking if onboarding branch needs rebasing');
+  const prShort = await platform.getBranchPr(`${config.branchPrefix}configure`);
+  const pr = await platform.getPr(prShort.number);
+  if (!pr.isStale) {
+    logger.info('Onboarding branch is up to date');
+    return;
+  }
+  const renovateJson = {
+    extends: ['config:base'],
+  };
+  const onboardingBranch = `${config.branchPrefix}configure`;
+  let contents = JSON.stringify(renovateJson, null, 2) + '\n';
+  if (!pr.canRebase) {
+    const prFiles = await platform.getPrFiles(pr.number);
+    if (prFiles.length !== 1 || prFiles[0] !== 'renovate.json') {
+      logger.info(
+        { prFiles },
+        'Onboarding branch is stale but cannot be rebased'
+      );
+      return;
+    }
+    logger.info('Rebasing modified onboarding branch');
+    contents = await platform.getFile('renovate.json', onboardingBranch);
+  } else {
+    logger.info('Rebasing unmodified onboarding branch');
+  }
+  await platform.commitFilesToBranch(
+    onboardingBranch,
+    [
+      {
+        name: 'renovate.json',
+        contents,
+      },
+    ],
+    'Add renovate.json'
+  );
+}
+
+module.exports = {
+  rebaseOnboardingBranch,
+};
diff --git a/test/platform/__snapshots__/index.spec.js.snap b/test/platform/__snapshots__/index.spec.js.snap
index a639fe6db3..985ec7ba90 100644
--- a/test/platform/__snapshots__/index.spec.js.snap
+++ b/test/platform/__snapshots__/index.spec.js.snap
@@ -23,6 +23,7 @@ Array [
   "findPr",
   "createPr",
   "getPr",
+  "getPrFiles",
   "updatePr",
   "mergePr",
   "commitFilesToBranch",
@@ -54,6 +55,7 @@ Array [
   "findPr",
   "createPr",
   "getPr",
+  "getPrFiles",
   "updatePr",
   "mergePr",
   "commitFilesToBranch",
diff --git a/test/platform/github/__snapshots__/index.spec.js.snap b/test/platform/github/__snapshots__/index.spec.js.snap
index 8534a81715..ffd36a0913 100644
--- a/test/platform/github/__snapshots__/index.spec.js.snap
+++ b/test/platform/github/__snapshots__/index.spec.js.snap
@@ -683,6 +683,13 @@ Object {
 }
 `;
 
+exports[`platform/github getPrFiles() returns files 1`] = `
+Array [
+  "renovate.json",
+  "not renovate.json",
+]
+`;
+
 exports[`platform/github getRepos should return an array of repos 1`] = `
 Array [
   Array [
diff --git a/test/platform/github/index.spec.js b/test/platform/github/index.spec.js
index 38e0c19c6c..4b2372dc72 100644
--- a/test/platform/github/index.spec.js
+++ b/test/platform/github/index.spec.js
@@ -1117,6 +1117,23 @@ describe('platform/github', () => {
       expect(pr).toMatchSnapshot();
     });
   });
+  describe('getPrFiles()', () => {
+    it('should return empty if no prNo is passed', async () => {
+      const prFiles = await github.getPrFiles(null);
+      expect(prFiles).toEqual([]);
+    });
+    it('returns files', async () => {
+      get.mockReturnValueOnce({
+        body: [
+          { filename: 'renovate.json' },
+          { filename: 'not renovate.json' },
+        ],
+      });
+      const prFiles = await github.getPrFiles(123);
+      expect(prFiles).toMatchSnapshot();
+      expect(prFiles).toHaveLength(2);
+    });
+  });
   describe('updatePr(prNo, title, body)', () => {
     it('should update the PR', async () => {
       await initRepo('some/repo', 'token');
diff --git a/test/platform/gitlab/index.spec.js b/test/platform/gitlab/index.spec.js
index 89fef1c575..920b2033a7 100644
--- a/test/platform/gitlab/index.spec.js
+++ b/test/platform/gitlab/index.spec.js
@@ -572,6 +572,12 @@ describe('platform/gitlab', () => {
       expect(pr).toMatchSnapshot();
     });
   });
+  describe('getPrFiles()', () => {
+    it('should return empty', async () => {
+      const prFiles = await gitlab.getPrFiles();
+      expect(prFiles).toEqual([]);
+    });
+  });
   describe('updatePr(prNo, title, body)', () => {
     jest.resetAllMocks();
     it('updates the PR', async () => {
diff --git a/test/workers/repository/onboarding/branch/index.spec.js b/test/workers/repository/onboarding/branch/index.spec.js
index 18d26312d5..2abd6e1fd9 100644
--- a/test/workers/repository/onboarding/branch/index.spec.js
+++ b/test/workers/repository/onboarding/branch/index.spec.js
@@ -3,6 +3,8 @@ const {
   checkOnboardingBranch,
 } = require('../../../../../lib/workers/repository/onboarding/branch');
 
+jest.mock('../../../../../lib/workers/repository/onboarding/branch/rebase');
+
 describe('workers/repository/onboarding/branch', () => {
   describe('checkOnboardingBranch', () => {
     let config;
@@ -42,8 +44,17 @@ describe('workers/repository/onboarding/branch', () => {
       const res = await checkOnboardingBranch(config);
       expect(res.repoIsOnboarded).toBe(true);
     });
-    it('creates onboaring branch', async () => {
+    it('creates onboarding branch', async () => {
+      platform.getFileList.mockReturnValue(['package.json']);
+      const res = await checkOnboardingBranch(config);
+      expect(res.repoIsOnboarded).toBe(false);
+      expect(res.branchList).toEqual(['renovate/configure']);
+      expect(platform.setBaseBranch.mock.calls).toHaveLength(1);
+    });
+    it('updates onboarding branch', async () => {
       platform.getFileList.mockReturnValue(['package.json']);
+      platform.findPr.mockReturnValueOnce(null);
+      platform.findPr.mockReturnValueOnce({});
       const res = await checkOnboardingBranch(config);
       expect(res.repoIsOnboarded).toBe(false);
       expect(res.branchList).toEqual(['renovate/configure']);
diff --git a/test/workers/repository/onboarding/branch/rebase.spec.js b/test/workers/repository/onboarding/branch/rebase.spec.js
new file mode 100644
index 0000000000..06fa7885ca
--- /dev/null
+++ b/test/workers/repository/onboarding/branch/rebase.spec.js
@@ -0,0 +1,45 @@
+const defaultConfig = require('../../../../../lib/config/defaults').getConfig();
+const {
+  rebaseOnboardingBranch,
+} = require('../../../../../lib/workers/repository/onboarding/branch/rebase');
+
+describe('workers/repository/onboarding/branch/rebase', () => {
+  describe('rebaseOnboardingBranch()', () => {
+    let config;
+    beforeEach(() => {
+      jest.resetAllMocks();
+      config = {
+        ...defaultConfig,
+      };
+    });
+    it('does nothing if branch is up to date', async () => {
+      platform.getBranchPr.mockReturnValueOnce({ number: 1 });
+      platform.getPr.mockReturnValueOnce({ isStale: false });
+      await rebaseOnboardingBranch(config);
+      expect(platform.commitFilesToBranch.mock.calls).toHaveLength(0);
+    });
+    it('rebases unmodified branch', async () => {
+      platform.getBranchPr.mockReturnValueOnce({ number: 1 });
+      platform.getPr.mockReturnValueOnce({ isStale: true, canRebase: true });
+      await rebaseOnboardingBranch(config);
+      expect(platform.commitFilesToBranch.mock.calls).toHaveLength(1);
+    });
+    it('rebases modified branch', async () => {
+      platform.getBranchPr.mockReturnValueOnce({ number: 1 });
+      platform.getPr.mockReturnValueOnce({ isStale: true, canRebase: false });
+      platform.getPrFiles.mockReturnValueOnce(['renovate.json']);
+      await rebaseOnboardingBranch(config);
+      expect(platform.commitFilesToBranch.mock.calls).toHaveLength(1);
+    });
+    it('does not rebase modified branch with additional files', async () => {
+      platform.getBranchPr.mockReturnValueOnce({ number: 1 });
+      platform.getPr.mockReturnValueOnce({ isStale: true, canRebase: false });
+      platform.getPrFiles.mockReturnValueOnce([
+        'renovate.json',
+        'package.json',
+      ]);
+      await rebaseOnboardingBranch(config);
+      expect(platform.commitFilesToBranch.mock.calls).toHaveLength(0);
+    });
+  });
+});
-- 
GitLab