wip(octokit): used octokit for the combined status check

for #382

src/github/actions.js

@@ -18,18 +18,18 @@ export default function (githubCredentials) {
   const octokit = octokitFactory();
   const {token} = githubCredentials;
   octokit.authenticate({type: 'token', token});
-  const {get, post, put, del} = clientFactory(githubCredentials);
+  const {post, put, del} = clientFactory(githubCredentials);
 
-  function ensureAcceptability({repo, ref, url, pollWhenPending}, log, timeout = minutes(1)) {
+  function ensureAcceptability({repo, sha, url, pollWhenPending}, log, timeout = minutes(1)) {
     log(['info', 'PR', 'validating'], url);
 
-    return get(`https://api.github.com/repos/${repo.full_name}/commits/${ref}/status`)
-      .then(response => response.body)
+    return octokit.repos.getCombinedStatusForRef({owner: repo.owner.login, repo: repo.name, ref: sha})
+      .then(response => response.data)
       .then(({state}) => {
         switch (state) {
           case 'pending': {
             if (pollWhenPending) {
-              return poll({repo, ref, pollWhenPending}, log, timeout, ensureAcceptability).then(message => {
+              return poll({repo, sha, pollWhenPending}, log, timeout, ensureAcceptability).then(message => {
                 log(['info', 'PR', 'pending-status'], `retrying statuses for: ${url}`);
                 return message;
               });

src/github/request-methods.js

@@ -6,7 +6,6 @@ function configureHeaders(token) {
 
 export default function ({token}) {
   return {
-    get: url => highwire.get(url, configureHeaders(token)),
     post: (url, payload) => highwire.post(url, payload, configureHeaders(token)),
     put: (url, payload) => highwire.put(url, payload, configureHeaders(token)),
     del: url => highwire.del(url, configureHeaders(token))

src/process.js

@@ -7,7 +7,7 @@ export default function (
 ) {
   const {ensureAcceptability, acceptPR, deleteBranch, postErrorComment} = createActions(github);
 
-  return ensureAcceptability({repo: head.repo, ref: head.ref, url, pollWhenPending}, (...args) => request.log(...args))
+  return ensureAcceptability({repo: head.repo, sha: head.sha, url, pollWhenPending}, (...args) => request.log(...args))
     .then(() => acceptPR(url, head.sha, number, squash, acceptAction, (...args) => request.log(...args)))
     .then(() => deleteBranch(head, deleteBranches))
     .catch(err => {

test/integration/features/step_definitions/github-api-steps.js

@@ -54,15 +54,15 @@ defineSupportCode(({Before, After, Given, setWorldConstructor}) => {
             ref: this.ref,
             repo: {
               full_name: this.repoFullName,
-              name: 'temp-name',
+              name: this.repoName,
               owner: {login: this.repoOwner}
             }
           }
         });
     }
     githubScope
       .matchHeader('Authorization', authorizationHeader)
-      .get(`/search/issues?q=${encodeURIComponent(this.sha)}+type%3Apr`)
+      .get(`/search/issues?q=${this.sha}+type%3Apr`)
       .reply(OK, {
         items: [{
           url: 'https://api.github.com/123',
@@ -86,7 +86,7 @@ defineSupportCode(({Before, After, Given, setWorldConstructor}) => {
   Given(/^statuses exist for the PR$/, function (callback) {
     githubScope
       .matchHeader('Authorization', authorizationHeader)
-      .get(`/repos/${this.repoFullName}/commits/${this.ref}/status`)
+      .get(`/repos/${this.repoFullName}/commits/${this.sha}/status`)
       .reply(OK, {
         state: 'success'
       });
@@ -136,7 +136,7 @@ defineSupportCode(({Before, After, Given, setWorldConstructor}) => {
     this.comments = `/${any.word()}`;
     githubScope
       .matchHeader('Authorization', authorizationHeader)
-      .get(`/repos/${this.repoFullName}/commits/${this.ref}/status`)
+      .get(`/repos/${this.repoFullName}/commits/${this.sha}/status`)
       .reply(OK, {
         state: status
       });

test/integration/features/support/world.js

@@ -49,11 +49,11 @@ function buildWebhookPayload(
 
 export function World() {
   this.githubToken = any.word();
-  this.sha = any.string();
+  this.sha = any.word();
   this.ref = any.word();
   this.prNumber = any.integer();
   this.repoOwner = any.word();
-  this.repoName = 'test-repo';
+  this.repoName = any.word();
   this.repoFullName = `${this.repoOwner}/${this.repoName}`;
 
   this.receiveWebhook = ({event, action, prDetails, statusEventDetails}) => this.server.inject({

test/unit/github/actions-test.js

@@ -13,37 +13,49 @@ import {
 } from '../../../src/errors';
 
 suite('github actions', () => {
-  let actions, sandbox, get, post, put, del, octokitAuthenticate, octokitIssueSearch, octokitGetPr;
+  let
+    sandbox,
+    actions,
+    post,
+    put,
+    del,
+    octokitAuthenticate,
+    octokitIssueSearch,
+    octokitGetPr,
+    octokitCombinedStatus;
   const MINUTE = 1000 * 60;
   const token = any.simpleObject();
   const githubCredentials = {...any.simpleObject(), token};
   const url = any.url();
   const sha = any.string();
   const ref = any.string();
+  const repoOwner = any.word();
   const repoName = any.word();
+  const repo = {name: repoName, owner: {login: repoOwner}};
   const prNumber = any.string();
   const response = {body: any.simpleObject()};
   const log = () => undefined;
 
   setup(() => {
     sandbox = sinon.sandbox.create();
 
-    get = sinon.stub();
     post = sinon.stub();
     put = sinon.stub();
     del = sinon.stub();
     octokitAuthenticate = sinon.spy();
     octokitIssueSearch = sinon.stub();
     octokitGetPr = sinon.stub();
+    octokitCombinedStatus = sinon.stub();
 
     sandbox.stub(octokitFactory, 'default');
     sandbox.stub(poll, 'default');
-    sandbox.stub(clientFactory, 'default').withArgs(githubCredentials).returns({get, post, put, del});
+    sandbox.stub(clientFactory, 'default').withArgs(githubCredentials).returns({post, put, del});
 
     octokitFactory.default.returns({
       authenticate: octokitAuthenticate,
       search: {issues: octokitIssueSearch},
-      pullRequests: {get: octokitGetPr}
+      pullRequests: {get: octokitGetPr},
+      repos: {getCombinedStatusForRef: octokitCombinedStatus}
     });
     actions = actionsFactory(githubCredentials);
   });
@@ -54,65 +66,63 @@ suite('github actions', () => {
 
   suite('ensure PR can be merged', () => {
     test('that the passing status is acceptable', () => {
-      get.withArgs(`https://api.github.com/repos/${repoName}/commits/${ref}/status`).resolves({
-        body: {
-          state: 'success'
-        }
-      });
+      octokitCombinedStatus.withArgs({owner: repoOwner, repo: repoName, ref: sha}).resolves({data: {state: 'success'}});
 
       return assert.becomes(
-        actions.ensureAcceptability({repo: {full_name: repoName}, ref}, () => undefined),
+        actions.ensureAcceptability({repo, sha}, () => undefined),
         'All commit statuses passed'
       ).then(assertAuthenticatedForOctokit);
     });
 
     test('that the failing status results in rejection', () => {
-      get.resolves({body: {state: 'failure'}});
+      octokitCombinedStatus.withArgs({owner: repoOwner, repo: repoName, ref: sha}).resolves({data: {state: 'failure'}});
 
       return assert.isRejected(
-        actions.ensureAcceptability({repo: {full_name: repoName}, ref}, () => undefined),
+        actions.ensureAcceptability({repo, sha}, () => undefined),
         FailedStatusFoundError,
         /A failed status was found for this PR\./
       ).then(assertAuthenticatedForOctokit);
     });
 
     test('that the pending status delegates to poller', () => {
       const result = any.string();
-      get.resolves({body: {state: 'pending'}});
-      poll.default.withArgs({repo: {full_name: repoName}, ref, pollWhenPending: true}, log, MINUTE).resolves(result);
+      octokitCombinedStatus.withArgs({owner: repoOwner, repo: repoName, ref: sha}).resolves({data: {state: 'pending'}});
+      poll.default.withArgs({repo, sha, pollWhenPending: true}, log, MINUTE).resolves(result);
 
       return assert.becomes(
-        actions.ensureAcceptability({repo: {full_name: repoName}, ref, pollWhenPending: true}, log),
+        actions.ensureAcceptability({repo, sha, pollWhenPending: true}, log),
         result
       ).then(assertAuthenticatedForOctokit);
     });
 
     test('that the timeout is passed along to the poller', () => {
       const timeout = any.integer();
       const result = any.string();
-      get.resolves({body: {state: 'pending'}});
-      poll.default.withArgs({repo: {full_name: repoName}, ref, pollWhenPending: true}, log, timeout).resolves(result);
+      octokitCombinedStatus.withArgs({owner: repoOwner, repo: repoName, ref: sha}).resolves({data: {state: 'pending'}});
+      poll.default.withArgs({repo, sha, pollWhenPending: true}, log, timeout).resolves(result);
 
       return assert.becomes(
-        actions.ensureAcceptability({repo: {full_name: repoName}, ref, pollWhenPending: true}, log, timeout),
+        actions.ensureAcceptability({repo, sha, pollWhenPending: true}, log, timeout),
         result
       ).then(assertAuthenticatedForOctokit);
     });
 
     test('that the polling does not happen without the `pollWhenPending` flag', () => {
-      get.resolves({body: {state: 'pending'}});
+      octokitCombinedStatus.withArgs({owner: repoOwner, repo: repoName, ref: sha}).resolves({data: {state: 'pending'}});
 
       return assert.isRejected(
-        actions.ensureAcceptability({repo: {full_name: repoName}, ref}, log, any.integer()),
+        actions.ensureAcceptability({repo, sha}, log, any.integer()),
         'pending'
       ).then(assertAuthenticatedForOctokit);
     });
 
     test('that an invalid status results in rejection', () => {
-      get.resolves({body: {state: any.string()}});
+      octokitCombinedStatus
+        .withArgs({owner: repoOwner, repo: repoName, ref: sha})
+        .resolves({data: {state: any.string()}});
 
       return assert.isRejected(
-        actions.ensureAcceptability({repo: {full_name: repoName}, ref}, log),
+        actions.ensureAcceptability({repo, sha}, log),
         InvalidStatusFoundError,
         /An invalid status was found for this PR\./
       ).then(assertAuthenticatedForOctokit);
@@ -208,24 +218,16 @@ suite('github actions', () => {
   suite('PRs for a commit', () => {
     test('that PRs with HEAD matching a commit are fetched', () => {
       const pullRequests = any.listOf(any.simpleObject);
-      const ownerLogin = any.word();
       octokitIssueSearch.withArgs({q: `${ref}+type:pr`}).returns({data: {items: pullRequests}});
 
-      return assert.becomes(
-        actions.getPullRequestsForCommit({repo: {full_name: repoName, owner: {login: ownerLogin}}, ref}),
-        pullRequests
-      ).then(assertAuthenticatedForOctokit);
+      return assert.becomes(actions.getPullRequestsForCommit({ref}), pullRequests).then(assertAuthenticatedForOctokit);
     });
   });
 
   test('that the PR gets requested by issue number', () => {
-    const repoOwner = any.word();
     const pullRequest = any.simpleObject();
     octokitGetPr.withArgs({owner: repoOwner, repo: repoName, number: prNumber}).resolves({data: pullRequest});
 
-    return assert.becomes(
-      actions.getPullRequest({owner: {login: repoOwner}, name: repoName}, prNumber),
-      pullRequest
-    );
+    return assert.becomes(actions.getPullRequest(repo, prNumber), pullRequest);
   });
 });

test/unit/github/request-methods-test.js

@@ -15,20 +15,13 @@ suite('github api', () => {
     client = clientFactory({token: githubToken});
     sandbox = sinon.sandbox.create();
 
-    sandbox.stub(highwire, 'get');
     sandbox.stub(highwire, 'post');
     sandbox.stub(highwire, 'put');
     sandbox.stub(highwire, 'del');
   });
 
   teardown(() => sandbox.restore());
 
-  test('that the get method handles auth', () => {
-    highwire.get.withArgs(url, {headers: {Authorization: `token ${githubToken}`}}).resolves(response);
-
-    return assert.becomes(client.get(url), response);
-  });
-
   test('that the post method handles auth', () => {
     highwire.post.withArgs(url, payload, {headers: {Authorization: `token ${githubToken}`}}).resolves(response);
 

test/unit/process-test.js

@@ -50,7 +50,7 @@ suite('process', () => {
       const message = any.string();
       const message2 = any.string();
       const tags = any.listOf(any.string);
-      assert.calledWith(ensureAcceptability, {repo, ref, url, pollWhenPending});
+      assert.calledWith(ensureAcceptability, {repo, sha, url, pollWhenPending});
       assert.calledWith(acceptPR, url, sha, number, squash, acceptAction);
       assert.calledWith(deleteBranch, head, deleteBranches);