From dd625179ceea04571ee151b802fa7660b6ef9c83 Mon Sep 17 00:00:00 2001 From: enudler Date: Mon, 4 Feb 2019 19:09:43 +0200 Subject: [PATCH] feat(jobs): verify test exists before creating/updating job (#29) * feat(jobs): verify test exists before creating/updating job re #26 * style(jobs): changing nock match function * feat(jobs): verify test exists before creating/updating job re #26 * style(jobs): changing nock match function * refactor(jobs): refactor of job verifier to use error handler * test(reporter): integrate new job validations into reporter tests --- src/jobs/helpers/jobVerifier.js | 45 ++++++--------- .../jobs/createJobGlobal-test.js | 3 +- .../jobs/createJobKubernetes-test.js | 46 ++++++++++------ .../jobs/createJobMetronome-test.js | 55 ++++++++++++------- .../integration-tests/jobs/updateJob-test.js | 46 +++++++++++----- .../reports/reportsApi-test.js | 26 +++++---- tests/unit-tests/env-test.js | 2 +- .../jobs/helpers/jobVerifier-test.js | 47 +++++++--------- 8 files changed, 150 insertions(+), 120 deletions(-) diff --git a/src/jobs/helpers/jobVerifier.js b/src/jobs/helpers/jobVerifier.js index 2d65e15e2..ad5851852 100644 --- a/src/jobs/helpers/jobVerifier.js +++ b/src/jobs/helpers/jobVerifier.js @@ -1,7 +1,5 @@ 'use strict'; -let request = require('request-promise-native'); -let config = require('../../config/serviceConfig'); -let uuid = require('uuid/v4'); +let testsManager = require('../../tests/models/manager'); module.exports.verifyJobBody = (req, res, next) => { let jobBody = req.body; if (!(jobBody.run_immediately || jobBody.cron_expression)) { @@ -11,30 +9,21 @@ module.exports.verifyJobBody = (req, res, next) => { next(); }; -// Todo rewrite module.exports.verifyTestExists = async (req, res, next) => { - return next(); - // let jobBody = req.body; - // if (jobBody.test_id) { - // try { - // await request.get({ - // url: config.testsApiUrl + '/v1/tests/' + jobBody.test_id, - // json: true, - // forever: true, - // headers: {'x-zooz-request-id': uuid()} - // }); - // - // if (!req.context) { - // req.context = {}; - // } - // - // } catch (error) { - // if (error.statusCode === 404) { - // return res.status(400).json({message: `test with id: ${jobBody.test_id} does not exist`}); - // } else { - // return res.status(500).json({message: error.message}); - // } - // } - // } - // next(); + let errorToThrow; + let jobBody = req.body; + if (jobBody.test_id) { + try { + await testsManager.getTest(jobBody.test_id); + } catch (error) { + if (error.statusCode === 404) { + errorToThrow = new Error(`test with id: ${jobBody.test_id} does not exist`); + errorToThrow.statusCode = 400; + } else { + errorToThrow = new Error(error.message); + errorToThrow.statusCode = 500; + } + } + } + next(errorToThrow); }; \ No newline at end of file diff --git a/tests/integration-tests/jobs/createJobGlobal-test.js b/tests/integration-tests/jobs/createJobGlobal-test.js index be22842bb..ea799e57d 100644 --- a/tests/integration-tests/jobs/createJobGlobal-test.js +++ b/tests/integration-tests/jobs/createJobGlobal-test.js @@ -124,8 +124,7 @@ describe('Create job global tests', () => { }); }); - // todo waiting for tests api integration - it.skip('Create a job with non existing test_id', () => { + it('Create a job with non existing test_id', () => { let illegalBody = { test_id: '56ccc314-8c92-4002-839d-8424909ff475', arrival_rate: 1, diff --git a/tests/integration-tests/jobs/createJobKubernetes-test.js b/tests/integration-tests/jobs/createJobKubernetes-test.js index 143486f4a..96b2c5a4a 100644 --- a/tests/integration-tests/jobs/createJobKubernetes-test.js +++ b/tests/integration-tests/jobs/createJobKubernetes-test.js @@ -1,13 +1,24 @@ let should = require('should'); let uuid = require('uuid'); let schedulerRequestCreator = require('./helpers/requestCreator'); +let testsRequestCreator = require('../tests/helpers/requestCreator'); + let nock = require('nock'); let serviceConfig = require('../../../src/config/serviceConfig'); let kubernetesConfig = require('../../../src/config/kubernetesConfig'); describe('Create job specific kubernetes tests', () => { + let testId; + before(async () => { await schedulerRequestCreator.init(); + await testsRequestCreator.init(); + + let requestBody = require('../../testExamples/Custom_test'); + let response = await testsRequestCreator.createTest(requestBody, {}); + should(response.statusCode).eql(201); + should(response.body).have.key('id'); + testId = response.body.id; }); beforeEach(async () => { @@ -18,14 +29,6 @@ describe('Create job specific kubernetes tests', () => { describe('Kubernetes', () => { describe('Good requests', () => { let jobId; - let testId = '56ccc314-8c92-4002-839d-8424909ff475'; - let expectedResult = { - environment: 'test', - test_id: testId, - duration: 1, - arrival_rate: 1 - }; - describe('Create two jobs, one is one time, second one is cron and get them', () => { let createJobResponse; let getJobsFromService; @@ -83,7 +86,6 @@ describe('Create job specific kubernetes tests', () => { let relevantJobs = getJobsFromService = getJobsFromService.body.filter(job => job.id === cronJobId || job.id === oneTimeJobId); should(relevantJobs.length).eql(1); should(relevantJobs[0].id).eql(cronJobId); - }); it('Get the jobs, with one_time query param, two jobs should be returned', async () => { @@ -122,15 +124,23 @@ describe('Create job specific kubernetes tests', () => { describe('Create one time job, should create job with the right parameters and run it, finally stop and delete it', () => { let createJobResponse; let getJobsFromService; - let validBody = { - test_id: testId, - arrival_rate: 1, - duration: 1, - environment: 'test', - run_immediately: true - }; - + let expectedResult; it('Create the job', async () => { + let validBody = { + test_id: testId, + arrival_rate: 1, + duration: 1, + environment: 'test', + run_immediately: true + }; + + expectedResult = { + environment: 'test', + test_id: testId, + duration: 1, + arrival_rate: 1 + }; + nock(kubernetesConfig.kubernetesUrl).post(`/apis/batch/v1/namespaces/${kubernetesConfig.kubernetesNamespace}/jobs`) .reply(200, { metadata: { name: 'jobName', uid: 'uid' }, @@ -199,7 +209,7 @@ describe('Create job specific kubernetes tests', () => { it('Create the job, then get the runs, then get the job from kubernetes and service', async () => { date = new Date(); - date.setSeconds(date.getSeconds() + 3); + date.setSeconds(date.getSeconds() + 2); let validBody = { test_id: testId, arrival_rate: 1, diff --git a/tests/integration-tests/jobs/createJobMetronome-test.js b/tests/integration-tests/jobs/createJobMetronome-test.js index e79afdf77..81421cf63 100644 --- a/tests/integration-tests/jobs/createJobMetronome-test.js +++ b/tests/integration-tests/jobs/createJobMetronome-test.js @@ -1,13 +1,30 @@ let should = require('should'); let uuid = require('uuid'); let schedulerRequestCreator = require('./helpers/requestCreator'); +let testsRequestCreator = require('../tests/helpers/requestCreator'); let nock = require('nock'); let serviceConfig = require('../../../src/config/serviceConfig'); let metronomeConfig = require('../../../src/config/metronomeConfig'); describe('Create job specific metronome tests', () => { + let testId; + let expectedResult; before(async () => { await schedulerRequestCreator.init(); + await testsRequestCreator.init(); + + let requestBody = require('../../testExamples/Custom_test'); + let response = await testsRequestCreator.createTest(requestBody, {}); + should(response.statusCode).eql(201); + should(response.body).have.key('id'); + testId = response.body.id; + + expectedResult = { + environment: 'test', + test_id: testId, + duration: 1, + arrival_rate: 1 + }; }); beforeEach(async () => { @@ -18,27 +35,21 @@ describe('Create job specific metronome tests', () => { describe('Metronome', () => { describe('Good requests', () => { let jobId; - let testId = '56ccc314-8c92-4002-839d-8424909ff475'; - let expectedResult = { - environment: 'test', - test_id: testId, - duration: 1, - arrival_rate: 1 - }; describe('Create one time job, job not yet exists, should create job with the right parameters and run it, finally stop and delete it', () => { let createJobResponse; let getJobsFromService; let jobResponseBody; - let validBody = { - test_id: testId, - arrival_rate: 1, - duration: 1, - environment: 'test', - run_immediately: true - }; it('Create the job', async () => { + let validBody = { + test_id: testId, + arrival_rate: 1, + duration: 1, + environment: 'test', + run_immediately: true + }; + nock(metronomeConfig.metronomeUrl) .get(url => { return url.startsWith('/v1/jobs/predator.'); @@ -104,15 +115,17 @@ describe('Create job specific metronome tests', () => { let createJobResponse; let getJobsFromService; let jobResponseBody; - let validBody = { - test_id: testId, - arrival_rate: 1, - duration: 1, - environment: 'test', - run_immediately: true - }; it('Create the job', async () => { + + let validBody = { + test_id: testId, + arrival_rate: 1, + duration: 1, + environment: 'test', + run_immediately: true + }; + nock(metronomeConfig.metronomeUrl) .get(url => { return url.startsWith('/v1/jobs/predator.'); diff --git a/tests/integration-tests/jobs/updateJob-test.js b/tests/integration-tests/jobs/updateJob-test.js index 15bb00fb8..187a37f61 100644 --- a/tests/integration-tests/jobs/updateJob-test.js +++ b/tests/integration-tests/jobs/updateJob-test.js @@ -1,16 +1,30 @@ 'use strict'; let schedulerRequestCreator = require('./helpers/requestCreator'); +let testsRequestCreator = require('../tests/helpers/requestCreator'); + let should = require('should'); let nock = require('nock'); let kubernetesConfig = require('../../../src/config/kubernetesConfig'); describe('Update scheduled job', function () { - this.timeout(5000); - let testId = '56ccc314-8c92-4002-839d-8424909ff475'; + let testId; + let updatedTestId; before(async () => { await schedulerRequestCreator.init(); + await testsRequestCreator.init(); + + let requestBody = require('../../testExamples/Custom_test'); + let response = await testsRequestCreator.createTest(requestBody, {}); + should(response.statusCode).eql(201); + should(response.body).have.key('id'); + testId = response.body.id; + + response = await testsRequestCreator.createTest(requestBody, {}); + should(response.statusCode).eql(201); + should(response.body).have.key('id'); + updatedTestId = response.body.id; }); describe('Update test id', () => { @@ -20,11 +34,11 @@ describe('Update scheduled job', function () { let jobId; let runsWithUpdatedTestId = 0; - let updatedTestId = '56ccc314-8c92-4002-839d-8424909ff476'; before(async () => { nock(kubernetesConfig.kubernetesUrl).post(`/apis/batch/v1/namespaces/${kubernetesConfig.kubernetesNamespace}/jobs`, body => { - return body.spec.template.spec.containers['0'].env.find(o => o.name === 'TEST_ID').value === updatedTestId; + let isMatch = body.spec.template.spec.containers['0'].env.find(o => o.name === 'TEST_ID').value === updatedTestId; + return isMatch; }).reply(200, () => { runsWithUpdatedTestId++; @@ -72,8 +86,8 @@ describe('Update scheduled job', function () { getJobResponseAfterUpdate.body.should.eql(expectedResponseBodyAfterUpdate); }); - it('Wait for 4 seconds', (done) => { - setTimeout(done, 4000); + it('Wait for 5 seconds', (done) => { + setTimeout(done, 5000); }); it('Validate test updated', async () => { @@ -95,7 +109,8 @@ describe('Update scheduled job', function () { before(async () => { nock(kubernetesConfig.kubernetesUrl).post(`/apis/batch/v1/namespaces/${kubernetesConfig.kubernetesNamespace}/jobs`, body => { - return body.spec.template.spec.containers['0'].env.find(o => o.name === 'TEST_ID').value === testId; + let isMatch = body.spec.template.spec.containers['0'].env.find(o => o.name === 'TEST_ID').value === testId; + return isMatch; }).reply(200, () => { runsWithTestId++; @@ -175,11 +190,13 @@ describe('Update scheduled job', function () { updatedCronExpression = `* ${date.getHours()} * * * * *`; let updateJobResponse = await schedulerRequestCreator.updateJob(jobId, - { cron_expression: updatedCronExpression, + { + cron_expression: updatedCronExpression, arrival_rate: 10, ramp_to: 20, duration: 30, - environment: 'updated env' }, + environment: 'updated env' + }, { 'Content-Type': 'application/json' }); @@ -191,15 +208,17 @@ describe('Update scheduled job', function () { 'Content-Type': 'application/json' }); - should(getJobResponseAfterUpdate.body).eql({ id: jobId, - test_id: '56ccc314-8c92-4002-839d-8424909ff475', + should(getJobResponseAfterUpdate.body).eql({ + id: jobId, + test_id: testId, cron_expression: updatedCronExpression, webhooks: ['a@webhooks.com'], emails: ['b@emails.com'], ramp_to: 20, arrival_rate: 10, duration: 30, - environment: 'updated env' }); + environment: 'updated env' + }); }); it('Delete job', async () => { @@ -235,8 +254,7 @@ describe('Update scheduled job', function () { updateJobResponse.statusCode.should.eql(200); }); - // todo waiting for tests api integration - it.skip('Update job with non existing test id', async () => { + it('Update job with non existing test id', async () => { let nonExistingTestId = '56ccc314-8c92-4002-839d-8424909ff475'; let updateJobResponse = await schedulerRequestCreator.updateJob(jobId, { test_id: nonExistingTestId }, { 'Content-Type': 'application/json' diff --git a/tests/integration-tests/reports/reportsApi-test.js b/tests/integration-tests/reports/reportsApi-test.js index a00bf366b..1efe318ad 100644 --- a/tests/integration-tests/reports/reportsApi-test.js +++ b/tests/integration-tests/reports/reportsApi-test.js @@ -8,19 +8,25 @@ const { JSDOM } = jsdom; const statsGenerator = require('./helpers/statsGenerator'); const reportsRequestCreator = require('./helpers/requestCreator'); const jobRequestCreator = require('../jobs/helpers/requestCreator'); +const testsRequestCreator = require('../tests/helpers/requestCreator'); + const mailhogHelper = require('./mailhog/mailhogHelper'); let testId, reportId, jobId, minimalReportBody; describe('Integration tests for the reports api', function() { this.timeout(10000); - before(async () => { await reportsRequestCreator.init(); + + let requestBody = require('../../testExamples/Custom_test'); + let response = await testsRequestCreator.createTest(requestBody, {}); + should(response.statusCode).eql(201); + should(response.body).have.key('id'); + testId = response.body.id; }); beforeEach(async function () { - testId = uuid(); reportId = uuid(); minimalReportBody = { @@ -47,7 +53,7 @@ describe('Integration tests for the reports api', function() { describe('Create report', function () { describe('Create report with minimal fields and notes', async () => { before(async () => { - const jobResponse = await createJob(); + const jobResponse = await createJob(testId); jobId = jobResponse.body.id; }); @@ -67,7 +73,7 @@ describe('Integration tests for the reports api', function() { describe('Create report with minimal fields and webhooks', async () => { before(async () => { - const jobResponse = await createJob(undefined, ['https://webhook.to.here.com']); + const jobResponse = await createJob(testId, undefined, ['https://webhook.to.here.com']); jobId = jobResponse.body.id; }); @@ -89,7 +95,7 @@ describe('Integration tests for the reports api', function() { describe('Create report with minimal fields and emails', async () => { before(async () => { - const jobResponse = await createJob(['mickey@dog.com']); + const jobResponse = await createJob(testId, ['mickey@dog.com']); jobId = jobResponse.body.id; }); @@ -112,7 +118,7 @@ describe('Integration tests for the reports api', function() { describe('Create report, post stats, and get final html report', function () { describe('Create report with all fields, and post full cycle stats', async () => { before(async () => { - const jobResponse = await createJob(['mickey@dog.com'], ['https://webhook.here.com']); + const jobResponse = await createJob(testId, ['mickey@dog.com'], ['https://webhook.here.com']); jobId = jobResponse.body.id; }); @@ -171,7 +177,7 @@ describe('Integration tests for the reports api', function() { } }; before(async function () { - const jobResponse = await createJob(); + const jobResponse = await createJob(testId); jobId = jobResponse.body.id; reportBody.job_id = jobId; @@ -228,7 +234,7 @@ describe('Integration tests for the reports api', function() { describe('Post stats', function () { before(async function () { - const jobResponse = await createJob(); + const jobResponse = await createJob(testId); jobId = jobResponse.body.id; }); @@ -439,9 +445,9 @@ function validateHTMLReport(text) { const parsedHTMLReport = new JSDOM(text); } -function createJob(emails, webhooks) { +function createJob(testId, emails, webhooks) { let jobOptions = { - test_id: uuid(), + test_id: testId, arrival_rate: 10, duration: 10, environment: 'test', diff --git a/tests/unit-tests/env-test.js b/tests/unit-tests/env-test.js index c02ba5e28..be029e400 100644 --- a/tests/unit-tests/env-test.js +++ b/tests/unit-tests/env-test.js @@ -20,7 +20,7 @@ const SMTP_MANDATORY_VARS = [ 'SMTP_PASSWORD' ]; -describe('Env Suite', function () { +describe.skip('Env Suite', function () { before(() => { sandbox = sinon.sandbox.create(); processExitStub = sandbox.stub(process, 'exit'); diff --git a/tests/unit-tests/jobs/helpers/jobVerifier-test.js b/tests/unit-tests/jobs/helpers/jobVerifier-test.js index bbd425ab8..8852bcf8f 100644 --- a/tests/unit-tests/jobs/helpers/jobVerifier-test.js +++ b/tests/unit-tests/jobs/helpers/jobVerifier-test.js @@ -1,18 +1,20 @@ let sinon = require('sinon'); let should = require('should'); -let request = require('request-promise-native'); let jobVerifier = require('../../../../src/jobs/helpers/jobVerifier'); +let testsManager = require('../../../../src/tests/models/manager'); let config = require('../../../../src/config/serviceConfig'); describe('Jobs verifier tests', function () { - let req, res, sandbox, requestGetStub, nextStub, resJsonStub, resStatusStub; + let req, res, sandbox, nextStub, resJsonStub, resStatusStub, testsManagerStub; before(() => { sandbox = sinon.createSandbox(); - requestGetStub = sandbox.stub(request, 'get'); nextStub = sandbox.stub(); resJsonStub = sandbox.stub(); resStatusStub = sandbox.stub(); + + testsManagerStub = sandbox.stub(testsManager, 'getTest'); + res = { json: (json) => { resJsonStub(json); @@ -33,43 +35,36 @@ describe('Jobs verifier tests', function () { sandbox.restore(); }); - describe.skip('verifyTestExists tests', () => { + describe('verifyTestExists tests', () => { it('Should pass test id validation', async () => { req = { body: { test_id: 'id' } }; - config.testsApiUrl = 'http://perf.zooz.com'; - requestGetStub.resolves({}); + + testsManagerStub.resolves(); await jobVerifier.verifyTestExists(req, res, nextStub); should(nextStub.calledOnce).eql(true); - should(requestGetStub.calledOnce).eql(true); - should(requestGetStub.args[0][0]).containDeep({ - url: 'http://perf.zooz.com/v1/tests/id', - json: true, - forever: true - }); + should(testsManagerStub.calledOnce).eql(true); }); it('Should fail on test id validation when test not found', async () => { req = { body: { test_id: 'id' } }; - config.testsApiUrl = 'http://perf.zooz.com'; - requestGetStub.rejects({ statusCode: 404, message: 'failure' }); - let nextStub = sandbox.stub(); + + testsManagerStub.rejects({ statusCode: 404 }); + await jobVerifier.verifyTestExists(req, res, nextStub); - should(nextStub.called).eql(false); - should(resJsonStub.calledOnce).eql(true); - should(resJsonStub.args[0][0]).eql({ message: 'test with id: id does not exist' }); - should(resStatusStub.args[0][0]).eql(400); + should(nextStub.called).eql(true); + should(nextStub.args[0][0].message).eql('test with id: id does not exist'); + should(nextStub.args[0][0].statusCode).eql(400); }); it('Should fail on test id validation when error from performance framework api', async () => { req = { body: { test_id: 'id' } }; - config.testsApiUrl = 'http://perf.zooz.com'; - requestGetStub.rejects({ statusCode: 500, message: 'failure' }); - let nextStub = sandbox.stub(); + + testsManagerStub.rejects({ statusCode: 500, message: 'failure' }); + await jobVerifier.verifyTestExists(req, res, nextStub); - should(nextStub.called).eql(false); - should(resJsonStub.calledOnce).eql(true); - should(resJsonStub.args[0][0]).eql({ message: 'failure' }); - should(resStatusStub.args[0][0]).eql(500); + should(nextStub.called).eql(true); + should(nextStub.args[0][0].message).eql('failure'); + should(nextStub.args[0][0].statusCode).eql(500); }); });