From 5fbe77fa36f1ac5c6376dc8f9493dc1059173e26 Mon Sep 17 00:00:00 2001 From: Xiao Gui Date: Sat, 6 Jun 2020 12:25:19 +0200 Subject: [PATCH 1/4] fix api spec --- app/controller/api/api.spec.js | 71 +++++++++++++++++----------------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/app/controller/api/api.spec.js b/app/controller/api/api.spec.js index 8e882852..3c521466 100644 --- a/app/controller/api/api.spec.js +++ b/app/controller/api/api.spec.js @@ -9,21 +9,10 @@ const fs = require('fs') const express = require('express') const app = express() -let authorized = false - function buildFileID({source, slice}) { return `${source}&slice=${slice}`; } -app.use((req, res, next) => { - if (authorized) { - req.user = { - username: 'bobby' - } - } - return next() -}) - app.use(bodyParser.urlencoded({ extended: false })) /** @@ -32,12 +21,22 @@ app.use(bodyParser.urlencoded({ extended: false })) * these tests only check if api works as intended */ let authenticated = false -const USER = 'bobby' + +const USER_BOB = { + username: 'bob' +} +const USER_ALICE = { + username: 'alice' +} +const USER_CINDY = { + username: 'cindy' +} + app.use((req, res, next) => { if (authenticated) { - req.user = { - username: USER - } + req.user = USER_BOB + } else { + req.user = undefined } next() }) @@ -84,11 +83,7 @@ describe('controller/api/index.js', () => { findAnnotations = sinon.fake.resolves(returnFoundAnnotation ? annoationInDb : {Regions: []}) queryProject = sinon.fake.resolves({ collaborators: { - list: [{ - username: 'alice' - }, { - username: 'bob' - }] + list: [ USER_ALICE, USER_BOB ] } }) @@ -114,7 +109,21 @@ describe('controller/api/index.js', () => { describe('saveFromGUI', () => { describe('/GET', () => { - it('should return status 200', (done) => { + it('if user not part of project, should return 403', (done) => { + authenticated = false + chai.request(url) + .get('/') + .query({ + project: 'testProject' + }) + .end((err, res) => { + expect(res).to.have.status(403) + done() + }) + }) + + it('if user is part of project, should return status 200', (done) => { + authenticated = true chai.request(url) .get('/') .query({ @@ -165,11 +174,9 @@ describe('controller/api/index.js', () => { project }) .then(res => { + findAnnotations.callArg assert(findAnnotations.calledWith({ fileID: `${source}&slice=${slice}`, - user: { - $in: ['alice', 'bob'] - }, project })) }) @@ -185,9 +192,6 @@ describe('controller/api/index.js', () => { .then(res => { assert(findAnnotations.calledWith({ fileID: `undefined&slice=undefined`, - user: { - $in: ['alice', 'bob'] - }, project })) }) @@ -213,19 +217,16 @@ describe('controller/api/index.js', () => { .send(sendItem) const doTest = async () => { - await getTest() - assert(updateAnnotation.calledWith({ - fileID: '/path/to/json.json&slice=24', - user: 'anyone', - ...rest - })) + authenticated = false + const { status } = await getTest() + assert(updateAnnotation.notCalled) + expect(status).to.equal(403) authenticated = true - await getTest() assert(updateAnnotation.calledWith({ fileID: '/path/to/json.json&slice=24', - user: 'bobby', + user: USER_BOB.username, ...rest })) } From a1deb15e194c4fc2d41672ce833d9b65f0b14a9b Mon Sep 17 00:00:00 2001 From: Xiao Gui Date: Sat, 6 Jun 2020 12:38:47 +0200 Subject: [PATCH 2/4] update circleci node version (optional try catch) removed dep on pptr, now throws asking user to manually install --- .circleci/config.yml | 3 ++- package.json | 1 - test.js | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ade08a06..88622889 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -2,11 +2,12 @@ version : 2 jobs: build: docker: - - image: circleci/node:8-browsers + - image: circleci/node:10-browsers - image: circleci/mongo:3.4 steps: - checkout - run: npm install + - run: npm i puppeteer - run: name: Run server in background command: 'npm start' diff --git a/package.json b/package.json index 3f2d7257..52e81173 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,6 @@ "mock-fs": "^4.5.0", "morgan": "^1.9.1", "nock": "^10.0.6", - "puppeteer": "latest", "sinon": "^7.3.1" }, "author": "Roberto Toro (http://neuroanatomy.github.io)", diff --git a/test.js b/test.js index 72eeedce..a25ee0df 100644 --- a/test.js +++ b/test.js @@ -1,5 +1,11 @@ 'use strict'; const UI = require('./test/UI'); +try { + require('puppeteer') +} catch (e) { + console.warn(`[microdraw]: dependency error: puppeteer needs to be installed manually. - npm i puppeteer`) + process.exit(1) +} const puppeteer = require('puppeteer'); From c3d3a30f2194b986fdb755872e86dbe2923c912e Mon Sep 17 00:00:00 2001 From: Xiao Gui Date: Sat, 6 Jun 2020 13:44:42 +0200 Subject: [PATCH 3/4] 10s timeout for microdraw.spec.js --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 88622889..5579a103 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -19,7 +19,7 @@ jobs: # hence run before Query host - run: name: Test microdraw.js - command: npm run mocha -- ./test/e2e/microdraw.spec.js + command: npm run mocha -- ./test/e2e/microdraw.spec.js --timeout 10000 - run: name: Query host command: 'node host.js' From 6dd1d680d34d24478c13562ab5f4a9d9fa0faf59 Mon Sep 17 00:00:00 2001 From: Xiao Gui Date: Sat, 6 Jun 2020 20:01:34 +0200 Subject: [PATCH 4/4] rearranged spec file for easier diagnosis --- app/controller/api/api.spec.js | 514 ++++++++++++++++++++++----------- 1 file changed, 338 insertions(+), 176 deletions(-) diff --git a/app/controller/api/api.spec.js b/app/controller/api/api.spec.js index 3c521466..565bfbb1 100644 --- a/app/controller/api/api.spec.js +++ b/app/controller/api/api.spec.js @@ -13,14 +13,19 @@ function buildFileID({source, slice}) { return `${source}&slice=${slice}`; } -app.use(bodyParser.urlencoded({ extended: false })) +// app.use(bodyParser.urlencoded({ extended: false })) +app.use(bodyParser.urlencoded({ extended: true })) /** * simulate authenication status * testing authentication is not the aim of this test suite * these tests only check if api works as intended */ -let authenticated = false +let authenticatedUser = null + +const USER_ANONYMOUSE = { + username: 'anyone' +} const USER_BOB = { username: 'bob' @@ -33,10 +38,8 @@ const USER_CINDY = { } app.use((req, res, next) => { - if (authenticated) { - req.user = USER_BOB - } else { - req.user = undefined + if (authenticatedUser) { + req.user = authenticatedUser } next() }) @@ -68,6 +71,16 @@ describe('sinon works', () => { hello: 'world' })) }) + + it('stub can dynamically change return val', () => { + const stub = sinon.stub() + let flag = true + stub.callsFake(() => flag) + expect(stub()).to.equal(true) + + flag = false + expect(stub()).to.equal(false) + }) }) describe('controller/api/index.js', () => { @@ -75,17 +88,27 @@ describe('controller/api/index.js', () => { Regions: ['hello: world', 'foobar'] } let _server, + queryPublicProject = false, + publicProject = { + owner: USER_BOB.username, + collaborators: { + list: [ USER_ALICE, USER_BOB, USER_ANONYMOUSE ] + } + }, + privateProject = { + owner: USER_ALICE.username, + collaborators: { + list: [ USER_ALICE, USER_BOB ] + } + }, port = 10002, url = `http://127.0.0.1:${port}`, returnFoundAnnotation = true, - updateAnnotation = sinon.fake.resolves(), - findAnnotations = sinon.fake.resolves(returnFoundAnnotation ? annoationInDb : {Regions: []}) - queryProject = sinon.fake.resolves({ - collaborators: { - list: [ USER_ALICE, USER_BOB ] - } - }) + updateAnnotation = sinon.stub().resolves(), + findAnnotations = sinon.stub().callsFake(() => Promise.resolve( returnFoundAnnotation ? annoationInDb : {Regions: []} )), + queryProject = sinon.stub().callsFake(() => Promise.resolve(queryPublicProject ? publicProject : privateProject)) + before(() => { app.db = { @@ -109,32 +132,76 @@ describe('controller/api/index.js', () => { describe('saveFromGUI', () => { describe('/GET', () => { - it('if user not part of project, should return 403', (done) => { - authenticated = false - chai.request(url) - .get('/') - .query({ - project: 'testProject' - }) - .end((err, res) => { - expect(res).to.have.status(403) - done() - }) + + describe('public project', () => { + before(() => { + queryPublicProject = true + }) + + it('if user not part of project, should return 200', (done) => { + authenticatedUser = null + chai.request(url) + .get('/') + .query({ + project: 'testProject' + }) + .end((err, res) => { + expect(!!err).to.be.false + expect(res).to.have.status(200) + done() + }) + }) + + it('if user is part of project, should return status 200', (done) => { + authenticatedUser = USER_BOB + chai.request(url) + .get('/') + .query({ + project: 'testProject' + }) + .end((err, res) => { + expect(!!err).to.be.false + expect(res).to.have.status(200) + done() + }) + }) }) - it('if user is part of project, should return status 200', (done) => { - authenticated = true - chai.request(url) - .get('/') - .query({ - project: 'testProject' - }) - .end((err, res) => { - expect(res).to.have.status(200) - done() - }) + describe('private project', () => { + + before(() => { + queryPublicProject = false + }) + + it('if user not part of project, should return 403', (done) => { + authenticatedUser = null + chai.request(url) + .get('/') + .query({ + project: 'testProject' + }) + .end((err, res) => { + expect(!!err).to.be.false + expect(res).to.have.status(403) + done() + }) + }) + + it('if user is part of project, should return status 200', (done) => { + authenticatedUser = USER_BOB + chai.request(url) + .get('/') + .query({ + project: 'testProject' + }) + .end((err, res) => { + expect(!!err).to.be.false + expect(res).to.have.status(200) + done() + }) + }) }) - + it('fetching annotation from different project names work', (done) => { const getTest = name => chai.request(url) .get('/') @@ -163,43 +230,46 @@ describe('controller/api/index.js', () => { }) describe('/POST', () => { + describe('findAnnotation', () => { + it('should be called with correct arg', done => { - it('varying source and slice should call findAnnotation with the correct call sig', (done) => { - const project = 'projectIsAlreadyTestedAbove' - const getTest = ({ source, slice, project }) => chai.request(url) - .get('/') - .query({ - source, - slice, - project - }) - .then(res => { - findAnnotations.callArg - assert(findAnnotations.calledWith({ - fileID: `${source}&slice=${slice}`, - project - })) - }) - Promise.all([ - getTest({ source: 'test1', slice: '123', project }), - getTest({ source: 'test2', slice: '1234', project }), - getTest({ source: '', slice: '', project }), - chai.request(url) + const project = 'projectIsAlreadyTestedAbove' + const getTest = ({ source, slice, project }) => chai.request(url) .get('/') .query({ + source, + slice, project }) .then(res => { + findAnnotations.callArg assert(findAnnotations.calledWith({ - fileID: `undefined&slice=undefined`, + fileID: `${source}&slice=${slice}`, project })) }) - ]).then(() => done()) - .catch(done) + + Promise.all([ + getTest({ source: 'test1', slice: '123', project }), + getTest({ source: 'test2', slice: '1234', project }), + getTest({ source: '', slice: '', project }), + chai.request(url) + .get('/') + .query({ + project + }) + .then(res => { + assert(findAnnotations.calledWith({ + fileID: `undefined&slice=undefined`, + project + })) + }) + ]).then(() => done()) + .catch(done) + }) }) - it('varying authenticated state should result in different username being associated', (done) => { + describe('varying users', () => { const sendItem = { action: 'save', source: '/path/to/json.json', @@ -208,35 +278,56 @@ describe('controller/api/index.js', () => { annotation: 'testworld', project: 'alreadyTestedPreviously' } - const { action, source, slice, ...rest } = sendItem + let res - const getTest = () => chai.request(url) - .post('/') - .set('content-type', 'application/x-www-form-urlencoded') - .send(sendItem) - - const doTest = async () => { - authenticated = false - const { status } = await getTest() - assert(updateAnnotation.notCalled) - expect(status).to.equal(403) - - authenticated = true - await getTest() - assert(updateAnnotation.calledWith({ - fileID: '/path/to/json.json&slice=24', - user: USER_BOB.username, - ...rest - })) - } + beforeEach(async () => { + res = await chai.request(url) + .post('/') + .set('content-type', 'application/x-www-form-urlencoded') + .send(sendItem) + }) - doTest() - .then(() => done()) - .catch(done) + it('ok', () => { + assert(true) + }) + + describe('unauthenticated user', () => { + before(() => { + authenticatedUser = null + }) + + it('should return 403', () => { + const { status } = res + expect(status).to.equal(403) + }) + }) + + describe('authenicated user, with correct permission', () => { + before(() => { + authenticatedUser = USER_BOB + }) + + it('should return 200', () => { + const { status } = res + expect(status).to.equal(200) + }) + + it('updateAnnotation should be called', () => { + assert(updateAnnotation.called) + }) + + it('updateAnnotation called with correct arg', () => { + + assert(updateAnnotation.calledWith({ + fileID: '/path/to/json.json&slice=24', + user: USER_BOB.username, + ...rest + })) + }) + }) }) }) - }) describe('saveFromAPI', () => { @@ -298,6 +389,18 @@ describe('controller/api/index.js', () => { project: 'alreadyTestedPreviously' }) + let readFileStub + + before(() => { + readFileStub = sinon.stub(fs, 'readFileSync') + readFileStub.withArgs(FILENAME1).returns(Buffer.from(JSON.stringify(correctJson))) + readFileStub.withArgs(FILENAME2).returns(Buffer.from(JSON.stringify(incorrectJSON))) + }) + + after(() => { + readFileStub.restore() + }) + const getTest = (queryParam, { illFormedJson = false } = {}) => chai.request(url) .post('/upload') .attach( @@ -311,107 +414,166 @@ describe('controller/api/index.js', () => { ) .query(queryParam) - let readFileStub describe('/POST', () => { - beforeEach(() => { - authenticated = true - returnFoundAnnotation = true - - readFileStub = sinon.stub(fs, 'readFileSync') - readFileStub.withArgs(FILENAME1).returns(Buffer.from(JSON.stringify(correctJson))) - readFileStub.withArgs(FILENAME2).returns(Buffer.from(JSON.stringify(incorrectJSON))) - }) - - afterEach(() => { - readFileStub.restore() - }) - - describe('authenciation status behave expectedly', () => { - it('unauthenticated user gets 401, and findannotations/update annotations NOT called', (done) => { - authenticated = false - - const doTest = async () => { - - const res = await getTest({ ...getQueryParam({ action: 'save' }) }) - - assert(findAnnotations.notCalled) - assert(updateAnnotation.notCalled) - assert(res.status === 401) - expect(res.body).to.deep.equal({ - msg: 'Invalid user' - }) - - const res2 = await getTest({ ...getQueryParam({ action: 'append' }) }) + describe('unauthenicated user', () => { + describe('action=save', () => { - assert(findAnnotations.notCalled) - assert(updateAnnotation.notCalled) - assert(res2.status === 401) - expect(res2.body).to.deep.equal({ - msg: 'Invalid user' + let res + before(async () => { + authenticatedUser = null + res = await getTest({ ...getQueryParam({ action: 'save' }) }) }) - } - - doTest() - .then(() => done()) - .catch(done) - }) - - it('authenticated user gets 200, findannotation and updateannotation called', (done) => { - authenticated = true - - const doTest = async () => { - - const param = getQueryParam({ action: 'save' }) - const { source, slice, action, project, ...rest } = param - const res = await getTest(param) - - /** - * action=save does not call findAnnotation - */ - assert(findAnnotations.notCalled) - assert(updateAnnotation.calledWith({ - fileID: buildFileID({ source, slice }), - user: 'bobby', - project, - annotation: JSON.stringify({ - Regions: correctJson.map(v => v.annotation) - }), - ...rest, - - })) - assert(res.status === 200) - expect(res.body).to.deep.equal({ - msg: 'Annotation successfully saved' + + it('returns 401', () => { + assert(res.status === 401) }) - - const res2 = await getTest({ ...getQueryParam({ action: 'append' }) }) - - assert(findAnnotations.calledWith({ - fileID: buildFileID({ source, slice}), - user: 'bobby', - project - })) - assert(updateAnnotation.calledWith({ - fileID: buildFileID({ source, slice}), - user: 'bobby', - project, - annotation: JSON.stringify({ - Regions: correctJson.map(v => v.annotation) - }), - ...rest, - - })) - assert(res2.status === 200) - expect(res2.body).to.deep.equal({ - msg: 'Annotation successfully saved' + + it('status text is as expected', () => { + expect(res.body).to.deep.equal({ + msg: 'Invalid user' + }) + }) + + it('findAnnotation is NOT called', () => { + assert(findAnnotations.notCalled) + }) + + it('updateAnnotation NOT called', () => { + assert(updateAnnotation.notCalled) }) - } + }) + describe('action=append', () => { - doTest().then(() => done()).catch(done) + let res + before(async () => { + authenticatedUser = null + res = await getTest({ ...getQueryParam({ action: 'append' }) }) + }) + + it('returns 401', () => { + assert(res.status === 401) + }) + + it('status text is as expected', () => { + expect(res.body).to.deep.equal({ + msg: 'Invalid user' + }) + }) + + it('findAnnotation is NOT called', () => { + assert(findAnnotations.notCalled) + }) + + it('updateAnnotation NOT called', () => { + assert(updateAnnotation.notCalled) + }) + }) }) + + // describe('authenicated user', () => { + + // describe('action=save', () => { + // let res, param + // before(async () => { + // authenticatedUser = USER_BOB + + // param = getQueryParam({ action: 'save' }) + + // const { source: paramSource, slice, action, project, ...rest } = param + // res = await getTest(param) + // }) + + // it('returns 200', () => { + // assert(res.status === 200) + // }) + + // it('return body text is as expected', () => { + // expect(res.body).to.deep.equal({ + // msg: 'Annotation successfully saved' + // }) + // }) + + // /** + // * action=save does not call findAnnotation + // */ + // it('findAnnotation not called', () => { + // assert(findAnnotations.notCalled) + // }) + + // it('updateAnnotation is called', () => { + // assert(updateAnnotation.called) + // }) + + // it('updateAnnotation called with correct param', () => { + + // const { source, slice, ...rest } = param + + // console.log('--------------------') + // console.log(updateAnnotation.firstCall.args) + // assert(updateAnnotation.calledWith({ + // fileID: buildFileID({ source, slice }), + // user: USER_BOB.username, + // project, + // annotation: JSON.stringify({ + // Regions: correctJson.map(v => v.annotation) + // }), + // ...rest, + // })) + // }) + // }) + + // describe('action=append', () => { + // let res2 + + // before(async () => { + // authenticatedUser = USER_BOB + // res2 = await getTest({ ...getQueryParam({ action: 'append' }) }) + // }) + + // it('returns 200', () => { + // assert(res2.status === 200) + // }) + + // it('body text as expected', () => { + // expect(res2.body).to.deep.equal({ + // msg: 'Annotation successfully saved' + // }) + // }) + + // it('findAnnotation is called', () => { + // assert(findAnnotations.called) + // }) + + // it('findAnnotation called with correct param', () => { + // assert(findAnnotations.calledWith({ + // fileID: buildFileID({ source, slice}), + // user: USER_BOB.username, + // project + // })) + // }) + + // it('update annotation called', () => { + // assert(updateAnnotation.called) + // }) + + // it('updatedannotation called with correct param', () => { + + // assert(updateAnnotation.calledWith({ + // fileID: buildFileID({ source, slice}), + // user: USER_BOB.username, + // project, + // annotation: JSON.stringify({ + // Regions: correctJson.map(v => v.annotation) + // }), + // ...rest, + + // })) + // }) + // }) + // }) }) }) })