From 48bc7595a8c715f7d228674173cae5a3684683e9 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Thu, 4 Jun 2020 09:49:46 +0200 Subject: [PATCH 1/2] fix formatting --- services/contacts/app.js | 4 +- services/contacts/app/js/ContactManager.js | 8 ++-- services/contacts/app/js/Errors.js | 2 +- services/contacts/app/js/HttpController.js | 10 ++-- .../test/acceptance/js/ContactsApp.js | 4 +- .../acceptance/js/GettingContactsTests.js | 26 +++++----- .../test/unit/js/ContactsManagerTests.js | 48 ++++++++++--------- .../test/unit/js/HttpControllerTests.js | 48 +++++++++---------- 8 files changed, 76 insertions(+), 74 deletions(-) diff --git a/services/contacts/app.js b/services/contacts/app.js index f99aff0661..0565974c0f 100644 --- a/services/contacts/app.js +++ b/services/contacts/app.js @@ -34,7 +34,7 @@ app.post( app.get('/status', (req, res) => res.send('contacts is alive')) -app.use(function(error, req, res, next) { +app.use(function (error, req, res, next) { logger.error({ err: error }, 'request errored') if (error instanceof Errors.NotFoundError) { return res.send(404) @@ -48,7 +48,7 @@ const { host } = Settings.internal.contacts if (!module.parent) { // Called directly - app.listen(port, host, function(error) { + app.listen(port, host, function (error) { if (error != null) { throw error } diff --git a/services/contacts/app/js/ContactManager.js b/services/contacts/app/js/ContactManager.js index 070373b4a4..2a874bc86b 100644 --- a/services/contacts/app/js/ContactManager.js +++ b/services/contacts/app/js/ContactManager.js @@ -18,7 +18,7 @@ const metrics = require('metrics-sharelatex') module.exports = ContactManager = { touchContact(user_id, contact_id, callback) { if (callback == null) { - callback = function(error) {} + callback = function (error) {} } try { user_id = ObjectId(user_id.toString()) @@ -45,7 +45,7 @@ module.exports = ContactManager = { getContacts(user_id, callback) { if (callback == null) { - callback = function(error) {} + callback = function (error) {} } try { user_id = ObjectId(user_id.toString()) @@ -58,7 +58,7 @@ module.exports = ContactManager = { { user_id }, - function(error, user) { + function (error, user) { if (error != null) { return callback(error) } @@ -67,7 +67,7 @@ module.exports = ContactManager = { ) } } -;['touchContact', 'getContacts'].map(method => +;['touchContact', 'getContacts'].map((method) => metrics.timeAsyncMethod( ContactManager, method, diff --git a/services/contacts/app/js/Errors.js b/services/contacts/app/js/Errors.js index f3bc8e37eb..a950b6de20 100644 --- a/services/contacts/app/js/Errors.js +++ b/services/contacts/app/js/Errors.js @@ -5,7 +5,7 @@ // TODO: This file was created by bulk-decaffeinate. // Fix any style issues and re-enable lint. let Errors -var NotFoundError = function(message) { +var NotFoundError = function (message) { const error = new Error(message) error.name = 'NotFoundError' error.__proto__ = NotFoundError.prototype diff --git a/services/contacts/app/js/HttpController.js b/services/contacts/app/js/HttpController.js index fae2ff0b4e..bca67671e0 100644 --- a/services/contacts/app/js/HttpController.js +++ b/services/contacts/app/js/HttpController.js @@ -25,11 +25,11 @@ module.exports = HttpController = { logger.log({ user_id, contact_id }, 'adding contact') - return ContactManager.touchContact(user_id, contact_id, function(error) { + return ContactManager.touchContact(user_id, contact_id, function (error) { if (error != null) { return next(error) } - return ContactManager.touchContact(contact_id, user_id, function(error) { + return ContactManager.touchContact(contact_id, user_id, function (error) { if (error != null) { return next(error) } @@ -52,7 +52,7 @@ module.exports = HttpController = { logger.log({ user_id }, 'getting contacts') - return ContactManager.getContacts(user_id, function(error, contact_dict) { + return ContactManager.getContacts(user_id, function (error, contact_dict) { if (error != null) { return next(error) } @@ -70,7 +70,7 @@ module.exports = HttpController = { HttpController._sortContacts(contacts) contacts = contacts.slice(0, limit) - const contact_ids = contacts.map(contact => contact.user_id) + const contact_ids = contacts.map((contact) => contact.user_id) return res.status(200).send({ contact_ids @@ -79,7 +79,7 @@ module.exports = HttpController = { }, _sortContacts(contacts) { - return contacts.sort(function(a, b) { + return contacts.sort(function (a, b) { // Sort by decreasing count, descreasing timestamp. // I.e. biggest count, and most recent at front. if (a.n > b.n) { diff --git a/services/contacts/test/acceptance/js/ContactsApp.js b/services/contacts/test/acceptance/js/ContactsApp.js index d192b0386c..ea1103a2c3 100644 --- a/services/contacts/test/acceptance/js/ContactsApp.js +++ b/services/contacts/test/acceptance/js/ContactsApp.js @@ -20,7 +20,7 @@ module.exports = { callbacks: [], ensureRunning(callback) { if (callback == null) { - callback = function(error) {} + callback = function (error) {} } if (this.running) { return callback() @@ -29,7 +29,7 @@ module.exports = { } else { this.initing = true this.callbacks.push(callback) - return app.listen(3036, 'localhost', error => { + return app.listen(3036, 'localhost', (error) => { if (error != null) { throw error } diff --git a/services/contacts/test/acceptance/js/GettingContactsTests.js b/services/contacts/test/acceptance/js/GettingContactsTests.js index f355ee11b7..266c19ffce 100644 --- a/services/contacts/test/acceptance/js/GettingContactsTests.js +++ b/services/contacts/test/acceptance/js/GettingContactsTests.js @@ -21,14 +21,14 @@ const async = require('async') const ContactsApp = require('./ContactsApp') const HOST = 'http://localhost:3036' -describe('Getting Contacts', function() { - describe('with no contacts', function() { - beforeEach(function(done) { +describe('Getting Contacts', function () { + describe('with no contacts', function () { + beforeEach(function (done) { this.user_id = ObjectId().toString() return ContactsApp.ensureRunning(done) }) - return it('should return an empty array', function(done) { + return it('should return an empty array', function (done) { return request( { method: 'GET', @@ -44,8 +44,8 @@ describe('Getting Contacts', function() { }) }) - return describe('with contacts', function() { - beforeEach(function(done) { + return describe('with contacts', function () { + beforeEach(function (done) { this.user_id = ObjectId().toString() this.contact_id_1 = ObjectId().toString() this.contact_id_2 = ObjectId().toString() @@ -66,17 +66,17 @@ describe('Getting Contacts', function() { return async.series( [ // 2 is preferred since touched twice, then 3 since most recent, then 1 - cb => ContactsApp.ensureRunning(cb), - cb => touchContact(this.user_id, this.contact_id_1, cb), - cb => touchContact(this.user_id, this.contact_id_2, cb), - cb => touchContact(this.user_id, this.contact_id_2, cb), - cb => touchContact(this.user_id, this.contact_id_3, cb) + (cb) => ContactsApp.ensureRunning(cb), + (cb) => touchContact(this.user_id, this.contact_id_1, cb), + (cb) => touchContact(this.user_id, this.contact_id_2, cb), + (cb) => touchContact(this.user_id, this.contact_id_2, cb), + (cb) => touchContact(this.user_id, this.contact_id_3, cb) ], done ) }) - it('should return a sorted list of contacts', function(done) { + it('should return a sorted list of contacts', function (done) { return request( { method: 'GET', @@ -95,7 +95,7 @@ describe('Getting Contacts', function() { ) }) - return it('should respect a limit and only return top X contacts', function(done) { + return it('should respect a limit and only return top X contacts', function (done) { return request( { method: 'GET', diff --git a/services/contacts/test/unit/js/ContactsManagerTests.js b/services/contacts/test/unit/js/ContactsManagerTests.js index e775cb04fc..f2a1884a47 100644 --- a/services/contacts/test/unit/js/ContactsManagerTests.js +++ b/services/contacts/test/unit/js/ContactsManagerTests.js @@ -18,8 +18,8 @@ const SandboxedModule = require('sandboxed-module') const { ObjectId } = require('mongojs') const tk = require('timekeeper') -describe('ContactManager', function() { - beforeEach(function() { +describe('ContactManager', function () { + beforeEach(function () { tk.freeze(Date.now()) this.ContactManager = SandboxedModule.require(modulePath, { requires: { @@ -36,17 +36,17 @@ describe('ContactManager', function() { return (this.callback = sinon.stub()) }) - afterEach(function() { + afterEach(function () { return tk.reset() }) - describe('touchContact', function() { - beforeEach(function() { + describe('touchContact', function () { + beforeEach(function () { return (this.db.contacts.update = sinon.stub().callsArg(3)) }) - describe('with a valid user_id', function() { - beforeEach(function() { + describe('with a valid user_id', function () { + beforeEach(function () { return this.ContactManager.touchContact( this.user_id, (this.contact_id = 'mock_contact'), @@ -54,12 +54,12 @@ describe('ContactManager', function() { ) }) - it('should increment the contact count and timestamp', function() { + it('should increment the contact count and timestamp', function () { return this.db.contacts.update .calledWith( { user_id: sinon.match( - o => o.toString() === this.user_id.toString() + (o) => o.toString() === this.user_id.toString() ) }, { @@ -77,13 +77,13 @@ describe('ContactManager', function() { .should.equal(true) }) - return it('should call the callback', function() { + return it('should call the callback', function () { return this.callback.called.should.equal(true) }) }) - return describe('with an invalid user id', function() { - beforeEach(function() { + return describe('with an invalid user id', function () { + beforeEach(function () { return this.ContactManager.touchContact( 'not-valid-object-id', this.contact_id, @@ -91,14 +91,14 @@ describe('ContactManager', function() { ) }) - return it('should call the callback with an error', function() { + return it('should call the callback with an error', function () { return this.callback.calledWith(sinon.match(Error)).should.equal(true) }) }) }) - return describe('getContacts', function() { - beforeEach(function() { + return describe('getContacts', function () { + beforeEach(function () { this.user = { contacts: ['mock', 'contacts'] } @@ -107,35 +107,37 @@ describe('ContactManager', function() { .callsArgWith(1, null, this.user)) }) - describe('with a valid user_id', function() { - beforeEach(function() { + describe('with a valid user_id', function () { + beforeEach(function () { return this.ContactManager.getContacts(this.user_id, this.callback) }) - it("should find the user's contacts", function() { + it("should find the user's contacts", function () { return this.db.contacts.findOne .calledWith({ - user_id: sinon.match(o => o.toString() === this.user_id.toString()) + user_id: sinon.match( + (o) => o.toString() === this.user_id.toString() + ) }) .should.equal(true) }) - return it('should call the callback with the contacts', function() { + return it('should call the callback with the contacts', function () { return this.callback .calledWith(null, this.user.contacts) .should.equal(true) }) }) - return describe('with an invalid user id', function() { - beforeEach(function() { + return describe('with an invalid user id', function () { + beforeEach(function () { return this.ContactManager.getContacts( 'not-valid-object-id', this.callback ) }) - return it('should call the callback with an error', function() { + return it('should call the callback with an error', function () { return this.callback.calledWith(sinon.match(Error)).should.equal(true) }) }) diff --git a/services/contacts/test/unit/js/HttpControllerTests.js b/services/contacts/test/unit/js/HttpControllerTests.js index e3d183eebd..4c2e4a7695 100644 --- a/services/contacts/test/unit/js/HttpControllerTests.js +++ b/services/contacts/test/unit/js/HttpControllerTests.js @@ -17,8 +17,8 @@ const { expect } = chai const modulePath = '../../../app/js/HttpController.js' const SandboxedModule = require('sandboxed-module') -describe('HttpController', function() { - beforeEach(function() { +describe('HttpController', function () { + beforeEach(function () { this.HttpController = SandboxedModule.require(modulePath, { requires: { './ContactManager': (this.ContactManager = {}), @@ -36,43 +36,43 @@ describe('HttpController', function() { return (this.next = sinon.stub()) }) - describe('addContact', function() { - beforeEach(function() { + describe('addContact', function () { + beforeEach(function () { this.req.params = { user_id: this.user_id } return (this.ContactManager.touchContact = sinon.stub().callsArg(2)) }) - describe('with a valid user_id and contact_id', function() { - beforeEach(function() { + describe('with a valid user_id and contact_id', function () { + beforeEach(function () { this.req.body = { contact_id: this.contact_id } return this.HttpController.addContact(this.req, this.res, this.next) }) - it("should update the contact in the user's contact list", function() { + it("should update the contact in the user's contact list", function () { return this.ContactManager.touchContact .calledWith(this.user_id, this.contact_id) .should.equal(true) }) - it("should update the user in the contact's contact list", function() { + it("should update the user in the contact's contact list", function () { return this.ContactManager.touchContact .calledWith(this.contact_id, this.user_id) .should.equal(true) }) - return it('should send back a 204 status', function() { + return it('should send back a 204 status', function () { this.res.status.calledWith(204).should.equal(true) return this.res.end.called.should.equal(true) }) }) - return describe('with an invalid contact id', function() { - beforeEach(function() { + return describe('with an invalid contact id', function () { + beforeEach(function () { this.req.body = { contact_id: '' } return this.HttpController.addContact(this.req, this.res, this.next) }) - return it('should return 400, Bad Request', function() { + return it('should return 400, Bad Request', function () { this.res.status.calledWith(400).should.equal(true) return this.res.send .calledWith('contact_id should be a non-blank string') @@ -81,8 +81,8 @@ describe('HttpController', function() { }) }) - return describe('getContacts', function() { - beforeEach(function() { + return describe('getContacts', function () { + beforeEach(function () { this.req.params = { user_id: this.user_id } const now = Date.now() this.contacts = { @@ -95,18 +95,18 @@ describe('HttpController', function() { .callsArgWith(1, null, this.contacts)) }) - describe('normally', function() { - beforeEach(function() { + describe('normally', function () { + beforeEach(function () { return this.HttpController.getContacts(this.req, this.res, this.next) }) - it('should look up the contacts in mongo', function() { + it('should look up the contacts in mongo', function () { return this.ContactManager.getContacts .calledWith(this.user_id) .should.equal(true) }) - return it('should return a sorted list of contacts by count and timestamp', function() { + return it('should return a sorted list of contacts by count and timestamp', function () { return this.res.send .calledWith({ contact_ids: ['user-id-2', 'user-id-1', 'user-id-3'] @@ -115,13 +115,13 @@ describe('HttpController', function() { }) }) - describe('with more contacts than the limit', function() { - beforeEach(function() { + describe('with more contacts than the limit', function () { + beforeEach(function () { this.req.query = { limit: 2 } return this.HttpController.getContacts(this.req, this.res, this.next) }) - return it('should return the most commonly used contacts up to the limit', function() { + return it('should return the most commonly used contacts up to the limit', function () { return this.res.send .calledWith({ contact_ids: ['user-id-2', 'user-id-1'] @@ -130,15 +130,15 @@ describe('HttpController', function() { }) }) - describe('without a contact list', function() { - beforeEach(function() { + describe('without a contact list', function () { + beforeEach(function () { this.ContactManager.getContacts = sinon .stub() .callsArgWith(1, null, null) return this.HttpController.getContacts(this.req, this.res, this.next) }) - return it('should return an empty list', function() { + return it('should return an empty list', function () { return this.res.send .calledWith({ contact_ids: [] From 85d8b04a6ca3ebe906f6858e4704f4023ee1025f Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Thu, 4 Jun 2020 10:19:52 +0200 Subject: [PATCH 2/2] Revert "[misc] make: ignore a lint/format task failure" This reverts commit 6daa8ff2467557f710adcddbf7277b1ce135991d. --- services/contacts/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/contacts/Makefile b/services/contacts/Makefile index 528bf7b0bd..7e0b5418ea 100644 --- a/services/contacts/Makefile +++ b/services/contacts/Makefile @@ -17,13 +17,13 @@ clean: docker rmi gcr.io/overleaf-ops/$(PROJECT_NAME):$(BRANCH_NAME)-$(BUILD_NUMBER) format: - $(DOCKER_COMPOSE) run --rm test_unit npm run format || true + $(DOCKER_COMPOSE) run --rm test_unit npm run format format_fix: $(DOCKER_COMPOSE) run --rm test_unit npm run format:fix lint: - $(DOCKER_COMPOSE) run --rm test_unit npm run lint || true + $(DOCKER_COMPOSE) run --rm test_unit npm run lint test: format lint test_unit test_acceptance