From 251c9c0857535ee20bbcb2fe4c79582cd4b04789 Mon Sep 17 00:00:00 2001 From: Loic Huder <loic.huder@esrf.fr> Date: Mon, 24 Aug 2020 16:29:21 +0200 Subject: [PATCH 1/2] Fix bug in deleteItem and getParcelsByStatus --- app/controllers/item.controller.js | 2 +- app/controllers/parcel.controller.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/item.controller.js b/app/controllers/item.controller.js index bf49be11..8d2ffb7d 100644 --- a/app/controllers/item.controller.js +++ b/app/controllers/item.controller.js @@ -114,7 +114,7 @@ function deleteItem(req, res) { global.gLogger.warn("Delete item from parcel returned not ok"); } helper - .findParcel({ investigationId: req.params.investigationId, _id: req.params.parcelId }) + .findParcels({ investigationId: req.params.investigationId, _id: req.params.parcelId }) .then((parcel) => { /**Remove item from Items collection */ Item.deleteOne({ _id: req.body._id }) diff --git a/app/controllers/parcel.controller.js b/app/controllers/parcel.controller.js index a008b178..05a479b0 100644 --- a/app/controllers/parcel.controller.js +++ b/app/controllers/parcel.controller.js @@ -177,7 +177,7 @@ function getParcelsByStatus(req, res) { //TODO: this would be much more efficient if we can use the query instead of filtering all results // `status` is a virtual field and cannot be queried directly (https://mongoosejs.com/docs/tutorials/virtuals.html#limitations) helper - .findParcel({}) + .findParcels({}) .then((parcels) => { const filteredByStatus = parcels.filter((parcel) => parcel.status === req.params.status.toUpperCase()); res.send(filteredByStatus); -- GitLab From cb5ec6436835591d99f63755dafdd07afb38e9ed Mon Sep 17 00:00:00 2001 From: Loic Huder <loic.huder@esrf.fr> Date: Mon, 24 Aug 2020 16:29:42 +0200 Subject: [PATCH 2/2] Updated address and parcel tests --- test/global.js | 5 +- test/routes/test.address.routes.js | 49 ++++++++------------ test/routes/test.parcel.routes.js | 74 ++++++++++++------------------ 3 files changed, 50 insertions(+), 78 deletions(-) diff --git a/test/global.js b/test/global.js index 336935b1..64f3c68b 100644 --- a/test/global.js +++ b/test/global.js @@ -4,7 +4,6 @@ global.gResource = require("./resources/global.resource.js"); global.gResource.investigations = require("./resources/investigations.resource.js"); global.chai = require("chai"); global.expect = global.chai.expect; -global.should = global.chai.should(); global.chaiHttp = require("chai-http"); global.chai.use(global.chaiHttp); @@ -27,7 +26,7 @@ before((done) => { //initialize the icat+ server global.gServer = require("../server.js"); global.gServer.onCacheInitialized = function () { - global.gRequester = chai.request(global.gServer).keepOpen(); + global.gRequester = global.chai.request(global.gServer).keepOpen(); console.log("Server is listening..."); setTimeout(() => { done(); @@ -47,7 +46,7 @@ before((done) => { } else { global.gServer = require("../server.js"); global.gServer.onCacheInitialized = function () { - global.gRequester = chai.request(global.gServer).keepOpen(); + global.gRequester = global.chai.request(global.gServer).keepOpen(); console.log("Server is listening..."); setTimeout(() => { done(); diff --git a/test/routes/test.address.routes.js b/test/routes/test.address.routes.js index 33c6363a..1393d8d3 100644 --- a/test/routes/test.address.routes.js +++ b/test/routes/test.address.routes.js @@ -14,9 +14,9 @@ describe("Tracking Address", () => { try { const response = await addressHelper.createAddress(element.user, element.address, element.investigationId); expect(response.status).to.equal(element.expected.status); - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -29,9 +29,9 @@ describe("Tracking Address", () => { try { const getAddressByInvestigationIdResponse = await addressHelper.getAddressByInvestigationId(element.user, element.investigationId); expect(getAddressByInvestigationIdResponse.status).to.equal(element.expected.status); - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -40,18 +40,16 @@ describe("Tracking Address", () => { * Get Address by sessionId */ describe("GET /tracking/{sessionId}/address", () => { - it.each(addressResource.getAddressByUsername, "%s", ["description"], (element, next) => { - sessionHelper.doGetSession(element.user).then((getSessionResponse) => { + it.each(addressResource.getAddressByUsername, "%s", ["description"], async (element, next) => { + try { + const getSessionResponse = await sessionHelper.doGetSession(element.user); expect(getSessionResponse.status).equal(200); - global.gRequester - .get(`/tracking/${getSessionResponse.body.sessionId}/address`) - .set("Content-Type", "application/json") - .send() - .end((err, response) => { - expect(response.status).to.equal(element.expected.status); - next(); - }); - }); + const response = await global.gRequester.get(`/tracking/${getSessionResponse.body.sessionId}/address`).set("Content-Type", "application/json").send(); + expect(response.status).to.equal(element.expected.status); + return next(); + } catch (err) { + return next(err); + } }); }); @@ -72,11 +70,11 @@ describe("Tracking Address", () => { const updateAddressResponse = await addressHelper.updateAddress(element.addressUpdator, updatedAddress, element.investigationId); expect(updateAddressResponse.status).to.equal(element.expected.status); if (updateAddressResponse.status < 300) { - testAdress(updateAddressResponse.body, element.expected.address); + expect(updateAddressResponse.body).to.include(element.expected.address); } - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -99,21 +97,10 @@ describe("Tracking Address", () => { if (element.expected.status < 300) { expect(deleteAddressResponse.body.status).to.be.equal("REMOVED"); } - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); }); - -/** - * Test function which tests that an actual tag corresponds to the expected tag. - * @param {object} actualTag the tag to test - * @param {object} expectedTag the expected tag - */ -function testAdress(actualAddress, expectedAddres) { - for (const [key, value] of Object.entries(expectedAddres)) { - expect(actualAddress[key]).to.equal(value); - } -} diff --git a/test/routes/test.parcel.routes.js b/test/routes/test.parcel.routes.js index d32693cd..70b5bae9 100644 --- a/test/routes/test.parcel.routes.js +++ b/test/routes/test.parcel.routes.js @@ -15,19 +15,7 @@ describe("Tracking Parcel", () => { const createParcelResponse = await parcelHelper.createParcel(element.parcelCreator, element.parcel, element.investigationId, createShipmentResponse.body._id); expect(createParcelResponse.status).to.equal(200); - /**var addItemResponse = await parcelHelper.addItem( - element.user, - element.investigationId, - createParcelResponse.body.parcels[0], - element.item - ); - expect(addItemResponse.status).to.equal(element.expected.status); - **/ - for (let i = 0; i < element.statuses.length; i++) { - const status = element.statuses[i].status; - const user = element.statuses[i].user; - const containsDangerousGoods = element.statuses[i].containsDangerousGoods; - + element.statuses.map(async ({ status, user, containsDangerousGoods }) => { const setParcelStatusResponse = await parcelHelper.setParcelStatus(user, element.investigationId, createParcelResponse.body.parcels[0], status, { containsDangerousGoods, }); @@ -35,11 +23,11 @@ describe("Tracking Parcel", () => { const getParcelByIdResponse = await parcelHelper.getParcelById(element.user, createParcelResponse.body.investigationId, createParcelResponse.body.parcels[0]); expect(getParcelByIdResponse.body.status).to.equal(status); - } + }); - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -52,9 +40,9 @@ describe("Tracking Parcel", () => { element.parcel.shipmentId = createShipmentResponse.body._id; const createParcelResponse = await parcelHelper.createParcel(element.parcelCreator, element.parcel, element.investigationId, createShipmentResponse.body._id); expect(createParcelResponse.status).to.equal(element.expected.status); - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -71,9 +59,9 @@ describe("Tracking Parcel", () => { expect(parcel.status).to.equal(element.status); }); } - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -91,26 +79,24 @@ describe("Tracking Parcel", () => { if (element.expected.status < 300) { expect(getParcelByShipmentIdResponse.body).to.be.an("array").to.have.lengthOf(1); } - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); describe("GET /tracking/{sessionId}/parcel", () => { - it.each(resource.getParcelBySessionId, "%s", ["description"], (element, next) => { - sessionHelper.doGetSession(element.user).then((getSessionResponse) => { + it.each(resource.getParcelBySessionId, "%s", ["description"], async (element, next) => { + try { + const getSessionResponse = await sessionHelper.doGetSession(element.user); expect(getSessionResponse.status).equal(200); - global.gRequester - .get(`/tracking/${getSessionResponse.body.sessionId}/parcel`) - .set("Content-Type", "application/json") - .send() - .end((err, response) => { - expect(response.status).to.equal(element.expected.status); - next(); - }); - }); + const response = await global.gRequester.get(`/tracking/${getSessionResponse.body.sessionId}/parcel`).set("Content-Type", "application/json").send(); + expect(response.status).to.equal(element.expected.status); + return next(); + } catch (e) { + return next(e); + } }); }); @@ -129,9 +115,9 @@ describe("Tracking Parcel", () => { expect(getParcelByShipmentIdResponse.body).not.to.be.an("array"); expect(parcelId).to.be.equal(getParcelByShipmentIdResponse.body._id); } - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -153,9 +139,9 @@ describe("Tracking Parcel", () => { if (element.expected.status < 300) { expect(addItemResponse.body.items).to.be.an("array").to.have.lengthOf(1); } - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -163,7 +149,7 @@ describe("Tracking Parcel", () => { /** * Creates a shipment, then a parcel, then removes the parcel */ - describe("XXDELETE /tracking/:sessionId/investigation/id/:investigationId/parcel/id/:parcelId", () => { + describe("DELETE /tracking/:sessionId/investigation/id/:investigationId/parcel/id/:parcelId", () => { it.each(resource.deleteParcel, "%s", ["description"], async (element, next) => { try { const createShipmentResponse = await shipmentHelper.createShipment(element.shipmentCreator, element.shipment, element.investigationId); @@ -176,9 +162,9 @@ describe("Tracking Parcel", () => { expect(deleteParcelResponse.status).to.equal(element.expected.status); - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -203,9 +189,9 @@ describe("Tracking Parcel", () => { const removeItemResponse = await parcelHelper.removeItem(element.user, element.investigationId, createParcelResponse.body.parcels[0], addItemResponse.body.items[0]); expect(removeItemResponse.status).to.be.equal(element.expected.status); - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); @@ -233,9 +219,9 @@ describe("Tracking Parcel", () => { ); expect(transferItemResponse.status).to.be.equal(element.expected.status); - next(); + return next(); } catch (e) { - next(e); + return next(e); } }); }); -- GitLab