From e9f672060e4f93c82f22ddcf5aac91f8ec6cef3f Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@keylocation.sg> Date: Mon, 14 Aug 2017 06:27:00 +0200 Subject: [PATCH] fix: disable unpublish-safe default (#672) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was a mistake to enable this feature by default, and should be disabled. A check has been made to set the status to “success†if it was previously “pending†and the setting is now false. This should also cover the case when someone enables it and then disables it. Fixes #667 --- lib/config/definitions.js | 1 + lib/workers/branch/index.js | 16 +++++++++++----- lib/workers/package/versions.js | 1 + test/workers/branch/index.spec.js | 17 +++++++++++++++-- .../package/__snapshots__/index.spec.js.snap | 2 +- .../package/__snapshots__/versions.spec.js.snap | 12 ++++++++++++ 6 files changed, 41 insertions(+), 8 deletions(-) diff --git a/lib/config/definitions.js b/lib/config/definitions.js index 05191aeeaa..7409ca3d34 100644 --- a/lib/config/definitions.js +++ b/lib/config/definitions.js @@ -347,6 +347,7 @@ const options = [ name: 'unpublishSafe', description: 'Set a status check for unpublish-safe upgrades', type: 'boolean', + default: false, }, { name: 'prCreation', diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js index ac181b4b1e..cec9425ef9 100644 --- a/lib/workers/branch/index.js +++ b/lib/workers/branch/index.js @@ -197,15 +197,21 @@ async function ensureBranch(config) { // Return now if no branch exists return false; } - // Set unpublishable status check - if (config.unpublishSafe && typeof unpublishable !== 'undefined') { - const context = 'renovate/unpublish-safe'; - const state = unpublishable ? 'success' : 'pending'; + const context = 'renovate/unpublish-safe'; + const existingState = await api.getBranchStatusCheck(branchName, context); + // If status check was enabled and then is disabled, any "pending" status check needs to be set to "success" + const removeStatusCheck = + existingState === 'pending' && !config.unpublishSafe; + if ( + (config.unpublishSafe || removeStatusCheck) && + typeof unpublishable !== 'undefined' + ) { + // Set unpublishable status check + const state = unpublishable || removeStatusCheck ? 'success' : 'pending'; const description = unpublishable ? 'Packages are at least 24 hours old' : 'Packages < 24 hours old can be unpublished'; // Check if state needs setting - const existingState = await api.getBranchStatusCheck(branchName, context); if (existingState === state) { logger.debug('Status check is already up-to-date'); } else { diff --git a/lib/workers/package/versions.js b/lib/workers/package/versions.js index 0c4c3a59ae..ff4450813a 100644 --- a/lib/workers/package/versions.js +++ b/lib/workers/package/versions.js @@ -37,6 +37,7 @@ function determineUpgrades(npmDep, config) { type: 'pin', isPin: true, automergeEnabled: true, + unpublishSafe: false, newVersion: maxSatisfying, newVersionMajor: semver.major(maxSatisfying), groupName: 'Pin Dependencies', diff --git a/test/workers/branch/index.spec.js b/test/workers/branch/index.spec.js index b97729532e..adb9b09319 100644 --- a/test/workers/branch/index.spec.js +++ b/test/workers/branch/index.spec.js @@ -168,21 +168,34 @@ describe('workers/branch', () => { branchWorker.getParentBranch.mockReturnValueOnce('dummy branch'); packageJsonHelper.setNewValue.mockReturnValueOnce('new content'); config.api.branchExists.mockReturnValueOnce(true); + config.unpublishSafe = true; config.upgrades[0].unpublishable = true; config.upgrades.push({ ...config }); config.upgrades[1].unpublishable = false; expect(await branchWorker.ensureBranch(config)).toBe(true); expect(config.api.setBranchStatus.mock.calls).toHaveLength(1); }); - it('skips branch status success', async () => { + it('skips branch status pending', async () => { branchWorker.getParentBranch.mockReturnValueOnce('dummy branch'); packageJsonHelper.setNewValue.mockReturnValueOnce('new content'); config.api.branchExists.mockReturnValueOnce(true); + config.unpublishSafe = true; + config.api.getBranchStatusCheck.mockReturnValueOnce('pending'); config.upgrades[0].unpublishable = true; - config.api.getBranchStatusCheck.mockReturnValueOnce('success'); + config.upgrades.push({ ...config }); + config.upgrades[1].unpublishable = false; expect(await branchWorker.ensureBranch(config)).toBe(true); expect(config.api.setBranchStatus.mock.calls).toHaveLength(0); }); + it('skips branch status success if setting disabled', async () => { + branchWorker.getParentBranch.mockReturnValueOnce('dummy branch'); + packageJsonHelper.setNewValue.mockReturnValueOnce('new content'); + config.api.branchExists.mockReturnValueOnce(true); + config.upgrades[0].unpublishable = true; + config.api.getBranchStatusCheck.mockReturnValueOnce('pending'); + expect(await branchWorker.ensureBranch(config)).toBe(true); + expect(config.api.setBranchStatus.mock.calls).toHaveLength(1); + }); it('automerges successful branches', async () => { branchWorker.getParentBranch.mockReturnValueOnce('dummy branch'); packageJsonHelper.setNewValue.mockReturnValueOnce('new content'); diff --git a/test/workers/package/__snapshots__/index.spec.js.snap b/test/workers/package/__snapshots__/index.spec.js.snap index a5953d969c..c08eb58c68 100644 --- a/test/workers/package/__snapshots__/index.spec.js.snap +++ b/test/workers/package/__snapshots__/index.spec.js.snap @@ -209,7 +209,7 @@ This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](ht "semanticPrefix": "chore(deps):", "timezone": null, "type": "error", - "unpublishSafe": true, + "unpublishSafe": false, }, ] `; diff --git a/test/workers/package/__snapshots__/versions.spec.js.snap b/test/workers/package/__snapshots__/versions.spec.js.snap index 6e47c491e0..fbf2467a0a 100644 --- a/test/workers/package/__snapshots__/versions.spec.js.snap +++ b/test/workers/package/__snapshots__/versions.spec.js.snap @@ -13,6 +13,7 @@ Array [ "newVersion": "0.4.4", "newVersionMajor": 0, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, Object { @@ -76,6 +77,7 @@ Array [ "newVersion": "0.9.7", "newVersionMajor": 0, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, ] @@ -94,6 +96,7 @@ Array [ "newVersion": "1.4.1", "newVersionMajor": 1, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, ] @@ -157,6 +160,7 @@ Array [ "newVersion": "0.4.4", "newVersionMajor": 0, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, ] @@ -202,6 +206,7 @@ Array [ "newVersion": "0.4.4", "newVersionMajor": 0, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, Object { @@ -231,6 +236,7 @@ Array [ "newVersion": "0.4.4", "newVersionMajor": 0, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, Object { @@ -413,6 +419,7 @@ Array [ "newVersion": "2.0.3", "newVersionMajor": 2, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, ] @@ -469,6 +476,7 @@ Array [ "newVersion": "0.4.4", "newVersionMajor": 0, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, ] @@ -509,6 +517,7 @@ Array [ "newVersion": "0.4.4", "newVersionMajor": 0, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, ] @@ -555,6 +564,7 @@ Array [ "newVersion": "1.3.0", "newVersionMajor": 1, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, ] @@ -630,6 +640,7 @@ Array [ "newVersion": "1.0.1", "newVersionMajor": 1, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, ] @@ -751,6 +762,7 @@ Array [ "newVersion": "1.3.0", "newVersionMajor": 1, "type": "pin", + "unpublishSafe": false, "unpublishable": false, }, ] -- GitLab