fix: properly handle failure to start CI

nodejs · Aug 4, 2020 · 48c306b · 48c306b
1 parent 332576a
commit 48c306b
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 43 deletions.
diff --git a/lib/ci/run_ci.js b/lib/ci/run_ci.js
@@ -59,13 +59,19 @@ class RunPRJob {
 
     try {
       cli.startSpinner('Starting PR CI job');
-      await this.request.text(CI_PR_URL, {
+      const response = await this.request.fetch(CI_PR_URL, {
         method: 'POST',
         headers: {
           'Jenkins-Crumb': crumb
         },
         body: this.payload
       });
+      if (response.status !== 201) {
+        cli.stopSpinner(
+          `Failed to start PR CI: ${response.status} ${response.statusText}`,
+          this.cli.SPINNER_STATUS.FAILED);
+        return false;
+      }
       cli.stopSpinner('PR CI job successfully started');
     } catch (err) {
       cli.stopSpinner('Failed to start CI', this.cli.SPINNER_STATUS.FAILED);

diff --git a/lib/request.js b/lib/request.js
@@ -17,23 +17,22 @@ class Request {
     return fs.readFileSync(filePath, 'utf8');
   }
 
-  async text(url, options = {}) {
+  async fetch(url, options) {
     options.agent = this.proxyAgent;
     if (url.startsWith(`https://${CI_DOMAIN}`)) {
       options.headers = options.headers || {};
       Object.assign(options.headers, this.getJenkinsHeaders());
     }
-    return fetch(url, options).then(res => res.text());
+    return fetch(url, options);
+  }
+
+  async text(url, options = {}) {
+    return this.fetch(url, options).then(res => res.text());
   }
 
   async json(url, options = {}) {
-    options.agent = this.proxyAgent;
     options.headers = options.headers || {};
-    options.headers.Accept = 'application/json';
-    if (url.startsWith(`https://${CI_DOMAIN}`)) {
-      Object.assign(options.headers, this.getJenkinsHeaders());
-    }
-    return fetch(url, options).then(res => res.json());
+    return this.fetch(url, options).then(res => res.json());
   }
 
   async gql(name, variables, path) {

diff --git a/test/unit/ci_start.js → test/unit/ci_start.test.js b/test/unit/ci_start.js → test/unit/ci_start.test.js
@@ -13,17 +13,42 @@ const {
 const TestCLI = require('../fixtures/test_cli');
 
 describe('Jenkins', () => {
-  it('should fail if starting node-pull-request fails', async() => {
+  const owner = 'nodejs';
+  const repo = 'node-auto-test';
+  const prid = 123456;
+  const crumb = 'asdf1234';
+
+  before(() => {
+    sinon.stub(FormData.prototype, 'append').callsFake(function(key, value) {
+      assert.strictEqual(key, 'json');
+      const { parameter } = JSON.parse(value);
+      const expectedParameters = {
+        CERTIFY_SAFE: 'on',
+        TARGET_GITHUB_ORG: owner,
+        TARGET_REPO_NAME: repo,
+        PR_ID: prid,
+        REBASE_ONTO: '<pr base branch>',
+        DESCRIPTION_SETTER_DESCRIPTION: ''
+      };
+      for (const { name, value } of parameter) {
+        assert.strictEqual(value, expectedParameters[name]);
+        delete expectedParameters[name];
+      }
+      assert.strictEqual(Object.keys(expectedParameters).length, 0);
+
+      this._validated = true;
+
+      return FormData.prototype.append.wrappedMethod.bind(this)(key, value);
+    });
+  });
+
+  it('should fail if starting node-pull-request throws', async() => {
     const cli = new TestCLI();
-    const crumb = 'asdf1234';
     const request = {
       text: sinon.stub().throws(),
       json: sinon.stub().withArgs(CI_CRUMB_URL)
         .returns(Promise.resolve({ crumb }))
     };
-    const owner = 'nodejs';
-    const repo = 'node-auto-test';
-    const prid = 123456;
 
     const jobRunner = new RunPRJob(cli, request, owner, repo, prid);
     assert.strictEqual(await jobRunner.start(), false);
@@ -34,55 +59,46 @@ describe('Jenkins', () => {
     const request = {
       json: sinon.stub().throws()
     };
-    const owner = 'nodejs';
-    const repo = 'node-auto-test';
-    const prid = 123456;
 
     const jobRunner = new RunPRJob(cli, request, owner, repo, prid);
     assert.strictEqual(await jobRunner.start(), false);
   });
 
   it('should start node-pull-request', async() => {
     const cli = new TestCLI();
-    const crumb = 'asdf1234';
-    const owner = 'nodejs';
-    const repo = 'node-auto-test';
-    const prid = 123456;
-
-    sinon.stub(FormData.prototype, 'append').callsFake(function(key, value) {
-      assert.strictEqual(key, 'json');
-      const { parameter } = JSON.parse(value);
-      const expectedParameters = {
-        CERTIFY_SAFE: 'on',
-        TARGET_GITHUB_ORG: owner,
-        TARGET_REPO_NAME: repo,
-        PR_ID: prid,
-        REBASE_ONTO: '<pr base branch>',
-        DESCRIPTION_SETTER_DESCRIPTION: ''
-      };
-      for (const { name, value } of parameter) {
-        assert.strictEqual(value, expectedParameters[name]);
-        delete expectedParameters[name];
-      }
-      assert.strictEqual(Object.keys(expectedParameters).length, 0);
 
-      this._validated = true;
+    const request = {
+      fetch: sinon.stub()
+        .callsFake((url, { method, headers, body }) => {
+          assert.strictEqual(url, CI_PR_URL);
+          assert.strictEqual(method, 'POST');
+          assert.deepStrictEqual(headers, { 'Jenkins-Crumb': crumb });
+          assert.ok(body._validated);
+          return Promise.resolve({ status: 201 });
+        }),
+      json: sinon.stub().withArgs(CI_CRUMB_URL)
+        .returns(Promise.resolve({ crumb }))
+    };
+    const jobRunner = new RunPRJob(cli, request, owner, repo, prid);
+    assert.ok(await jobRunner.start());
+  });
 
-      return FormData.prototype.append.wrappedMethod.bind(this)(key, value);
-    });
+  it('should return false if node-pull-request not started', async() => {
+    const cli = new TestCLI();
 
     const request = {
-      text: sinon.stub()
+      fetch: sinon.stub()
         .callsFake((url, { method, headers, body }) => {
           assert.strictEqual(url, CI_PR_URL);
           assert.strictEqual(method, 'POST');
           assert.deepStrictEqual(headers, { 'Jenkins-Crumb': crumb });
           assert.ok(body._validated);
+          return Promise.resolve({ status: 401 });
         }),
       json: sinon.stub().withArgs(CI_CRUMB_URL)
         .returns(Promise.resolve({ crumb }))
     };
     const jobRunner = new RunPRJob(cli, request, owner, repo, prid);
-    assert.ok(await jobRunner.start());
+    assert.strictEqual(await jobRunner.start(), false);
   });
 });