From 117316c0b135a40f03f2ad6cc2c74640c43d0620 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Tue, 8 Aug 2017 23:03:52 +0200
Subject: [PATCH] fix: check current status check before setting (#655)
Fixes #649
---
lib/api/github.js | 14 ++++-
lib/api/gitlab.js | 18 ++++++
lib/workers/branch/index.js | 22 ++++---
test/api/github.spec.js | 60 +++++++++++++++++++
test/api/gitlab.spec.js | 45 ++++++++++++++
.../branch/__snapshots__/index.spec.js.snap | 2 +
test/workers/branch/index.spec.js | 6 +-
7 files changed, 157 insertions(+), 10 deletions(-)
diff --git a/lib/api/github.js b/lib/api/github.js
index 2a116ca4a8..6d30329ba3 100644
--- a/lib/api/github.js
+++ b/lib/api/github.js
@@ -59,6 +59,7 @@ module.exports = {
isBranchStale,
getBranchPr,
getBranchStatus,
+ getBranchStatusCheck,
setBranchStatus,
deleteBranch,
mergeBranch,
@@ -359,11 +360,22 @@ async function getBranchStatus(branchName, requiredStatusChecks) {
return 'failed';
}
const gotString = `repos/${config.repoName}/commits/${branchName}/status`;
- logger.debug(gotString);
const res = await ghGotRetry(gotString);
return res.body.state;
}
+async function getBranchStatusCheck(branchName, context) {
+ const branchCommit = await getBranchCommit(branchName);
+ const url = `repos/${config.repoName}/commits/${branchCommit}/statuses`;
+ const res = await ghGotRetry(url);
+ for (const check of res.body) {
+ if (check.context === context) {
+ return check.state;
+ }
+ }
+ return null;
+}
+
async function setBranchStatus(
branchName,
context,
diff --git a/lib/api/gitlab.js b/lib/api/gitlab.js
index a2976d3e59..c5391f4e6b 100644
--- a/lib/api/gitlab.js
+++ b/lib/api/gitlab.js
@@ -15,6 +15,7 @@ module.exports = {
getBranch,
getBranchPr,
getBranchStatus,
+ getBranchStatusCheck,
setBranchStatus,
deleteBranch,
// issue
@@ -222,6 +223,23 @@ async function getBranchStatus(branchName, requiredStatusChecks) {
return status;
}
+async function getBranchStatusCheck(branchName, context) {
+ // First, get the branch to find the commit SHA
+ let url = `projects/${config.repoName}/repository/branches/${branchName}`;
+ let res = await glGot(url);
+ const branchSha = res.body.commit.id;
+ // Now, check the statuses for that commit
+ url = `projects/${config.repoName}/repository/commits/${branchSha}/statuses`;
+ res = await glGot(url);
+ logger.debug(`Got res with ${res.body.length} results`);
+ for (const check of res.body) {
+ if (check.name === context) {
+ return check.state;
+ }
+ }
+ return null;
+}
+
async function setBranchStatus(
branchName,
context,
diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js
index d2f3bbbbe4..ac181b4b1e 100644
--- a/lib/workers/branch/index.js
+++ b/lib/workers/branch/index.js
@@ -199,17 +199,25 @@ async function ensureBranch(config) {
}
// Set unpublishable status check
if (config.unpublishSafe && typeof unpublishable !== 'undefined') {
+ const context = 'renovate/unpublish-safe';
const state = unpublishable ? 'success' : 'pending';
const description = unpublishable
? 'Packages are at least 24 hours old'
: 'Packages < 24 hours old can be unpublished';
- await api.setBranchStatus(
- branchName,
- 'renovate/unpublish-safe',
- description,
- state,
- 'https://github.com/singapore/renovate/blob/master/docs/status-checks.md#unpublish-safe'
- );
+ // 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 {
+ logger.debug(`Updating status check state to ${state}`);
+ await api.setBranchStatus(
+ branchName,
+ context,
+ description,
+ state,
+ 'https://github.com/singapore/renovate/blob/master/docs/status-checks.md#unpublish-safe'
+ );
+ }
}
if (config.automergeEnabled === false || config.automergeType === 'pr') {
// No branch automerge
diff --git a/test/api/github.spec.js b/test/api/github.spec.js
index 6662ed1034..8bf3de3256 100644
--- a/test/api/github.spec.js
+++ b/test/api/github.spec.js
@@ -823,6 +823,66 @@ describe('api/github', () => {
expect(res).toEqual('failed');
});
});
+ describe('getBranchStatusCheck', () => {
+ it('returns state if found', async () => {
+ await initRepo('some/repo', 'token');
+ // getBranchCommit
+ ghGot.mockImplementationOnce(() => ({
+ body: {
+ object: {
+ sha: '1235',
+ },
+ },
+ }));
+ ghGot.mockImplementationOnce(() => ({
+ body: [
+ {
+ context: 'context-1',
+ state: 'state-1',
+ },
+ {
+ context: 'context-2',
+ state: 'state-2',
+ },
+ {
+ context: 'context-3',
+ state: 'state-3',
+ },
+ ],
+ }));
+ const res = await github.getBranchStatusCheck('somebranch', 'context-2');
+ expect(res).toEqual('state-2');
+ });
+ it('returns null', async () => {
+ await initRepo('some/repo', 'token');
+ // getBranchCommit
+ ghGot.mockImplementationOnce(() => ({
+ body: {
+ object: {
+ sha: '1235',
+ },
+ },
+ }));
+ ghGot.mockImplementationOnce(() => ({
+ body: [
+ {
+ context: 'context-1',
+ state: 'state-1',
+ },
+ {
+ context: 'context-2',
+ state: 'state-2',
+ },
+ {
+ context: 'context-3',
+ state: 'state-3',
+ },
+ ],
+ }));
+ const res = await github.getBranchStatusCheck('somebranch', 'context-4');
+ expect(res).toEqual(null);
+ });
+ });
describe('setBranchStatus', () => {
it('sets branch status', async () => {
await initRepo('some/repo', 'token');
diff --git a/test/api/gitlab.spec.js b/test/api/gitlab.spec.js
index f339228423..10ac368182 100644
--- a/test/api/gitlab.spec.js
+++ b/test/api/gitlab.spec.js
@@ -326,6 +326,51 @@ describe('api/gitlab', () => {
expect(res).toEqual('foo');
});
});
+ describe('getBranchStatusCheck', () => {
+ beforeEach(() => {
+ glGot.mockReturnValueOnce({
+ body: {
+ commit: {
+ id: 1,
+ },
+ },
+ });
+ });
+ it('returns null if no results', async () => {
+ glGot.mockReturnValueOnce({
+ body: [],
+ });
+ const res = await gitlab.getBranchStatusCheck(
+ 'somebranch',
+ 'some-context'
+ );
+ expect(res).toEqual(null);
+ });
+ it('returns null if no matching results', async () => {
+ glGot.mockReturnValueOnce({
+ body: [{ name: 'context-1', status: 'pending' }],
+ });
+ const res = await gitlab.getBranchStatusCheck(
+ 'somebranch',
+ 'some-context'
+ );
+ expect(res).toEqual(null);
+ });
+ it('returns status if name found', async () => {
+ glGot.mockReturnValueOnce({
+ body: [
+ { name: 'context-1', state: 'pending' },
+ { name: 'some-context', state: 'success' },
+ { name: 'context-3', state: 'failed' },
+ ],
+ });
+ const res = await gitlab.getBranchStatusCheck(
+ 'somebranch',
+ 'some-context'
+ );
+ expect(res).toEqual('success');
+ });
+ });
describe('setBranchStatus', () => {
it('sets branch status', async () => {
await initRepo('some/repo', 'token');
diff --git a/test/workers/branch/__snapshots__/index.spec.js.snap b/test/workers/branch/__snapshots__/index.spec.js.snap
index 2ad706187a..61afb6cc4f 100644
--- a/test/workers/branch/__snapshots__/index.spec.js.snap
+++ b/test/workers/branch/__snapshots__/index.spec.js.snap
@@ -13,6 +13,7 @@ Object {
"branchExists": [Function],
"commitFilesToBranch": [Function],
"getBranchStatus": [Function],
+ "getBranchStatusCheck": [Function],
"getFileContent": [Function],
"mergeBranch": [Function],
"setBranchStatus": [Function],
@@ -38,6 +39,7 @@ Object {
"branchExists": [Function],
"commitFilesToBranch": [Function],
"getBranchStatus": [Function],
+ "getBranchStatusCheck": [Function],
"getFileContent": [Function],
"mergeBranch": [Function],
"setBranchStatus": [Function],
diff --git a/test/workers/branch/index.spec.js b/test/workers/branch/index.spec.js
index c0b57d4d53..b97729532e 100644
--- a/test/workers/branch/index.spec.js
+++ b/test/workers/branch/index.spec.js
@@ -119,6 +119,7 @@ describe('workers/branch', () => {
config.api.commitFilesToBranch = jest.fn();
config.api.getFileContent.mockReturnValueOnce('old content');
config.api.getBranchStatus = jest.fn();
+ config.api.getBranchStatusCheck = jest.fn();
config.api.setBranchStatus = jest.fn();
config.depName = 'dummy';
config.currentVersion = '1.0.0';
@@ -173,13 +174,14 @@ describe('workers/branch', () => {
expect(await branchWorker.ensureBranch(config)).toBe(true);
expect(config.api.setBranchStatus.mock.calls).toHaveLength(1);
});
- it('sets branch status success', async () => {
+ it('skips branch status success', 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('success');
expect(await branchWorker.ensureBranch(config)).toBe(true);
- expect(config.api.setBranchStatus.mock.calls).toHaveLength(1);
+ expect(config.api.setBranchStatus.mock.calls).toHaveLength(0);
});
it('automerges successful branches', async () => {
branchWorker.getParentBranch.mockReturnValueOnce('dummy branch');
--
GitLab