fix: remove hdel from start job (#28)

screwdriver-cd · Nov 13, 2017 · 9e9a4b2 · 9e9a4b2
1 parent 1709e8e
commit 9e9a4b2
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 42 deletions.
diff --git a/index.js b/index.js
@@ -2,9 +2,11 @@
 
 const NR = require('node-resque');
 const jobs = require('./lib/jobs');
+const Redis = require('ioredis');
 const request = require('request');
 const winston = require('winston');
 const { connectionDetails, queuePrefix } = require('./config/redis');
+const redis = new Redis(connectionDetails.port, connectionDetails.host, connectionDetails.options);
 
 /**
  * Update build status to FAILURE
@@ -19,29 +21,31 @@ const { connectionDetails, queuePrefix } = require('./config/redis');
  */
 function updateBuildStatus(updateConfig, callback) {
     const { failure, job, queue, workerId } = updateConfig;
-    const { apiUri, buildId, token } = updateConfig.job.args[0];
+    const { buildId } = updateConfig.job.args[0];
 
-    return request({
-        json: true,
-        method: 'PUT',
-        uri: `${apiUri}/v4/builds/${buildId}`,
-        payload: {
-            status: 'FAILURE'
-        },
-        auth: {
-            bearer: token
-        }
-    }, (err, response) => {
-        if (!err && response.statusCode === 200) {
-            // eslint-disable-next-line max-len
-            winston.error(`worker[${workerId}] ${job} failure ${queue} ${JSON.stringify(job)} >> successfully update build status: ${failure}`);
-            callback(null);
-        } else {
-            // eslint-disable-next-line max-len
-            winston.error(`worker[${workerId}] ${job} failure ${queue} ${JSON.stringify(job)} >> ${failure} ${err} ${response}`);
-            callback(err);
-        }
-    });
+    return redis.hget(`${queuePrefix}buildConfigs`, buildId)
+        .then(JSON.parse)
+        .then(fullBuildConfig => request({
+            json: true,
+            method: 'PUT',
+            uri: `${fullBuildConfig.apiUri}/v4/builds/${buildId}`,
+            payload: {
+                status: 'FAILURE'
+            },
+            auth: {
+                bearer: fullBuildConfig.token
+            }
+        }, (err, response) => {
+            if (!err && response.statusCode === 200) {
+                // eslint-disable-next-line max-len
+                winston.error(`worker[${workerId}] ${job} failure ${queue} ${JSON.stringify(job)} >> successfully update build status: ${failure}`);
+                callback(null);
+            } else {
+                // eslint-disable-next-line max-len
+                winston.error(`worker[${workerId}] ${job} failure ${queue} ${JSON.stringify(job)} >> ${failure} ${err} ${response}`);
+                callback(err);
+            }
+        }));
 }
 
 /**

diff --git a/lib/jobs.js b/lib/jobs.js
@@ -2,6 +2,7 @@
 
 const Redis = require('ioredis');
 const config = require('config');
+const winston = require('winston');
 const { connectionDetails, queuePrefix } = require('../config/redis');
 
 const redis = new Redis(connectionDetails.port, connectionDetails.host, connectionDetails.options);
@@ -46,9 +47,11 @@ const retryOptions = {
 function start(buildConfig, callback) {
     return redis.hget(`${queuePrefix}buildConfigs`, buildConfig.buildId)
         .then(JSON.parse)
-        .then(fullBuildConfig => redis.hdel(`${queuePrefix}buildConfigs`, buildConfig.buildId)
-            .then(() => executor.start(fullBuildConfig)))
-        .then(result => callback(null, result), err => callback(err));
+        .then(executor.start)
+        .then(result => callback(null, result), (err) => {
+            winston.error('err in start job: ', err);
+            callback(err);
+        });
 }
 
 /**

diff --git a/test/index.test.js b/test/index.test.js
@@ -17,16 +17,16 @@ describe('Index Test', () => {
     const verb = '+';
     const delay = '3ms';
     const workerId = 1;
-    const job = { args: [{ token: 'fake', buildId: 1, apiUri: 'foo.bar' }] };
+    const job = { args: [{ buildId: 1 }] };
     const queue = 'testbuilds';
     const failure = 'failed';
     const updateConfig = { job, queue, workerId, failure };
     const requestOptions = {
-        auth: { bearer: job.args[0].token },
+        auth: { bearer: 'fake' },
         json: true,
         method: 'PUT',
         payload: { status: 'FAILURE' },
-        uri: `${job.args[0].apiUri}/v4/builds/${job.args[0].buildId}`
+        uri: `foo.bar/v4/builds/${job.args[0].buildId}`
     };
 
     let mockJobs;
@@ -44,6 +44,8 @@ describe('Index Test', () => {
     let supportFunction;
     let updateBuildStatusMock;
     let processExitMock;
+    let mockRedis;
+    let mockRedisObj;
 
     before(() => {
         mockery.enable({
@@ -85,7 +87,12 @@ describe('Index Test', () => {
             connectionDetails: 'mockRedisConfig',
             queuePrefix: 'mockQueuePrefix_'
         };
+        mockRedisObj = {
+            hget: sinon.stub().resolves('{"apiUri": "foo.bar", "token": "fake"}')
+        };
+        mockRedis = sinon.stub().returns(mockRedisObj);
 
+        mockery.registerMock('ioredis', mockRedis);
         mockery.registerMock('./lib/jobs', mockJobs);
         mockery.registerMock('node-resque', nrMockClass);
         mockery.registerMock('winston', winstonMock);
@@ -114,6 +121,8 @@ describe('Index Test', () => {
             requestMock.yieldsAsync(null, { statusCode: 200 });
 
             supportFunction.updateBuildStatus(updateConfig, (err) => {
+                assert.calledWith(mockRedisObj.hget,
+                    'mockQueuePrefix_buildConfigs', job.args[0].buildId);
                 assert.calledWith(requestMock, requestOptions);
                 assert.isNull(err);
                 assert.calledWith(winstonMock.error,

diff --git a/test/jobs.test.js b/test/jobs.test.js
@@ -89,16 +89,13 @@ describe('Jobs Unit Test', () => {
 
             mockExecutor.start.resolves(null);
             mockRedisObj.hget.resolves(expectedConfig);
-            mockRedisObj.hdel.resolves(1);
 
             return jobs.start.perform({ buildId: expectedConfig.buildId }, (err, result) => {
                 assert.isNull(err);
                 assert.isNull(result);
 
                 assert.calledWith(mockExecutor.start, JSON.parse(expectedConfig));
-
                 assert.calledWith(mockRedisObj.hget, 'buildConfigs', expectedConfig.buildId);
-                assert.calledWith(mockRedisObj.hdel, 'buildConfigs', expectedConfig.buildId);
             });
         });
 
@@ -124,17 +121,6 @@ describe('Jobs Unit Test', () => {
                 assert.deepEqual(err, expectedError);
             });
         });
-
-        it('returns an error when redis fails to remove a config', () => {
-            const expectedError = new Error('hdel error');
-
-            mockRedisObj.hget.resolves('{}');
-            mockRedisObj.hdel.rejects(expectedError);
-
-            return jobs.start.perform({}, (err) => {
-                assert.deepEqual(err, expectedError);
-            });
-        });
     });
 
     describe('stop', () => {