Merge pull request #3390 from overleaf/jpa-faster-unit-tests

[perf] faster unit tests

GitOrigin-RevId: 188b8f3752638fde7a27a8d83b416bb9a6e3c95e

services/web/Makefile

@@ -41,7 +41,14 @@ test_module: test_unit_module test_acceptance_module
 # Unit tests
 #
 
-test_unit: test_unit_app test_unit_modules
+test_unit: test_unit_all
+test_unit_all:
+	COMPOSE_PROJECT_NAME=unit_test_all_$(BUILD_DIR_NAME) $(DOCKER_COMPOSE) run --rm test_unit npm run test:unit:all
+	COMPOSE_PROJECT_NAME=unit_test_all_$(BUILD_DIR_NAME) $(DOCKER_COMPOSE) down -v -t 0
+
+test_unit_all_silent:
+	COMPOSE_PROJECT_NAME=unit_test_all_$(BUILD_DIR_NAME) $(DOCKER_COMPOSE) run --rm test_unit npm run test:unit:all:silent
+	COMPOSE_PROJECT_NAME=unit_test_all_$(BUILD_DIR_NAME) $(DOCKER_COMPOSE) down -v -t 0
 
 test_unit_app:
 	COMPOSE_PROJECT_NAME=unit_test_$(BUILD_DIR_NAME) $(DOCKER_COMPOSE) down -v -t 0

services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js

@@ -23,11 +23,9 @@ const logger = require('logger-sharelatex')
 const Settings = require('settings-sharelatex')
 const EmailHelper = require('../Helpers/EmailHelper')
 const EditorRealTimeController = require('../Editor/EditorRealTimeController')
-const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const AuthenticationController = require('../Authentication/AuthenticationController')
 const rateLimiter = require('../../infrastructure/RateLimiter')
-const request = require('request')
 
 module.exports = CollaboratorsInviteController = {
   getAllInvites(req, res, next) {

services/web/app/src/Features/Collaborators/CollaboratorsInviteHandler.js

@@ -18,8 +18,6 @@ const CollaboratorsEmailHandler = require('./CollaboratorsEmailHandler')
 const CollaboratorsHandler = require('./CollaboratorsHandler')
 const UserGetter = require('../User/UserGetter')
 const ProjectGetter = require('../Project/ProjectGetter')
-const Async = require('async')
-const PrivilegeLevels = require('../Authorization/PrivilegeLevels')
 const Errors = require('../Errors/Errors')
 const Crypto = require('crypto')
 const NotificationsBuilder = require('../Notifications/NotificationsBuilder')

services/web/app/src/Features/Email/EmailBuilder.js

@@ -3,10 +3,10 @@ const settings = require('settings-sharelatex')
 const moment = require('moment')
 const EmailMessageHelper = require('./EmailMessageHelper')
 const StringHelper = require('../Helpers/StringHelper')
-const BaseWithHeaderEmailLayout = require(`./Layouts/BaseWithHeaderEmailLayout`)
+const BaseWithHeaderEmailLayout = require('./Layouts/BaseWithHeaderEmailLayout')
 const SpamSafe = require('./SpamSafe')
 const ctaEmailBody = require('./Bodies/cta-email')
-const NoCTAEmailBody = require(`./Bodies/NoCTAEmailBody`)
+const NoCTAEmailBody = require('./Bodies/NoCTAEmailBody')
 
 function _emailBodyPlainText(content, opts, ctaEmail) {
   let emailBody = `${content.greeting(opts, true)}`

services/web/app/src/Features/History/RestoreManager.js

@@ -18,7 +18,6 @@ const Path = require('path')
 const FileWriter = require('../../infrastructure/FileWriter')
 const FileSystemImportManager = require('../Uploads/FileSystemImportManager')
 const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
-const ProjectLocator = require('../Project/ProjectLocator')
 const EditorController = require('../Editor/EditorController')
 const Errors = require('../Errors/Errors')
 const moment = require('moment')

services/web/package-lock.json

@@ -31749,9 +31749,9 @@
       }
     },
     "sandboxed-module": {
-      "version": "0.2.0",
-      "resolved": "https://registry.npmjs.org/sandboxed-module/-/sandboxed-module-0.2.0.tgz",
-      "integrity": "sha512-szXUyf1v4ShPhq+/MoAeEoIldS/4Bfl8uBGVz+fgTm1fbnub7QX1sOmzhDdUAqAnkdpwnwgsR5b5nKnNggbd9w==",
+      "version": "0.3.0",
+      "resolved": "https://registry.npmjs.org/sandboxed-module/-/sandboxed-module-0.3.0.tgz",
+      "integrity": "sha1-8fvvvYCaT2kHO9B8rm/H2y6vX2o=",
       "dev": true,
       "requires": {
         "require-like": "0.1.2",
@@ -31761,7 +31761,7 @@
         "stack-trace": {
           "version": "0.0.6",
           "resolved": "https://registry.npmjs.org/stack-trace/-/stack-trace-0.0.6.tgz",
-          "integrity": "sha512-5/6uZt7RYjjAl8z2j1mXWAewz+I4Hk2/L/3n6NRLIQ31+uQ7nMd9O6G69QCdrrufHv0QGRRHl/jwUEGTqhelTA==",
+          "integrity": "sha1-HnGb1qJin/CcGJ4Xqe+QKpT8XbA=",
           "dev": true
         }
       }

services/web/package.json

@@ -15,6 +15,8 @@
     "test:acceptance:run_dir": "mocha --recursive --timeout 25000 --exit --grep=$MOCHA_GREP --require test/acceptance/bootstrap.js",
     "test:acceptance:app": "npm run test:acceptance:run_dir -- test/acceptance/src",
     "test:unit:run_dir": "mocha --recursive --timeout 25000 --exit --grep=$MOCHA_GREP --file test/unit/bootstrap.js",
+    "test:unit:all": "npm run test:unit:run_dir -- test/unit/src modules/*/test/unit/src",
+    "test:unit:all:silent": "npm run test:unit:all -- --reporter dot",
     "test:unit:app": "npm run test:unit:run_dir -- test/unit/src",
     "test:unit:app:parallel": "parallel --plain --keep-order --halt now,fail=1 npm run test:unit:run_dir -- {} ::: test/unit/src/*",
     "test:frontend": "NODE_ENV=test TZ=GMT mocha --recursive --timeout 5000 --exit --grep=$MOCHA_GREP --require test/frontend/bootstrap.js test/frontend modules/*/test/frontend",
@@ -215,7 +217,7 @@
     "prettier-eslint-cli": "^4.7.1",
     "requirejs": "^2.3.6",
     "samlp": "^3.4.1",
-    "sandboxed-module": "0.2.0",
+    "sandboxed-module": "0.3.0",
     "sinon": "^7.5.0",
     "sinon-chai": "^3.5.0",
     "sinon-mongoose": "^2.3.0",

services/web/test/unit/bootstrap.js

@@ -20,3 +20,35 @@ require('sinon-mongoose')
 afterEach(function() {
   sinon.restore()
 })
+
+const SandboxedModule = require('sandboxed-module')
+const PromisesUtils = require('../../app/src/util/promises')
+const Errors = require('../../app/src/Features/Errors/Errors')
+const GLOBAL_REQUIRE_CACHE_FOR_SANDBOXED_MODULES = {
+  // cache p-limit for all expressify/promisifyAll users
+  '../../util/promises': PromisesUtils,
+  '../../../../app/src/util/promises': PromisesUtils,
+
+  // Errors are widely used and instance checks need the exact same prototypes
+  '../Errors/Errors': Errors,
+  '../../../../app/src/Features/Errors/Errors': Errors,
+  '../../../../../app/src/Features/Errors/Errors': Errors
+}
+const LIBRARIES = [
+  '@overleaf/o-error',
+  'async',
+  'lodash',
+  'moment',
+  'underscore',
+  'xml2js',
+  'json2csv',
+  'sanitize-html',
+  'marked'
+]
+LIBRARIES.forEach(lib => {
+  GLOBAL_REQUIRE_CACHE_FOR_SANDBOXED_MODULES[lib] = require(lib)
+})
+
+SandboxedModule.configure({
+  requires: GLOBAL_REQUIRE_CACHE_FOR_SANDBOXED_MODULES
+})

services/web/test/unit/src/Authentication/AuthenticationManagerTests.js

@@ -11,7 +11,7 @@ const modulePath =
 
 describe('AuthenticationManager', function() {
   beforeEach(function() {
-    this.settings = { security: { bcryptRounds: 12 } }
+    this.settings = { security: { bcryptRounds: 4 } }
     this.AuthenticationManager = SandboxedModule.require(modulePath, {
       globals: {
         console: console
@@ -42,7 +42,7 @@ describe('AuthenticationManager', function() {
       this.bcrypt.hash = bcrypt.hash
       // Hash of 'testpassword'
       this.testPassword =
-        '$2a$12$zhtThy3R5tLtw5sCwr5XD.zhPENGn4ecjeMcP87oYSYrIICFqBpei'
+        '$2a$04$DcU/3UeJf1PfsWlQL./5H.rGTQL1Z1iyz6r7bN9Do8cy6pVWxpKpK'
     })
 
     describe('authenticate', function() {
@@ -128,7 +128,7 @@ describe('AuthenticationManager', function() {
             } = this.db.users.updateOne.lastCall.args[1].$set
             expect(hashedPassword).to.exist
             expect(hashedPassword.length).to.equal(60)
-            expect(hashedPassword).to.match(/^\$2a\$12\$[a-zA-Z0-9/.]{53}$/)
+            expect(hashedPassword).to.match(/^\$2a\$04\$[a-zA-Z0-9/.]{53}$/)
             done()
           }
         )
@@ -151,7 +151,7 @@ describe('AuthenticationManager', function() {
         beforeEach(function(done) {
           this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf'
           this.bcrypt.compare = sinon.stub().callsArgWith(2, null, true)
-          this.bcrypt.getRounds = sinon.stub().returns(12)
+          this.bcrypt.getRounds = sinon.stub().returns(4)
           this.AuthenticationManager.authenticate(
             { email: this.email },
             this.unencryptedPassword,
@@ -195,7 +195,7 @@ describe('AuthenticationManager', function() {
         beforeEach(function(done) {
           this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf'
           this.bcrypt.compare = sinon.stub().callsArgWith(2, null, true)
-          this.bcrypt.getRounds = sinon.stub().returns(7)
+          this.bcrypt.getRounds = sinon.stub().returns(1)
           this.AuthenticationManager.setUserPassword = sinon
             .stub()
             .callsArgWith(2, null)
@@ -239,7 +239,7 @@ describe('AuthenticationManager', function() {
           this.settings.security.disableBcryptRoundsUpgrades = true
           this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf'
           this.bcrypt.compare = sinon.stub().callsArgWith(2, null, true)
-          this.bcrypt.getRounds = sinon.stub().returns(7)
+          this.bcrypt.getRounds = sinon.stub().returns(1)
           this.AuthenticationManager.setUserPassword = sinon
             .stub()
             .callsArgWith(2, null)
@@ -670,7 +670,7 @@ describe('AuthenticationManager', function() {
       })
 
       it('should hash the password', function() {
-        this.bcrypt.genSalt.calledWith(12).should.equal(true)
+        this.bcrypt.genSalt.calledWith(4).should.equal(true)
         this.bcrypt.hash.calledWith(this.password, this.salt).should.equal(true)
       })
 

services/web/test/unit/src/Authorization/AuthorizationManagerTests.js

@@ -35,7 +35,6 @@ describe('AuthorizationManager', function() {
         '../../models/User': {
           User: (this.User = {})
         },
-        '../Errors/Errors': Errors,
         '../TokenAccess/TokenAccessHandler': (this.TokenAccessHandler = {
           validateTokenForAnonymousAccess: sinon
             .stub()

services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js

@@ -40,7 +40,6 @@ describe('AuthorizationMiddleware', function() {
           ObjectId: this.ObjectId
         },
         '../Errors/HttpErrorHandler': this.HttpErrorHandler,
-        '../Errors/Errors': Errors,
         '../Authentication/AuthenticationController': this
           .AuthenticationController,
         '../TokenAccess/TokenAccessHandler': this.TokenAccessHandler

services/web/test/unit/src/Collaborators/CollaboratorsControllerTests.js

@@ -70,7 +70,6 @@ describe('CollaboratorsController', function() {
         '../Tags/TagsHandler': this.TagsHandler,
         '../Authentication/AuthenticationController': this
           .AuthenticationController,
-        '../Errors/Errors': Errors,
         'logger-sharelatex': this.logger
       }
     })

services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js

@@ -55,7 +55,6 @@ describe('CollaboratorsGetter', function() {
         '../User/UserGetter': this.UserGetter,
         '../../models/Project': { Project },
         '../Project/ProjectGetter': this.ProjectGetter,
-        '../Errors/Errors': Errors,
         '../Project/ProjectEditorHandler': this.ProjectEditorHandler
       }
     })

services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js

@@ -77,7 +77,6 @@ describe('CollaboratorsHandler', function() {
         '../ThirdPartyDataStore/TpdsProjectFlusher': this.TpdsProjectFlusher,
         '../Project/ProjectGetter': this.ProjectGetter,
         '../Project/ProjectHelper': this.ProjectHelper,
-        '../Errors/Errors': Errors,
         './CollaboratorsGetter': this.CollaboratorsGetter
       }
     })
@@ -445,7 +444,7 @@ describe('CollaboratorsHandler', function() {
           this.fromUserId,
           this.toUserId
         )
-        await sleep(100) // let the background tasks run
+        await sleep(10) // let the background tasks run
         for (const project of this.projects) {
           expect(
             this.TpdsProjectFlusher.promises.flushProjectToTpds
@@ -463,7 +462,7 @@ describe('CollaboratorsHandler', function() {
           this.fromUserId,
           this.toUserId
         )
-        await sleep(100) // let the background tasks run
+        await sleep(10) // let the background tasks run
         expect(this.logger.err).to.have.been.called
       })
     })

services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js

@@ -62,7 +62,6 @@ describe('CollaboratorsInviteController', function() {
         '../Editor/EditorRealTimeController': (this.EditorRealTimeController = {
           emitToRoom: sinon.stub()
         }),
-        '../Notifications/NotificationsBuilder': (this.NotificationsBuilder = {}),
         '../Analytics/AnalyticsManager': this.AnalyticsManger,
         '../Authentication/AuthenticationController': this
           .AuthenticationController,

services/web/test/unit/src/Collaborators/OwnershipTransferHandlerTests.js

@@ -77,8 +77,7 @@ describe('OwnershipTransferHandler', function() {
           log() {},
           warn() {},
           err() {}
-        },
-        '../Errors/Errors': Errors
+        }
       }
     })
   })

services/web/test/unit/src/Compile/ClsiFormatCheckerTests.js

@@ -198,7 +198,7 @@ describe('ClsiFormatChecker', function() {
       it('should error when there is more than 5mb of data', function(done) {
         this.resources.push({
           path: 'massive.tex',
-          content: 'hello world\n'.repeat(833333) // over 5mb limit
+          content: 'hello world'.repeat(833333) // over 5mb limit
         })
 
         while (this.resources.length < 20) {
@@ -223,9 +223,7 @@ describe('ClsiFormatChecker', function() {
       it('should return nothing when project is correct size', function(done) {
         this.resources.push({
           path: 'massive.tex',
-          content: require('crypto')
-            .randomBytes(1000 * 1000 * 1)
-            .toString('hex')
+          content: 'x'.repeat(2 * 1000 * 1000)
         })
 
         while (this.resources.length < 20) {

services/web/test/unit/src/Docstore/DocstoreManagerTests.js

@@ -39,8 +39,7 @@ describe('DocstoreManager', function() {
           warn: sinon.stub(),
           error: sinon.stub(),
           err() {}
-        }),
-        '../Errors/Errors': Errors
+        })
       }
     })
 

services/web/test/unit/src/Documents/DocumentControllerTests.js

@@ -37,8 +37,7 @@ describe('DocumentController', function() {
         '../Project/ProjectGetter': (this.ProjectGetter = {}),
         '../Project/ProjectLocator': (this.ProjectLocator = {}),
         '../Project/ProjectEntityHandler': (this.ProjectEntityHandler = {}),
-        '../Project/ProjectEntityUpdateHandler': (this.ProjectEntityUpdateHandler = {}),
-        '../Errors/Errors': Errors
+        '../Project/ProjectEntityUpdateHandler': (this.ProjectEntityUpdateHandler = {})
       }
     })
     this.res = new MockResponse()

services/web/test/unit/src/Editor/EditorHttpControllerTests.js

@@ -149,8 +149,7 @@ describe('EditorHttpController', function() {
         '../../infrastructure/FileWriter': this.FileWriter,
         '../Project/ProjectEntityUpdateHandler': this
           .ProjectEntityUpdateHandler,
-        '../Errors/HttpErrorHandler': this.HttpErrorHandler,
-        '../Errors/Errors': Errors
+        '../Errors/HttpErrorHandler': this.HttpErrorHandler
       }
     })
   })

services/web/test/unit/src/Email/EmailBuilderTests.js

@@ -2,19 +2,21 @@ const SandboxedModule = require('sandboxed-module')
 const cheerio = require('cheerio')
 const path = require('path')
 const { expect } = require('chai')
-const _ = require('underscore')
-_.templateSettings = { interpolate: /\{\{(.+?)\}\}/g }
 
 const MODULE_PATH = path.join(
   __dirname,
   '../../../../app/src/Features/Email/EmailBuilder'
 )
 
+const EmailMessageHelper = require('../../../../app/src/Features/Email/EmailMessageHelper')
+const ctaEmailBody = require('../../../../app/src/Features/Email/Bodies/cta-email')
+const NoCTAEmailBody = require('../../../../app/src/Features/Email/Bodies/NoCTAEmailBody')
+const BaseWithHeaderEmailLayout = require('../../../../app/src/Features/Email/Layouts/BaseWithHeaderEmailLayout')
+
 describe('EmailBuilder', function() {
-  beforeEach(function() {
+  before(function() {
     this.settings = {
       appName: 'testApp',
-      brandPrefix: '',
       siteUrl: 'https://www.overleaf.com'
     }
     this.EmailBuilder = SandboxedModule.require(MODULE_PATH, {
@@ -22,6 +24,10 @@ describe('EmailBuilder', function() {
         console: console
       },
       requires: {
+        './EmailMessageHelper': EmailMessageHelper,
+        './Bodies/cta-email': ctaEmailBody,
+        './Bodies/NoCTAEmailBody': NoCTAEmailBody,
+        './Layouts/BaseWithHeaderEmailLayout': BaseWithHeaderEmailLayout,
         'settings-sharelatex': this.settings,
         'logger-sharelatex': {
           log() {}

services/web/test/unit/src/Email/EmailSenderTests.js

@@ -38,6 +38,7 @@ describe('EmailSender', function() {
       },
       requires: {
         nodemailer: this.ses,
+        'nodemailer-ses-transport': sinon.stub(),
         'nodemailer-mandrill-transport': {},
         'settings-sharelatex': this.Settings,
         '../../infrastructure/RateLimiter': this.RateLimiter,

services/web/test/unit/src/Errors/HttpErrorHandlerTests.js

@@ -18,6 +18,10 @@ describe('HttpErrorHandler', function() {
     this.HttpErrorHandler = SandboxedModule.require(modulePath, {
       globals: { console },
       requires: {
+        'settings-sharelatex': {
+          appName: 'Overleaf',
+          statusPageUrl: 'https://status.overlaf.com'
+        },
         'logger-sharelatex': this.logger
       }
     })

services/web/test/unit/src/FileStore/FileStoreControllerTests.js

@@ -1,6 +1,7 @@
 const { expect } = require('chai')
 const sinon = require('sinon')
 const SandboxedModule = require('sandboxed-module')
+const Errors = require('../../../../app/src/Features/Errors/Errors')
 
 const MODULE_PATH =
   '../../../../app/src/Features/FileStore/FileStoreController.js'
@@ -12,7 +13,6 @@ describe('FileStoreController', function() {
       getFileSize: sinon.stub()
     }
     this.ProjectLocator = { findElement: sinon.stub() }
-    this.Errors = { NotFoundError: sinon.stub() }
     this.controller = SandboxedModule.require(MODULE_PATH, {
       globals: {
         console: console
@@ -24,7 +24,6 @@ describe('FileStoreController', function() {
           err: sinon.stub()
         }),
         '../Project/ProjectLocator': this.ProjectLocator,
-        '../Errors/Errors': this.Errors,
         './FileStoreHandler': this.FileStoreHandler
       }
     })
@@ -227,7 +226,7 @@ describe('FileStoreController', function() {
     })
 
     it('returns 404 on NotFoundError', function(done) {
-      this.FileStoreHandler.getFileSize.yields(new this.Errors.NotFoundError())
+      this.FileStoreHandler.getFileSize.yields(new Errors.NotFoundError())
 
       this.res.end = () => {
         expect(this.res.status.lastCall.args).to.deep.equal([404])

services/web/test/unit/src/FileStore/FileStoreHandlerTests.js

@@ -76,7 +76,6 @@ describe('FileStoreHandler', function() {
         '../../models/File': {
           File: this.FileModel
         },
-        '../Errors/Errors': Errors,
         fs: this.fs
       }
     })

services/web/test/unit/src/HelperFiles/UrlHelperTests.js

@@ -8,7 +8,13 @@ const modulePath = require('path').join(
 
 describe('UrlHelper', function() {
   beforeEach(function() {
-    this.UrlHelper = SandboxedModule.require(modulePath, {})
+    this.settings = {
+      apis: { linkedUrlProxy: { url: undefined } },
+      siteUrl: 'http://localhost:3000'
+    }
+    this.UrlHelper = SandboxedModule.require(modulePath, {
+      requires: { 'settings-sharelatex': this.settings }
+    })
   })
   describe('getSafeRedirectPath', function() {
     it('sanitize redirect path to prevent open redirects', function() {

services/web/test/unit/src/History/HistoryControllerTests.js

@@ -39,7 +39,6 @@ describe('HistoryController', function() {
         }),
         '../Authentication/AuthenticationController': this
           .AuthenticationController,
-        '../Errors/Errors': Errors,
         './HistoryManager': (this.HistoryManager = {}),
         '../Project/ProjectDetailsHandler': (this.ProjectDetailsHandler = {}),
         '../Project/ProjectEntityUpdateHandler': (this.ProjectEntityUpdateHandler = {}),

services/web/test/unit/src/History/RestoreManagerTests.js

@@ -31,10 +31,9 @@ describe('RestoreManager', function() {
         console: console
       },
       requires: {
+        'settings-sharelatex': {},
         '../../infrastructure/FileWriter': (this.FileWriter = {}),
         '../Uploads/FileSystemImportManager': (this.FileSystemImportManager = {}),
-        '../Project/ProjectLocator': (this.ProjectLocator = {}),
-        '../Errors/Errors': Errors,
         '../Project/ProjectEntityHandler': (this.ProjectEntityHandler = {}),
         '../Editor/EditorController': (this.EditorController = {}),
         'logger-sharelatex': (this.logger = {

services/web/test/unit/src/InactiveData/InactiveProjectManagerTests.js

@@ -21,6 +21,7 @@ const modulePath = path.join(
   '../../../../app/src/Features/InactiveData/InactiveProjectManager'
 )
 const { expect } = require('chai')
+const { ObjectId } = require('mongodb')
 
 describe('InactiveProjectManager', function() {
   beforeEach(function() {
@@ -40,6 +41,7 @@ describe('InactiveProjectManager', function() {
         console: console
       },
       requires: {
+        mongodb: { ObjectId },
         'settings-sharelatex': this.settings,
         'logger-sharelatex': {
           log() {},

services/web/test/unit/src/Institutions/InstitutionsAPITests.js

@@ -47,8 +47,7 @@ describe('InstitutionsAPI', function() {
         },
         '../../../../../app/src/Features/V1/V1Api': {
           request: sinon.stub()
-        },
-        '../Errors/Errors': Errors
+        }
       }
     })
 

services/web/test/unit/src/Newsletter/NewsletterManagerTests.js

@@ -27,6 +27,7 @@ describe('NewsletterManager', function() {
 
     this.NewsletterManager = SandboxedModule.require(MODULE_PATH, {
       requires: {
+        'logger-sharelatex': { info: sinon.stub() },
         'mailchimp-api-v3': this.Mailchimp,
         'settings-sharelatex': this.Settings
       },

services/web/test/unit/src/Notifications/NotificationsControllerTests.js

@@ -51,11 +51,6 @@ describe('NotificationsController', function() {
       },
       requires: {
         './NotificationsHandler': this.handler,
-        underscore: (this.underscore = {
-          map(arr) {
-            return arr
-          }
-        }),
         'logger-sharelatex': {
           log() {},
           err() {}
@@ -73,7 +68,7 @@ describe('NotificationsController', function() {
       .callsArgWith(1, null, allNotifications)
     return this.controller.getAllUnreadNotifications(this.req, {
       send: body => {
-        body.should.equal(allNotifications)
+        body.should.deep.equal(allNotifications)
         this.handler.getUserNotifications.calledWith(user_id).should.equal(true)
         return done()
       }

services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -167,7 +167,6 @@ describe('ProjectController', function() {
         '../TokenAccess/TokenAccessHandler': this.TokenAccessHandler,
         '../Collaborators/CollaboratorsGetter': this.CollaboratorsGetter,
         './ProjectEntityHandler': this.ProjectEntityHandler,
-        '../Errors/Errors': Errors,
         '../../infrastructure/Features': this.Features,
         '../Notifications/NotificationsBuilder': this.NotificationBuilder,
         '../User/UserGetter': this.UserGetter,

services/web/test/unit/src/Project/ProjectDeleterTests.js

@@ -146,8 +146,7 @@ describe('ProjectDeleter', function() {
         './ProjectDetailsHandler': this.ProjectDetailsHandler,
         '../../infrastructure/mongodb': { db: this.db, ObjectId },
         '../History/HistoryManager': this.HistoryManager,
-        'logger-sharelatex': this.logger,
-        '../Errors/Errors': Errors
+        'logger-sharelatex': this.logger
       },
       globals: {
         console: console

services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js

@@ -3,6 +3,7 @@ const sinon = require('sinon')
 const { expect } = require('chai')
 const { ObjectId } = require('mongodb')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
+const ProjectHelper = require('../../../../app/src/Features/Project/ProjectHelper')
 
 const MODULE_PATH = '../../../../app/src/Features/Project/ProjectDetailsHandler'
 
@@ -69,6 +70,7 @@ describe('ProjectDetailsHandler', function() {
         console: console
       },
       requires: {
+        './ProjectHelper': ProjectHelper,
         './ProjectGetter': this.ProjectGetter,
         '../../models/Project': {
           Project: this.ProjectModel
@@ -81,7 +83,6 @@ describe('ProjectDetailsHandler', function() {
           err() {}
         },
         './ProjectTokenGenerator': this.ProjectTokenGenerator,
-        '../Errors/Errors': Errors,
         'settings-sharelatex': this.settings
       }
     })
@@ -227,6 +228,7 @@ describe('ProjectDetailsHandler', function() {
   })
 
   describe('generateUniqueName', function() {
+    // actually testing `ProjectHelper.promises.ensureNameIsUnique()`
     beforeEach(function() {
       this.longName = 'x'.repeat(this.handler.MAX_PROJECT_NAME_LENGTH - 5)
       const usersProjects = {

services/web/test/unit/src/Project/ProjectEntityHandlerTests.js

@@ -74,7 +74,6 @@ describe('ProjectEntityHandler', function() {
         '../../models/Project': {
           Project: this.ProjectModel
         },
-        '../Errors/Errors': Errors,
         './ProjectLocator': this.ProjectLocator,
         './ProjectGetter': (this.ProjectGetter = {}),
         '../ThirdPartyDataStore/TpdsUpdateSender': this.TpdsUpdateSender

services/web/test/unit/src/Project/ProjectEntityMongoUpdateHandlerTests.js

@@ -196,9 +196,7 @@ describe('ProjectEntityMongoUpdateHandler', function() {
         './ProjectEntityHandler': this.ProjectEntityHandler,
         './ProjectLocator': this.ProjectLocator,
         './ProjectGetter': this.ProjectGetter,
-        './FolderStructureBuilder': this.FolderStructureBuilder,
-        // We need to provide Errors here to make instance check work
-        '../Errors/Errors': Errors
+        './FolderStructureBuilder': this.FolderStructureBuilder
       }
     })
   })

services/web/test/unit/src/Project/ProjectEntityUpdateHandlerTests.js

@@ -154,11 +154,11 @@ describe('ProjectEntityUpdateHandler', function() {
         console: console
       },
       requires: {
+        'settings-sharelatex': { validRootDocExtensions: ['tex'] },
         'logger-sharelatex': this.logger,
         fs: this.fs,
         '../../models/Doc': { Doc: this.DocModel },
         '../Docstore/DocstoreManager': this.DocstoreManager,
-        '../Errors/Errors': Errors,
         '../../Features/DocumentUpdater/DocumentUpdaterHandler': this
           .DocumentUpdaterHandler,
         '../../models/File': { File: this.FileModel },

services/web/test/unit/src/Project/ProjectLocatorTests.js

@@ -58,7 +58,6 @@ describe('ProjectLocator', function() {
       requires: {
         '../../models/Project': { Project },
         '../../models/User': { User: this.User },
-        '../Errors/Errors': Errors,
         './ProjectGetter': this.ProjectGetter,
         './ProjectHelper': this.ProjectHelper,
         'logger-sharelatex': {

services/web/test/unit/src/Security/OneTimeTokenHandlerTests.js

@@ -40,7 +40,6 @@ describe('OneTimeTokenHandler', function() {
         crypto: {
           randomBytes: () => this.stubbedToken
         },
-        '../Errors/Errors': Errors,
         '../../infrastructure/mongodb': {
           db: (this.db = { tokens: {} })
         }

services/web/test/unit/src/Subscription/LimitationsManagerTests.js

@@ -19,7 +19,6 @@ const modulePath = require('path').join(
   __dirname,
   '../../../../app/src/Features/Subscription/LimitationsManager'
 )
-const Settings = require('settings-sharelatex')
 
 describe('LimitationsManager', function() {
   beforeEach(function() {

services/web/test/unit/src/Subscription/RecurlyWrapperTests.js

@@ -158,9 +158,7 @@ describe('RecurlyWrapper', function() {
             log: sinon.stub()
           },
           request: sinon.stub(),
-          xml2js: require('xml2js'),
-          './Errors': SubscriptionErrors,
-          '../Errors/Errors': Errors
+          './Errors': SubscriptionErrors
         }
       }
     ))

services/web/test/unit/src/Subscription/SubscriptionControllerTests.js

@@ -122,7 +122,6 @@ describe('SubscriptionController', function() {
         '../Errors/HttpErrorHandler': (this.HttpErrorHandler = {
           unprocessableEntity: sinon.stub()
         }),
-        '../Errors/Errors': Errors,
         './Errors': SubscriptionErrors
       }
     })

services/web/test/unit/src/Subscription/SubscriptionLocatorTests.js

@@ -35,6 +35,7 @@ describe('Subscription Locator Tests', function() {
         console: console
       },
       requires: {
+        './GroupPlansData': {},
         '../../models/Subscription': {
           Subscription: this.Subscription
         },

services/web/test/unit/src/Subscription/TeamInvitesHandlerTests.js

@@ -97,8 +97,7 @@ describe('TeamInvitesHandler', function() {
         './SubscriptionLocator': this.SubscriptionLocator,
         './SubscriptionUpdater': this.SubscriptionUpdater,
         './LimitationsManager': this.LimitationsManager,
-        '../Email/EmailHandler': this.EmailHandler,
-        '../Errors/Errors': Errors
+        '../Email/EmailHandler': this.EmailHandler
       }
     })
   })

services/web/test/unit/src/Templates/TemplatesControllerTests.js

@@ -14,6 +14,7 @@ const SandboxedModule = require('sandboxed-module')
 const assert = require('assert')
 const chai = require('chai')
 const sinon = require('sinon')
+const ProjectHelper = require('../../../../app/src/Features/Project/ProjectHelper')
 
 chai.should()
 const { expect } = chai
@@ -28,6 +29,7 @@ describe('TemplatesController', function() {
         console: console
       },
       requires: {
+        '../Project/ProjectHelper': ProjectHelper,
         '../Authentication/AuthenticationController': (this.AuthenticationController = {
           getLoggedInUserId: sinon.stub().returns(this.user_id)
         }),

services/web/test/unit/src/Templates/TemplatesManagerTests.js

@@ -69,6 +69,7 @@ describe('TemplatesManager', function() {
         console: console
       },
       requires: {
+        'request-promise-native': sinon.stub(),
         '../Uploads/ProjectUploadManager': this.ProjectUploadManager,
         '../Project/ProjectOptionsHandler': this.ProjectOptionsHandler,
         '../Project/ProjectRootDocManager': this.ProjectRootDocManager,
@@ -77,7 +78,6 @@ describe('TemplatesManager', function() {
           getLoggedInUserId: sinon.stub()
         }),
         '../../infrastructure/FileWriter': this.FileWriter,
-        './TemplatesPublisher': this.TemplatesPublisher,
         'logger-sharelatex': {
           log() {},
           err() {}

services/web/test/unit/src/ThirdPartyDataStore/TpdsProjectFlusherTests.js

@@ -50,6 +50,7 @@ describe('TpdsProjectFlusher', function() {
 
     this.TpdsProjectFlusher = SandboxedModule.require(MODULE_PATH, {
       requires: {
+        'logger-sharelatex': { debug() {} },
         '../DocumentUpdater/DocumentUpdaterHandler': this
           .DocumentUpdaterHandler,
         '../Project/ProjectGetter': this.ProjectGetter,

services/web/test/unit/src/TokenAccess/TokenAccessHandlerTests.js

@@ -41,8 +41,8 @@ describe('TokenAccessHandler', function() {
       requires: {
         mongodb: { ObjectId },
         '../../models/Project': { Project: (this.Project = {}) },
+        'logger-sharelatex': { err: sinon.stub() },
         'settings-sharelatex': (this.settings = {}),
-        '../Collaborators/CollaboratorsGetter': (this.CollaboratorsGetter = {}),
         '../User/UserGetter': (this.UserGetter = {}),
         '../V1/V1Api': (this.V1Api = {
           request: sinon.stub()

services/web/test/unit/src/Uploads/ArchiveManagerTests.js

@@ -50,6 +50,7 @@ describe('ArchiveManager', function() {
         console: console
       },
       requires: {
+        'settings-sharelatex': {},
         yauzl: (this.yauzl = {
           open: sinon.stub().callsArgWith(2, null, this.zipfile)
         }),

services/web/test/unit/src/Uploads/ProjectUploadControllerTests.js

@@ -48,6 +48,8 @@ describe('ProjectUploadController', function() {
         console: console
       },
       requires: {
+        multer: sinon.stub(),
+        'settings-sharelatex': { path: {} },
         './ProjectUploadManager': (this.ProjectUploadManager = {}),
         './FileSystemImportManager': (this.FileSystemImportManager = {}),
         'logger-sharelatex': (this.logger = {

services/web/test/unit/src/Uploads/ProjectUploadManagerTests.js

@@ -149,6 +149,7 @@ describe('ProjectUploadManager', function() {
         console: console
       },
       requires: {
+        'logger-sharelatex': {},
         'fs-extra': this.fs,
         './ArchiveManager': this.ArchiveManager,
         '../../models/Doc': { Doc: this.Doc },

services/web/test/unit/src/User/SAMLIdentityManagerTests.js

@@ -2,17 +2,13 @@ const sinon = require('sinon')
 const chai = require('chai')
 const { expect } = chai
 const SandboxedModule = require('sandboxed-module')
+const Errors = require('../../../../app/src/Features/Errors/Errors')
 const modulePath = '../../../../app/src/Features/User/SAMLIdentityManager.js'
 
 describe('SAMLIdentityManager', function() {
   const linkedEmail = 'another@example.com'
 
   beforeEach(function() {
-    this.Errors = {
-      EmailExistsError: sinon.stub(),
-      NotFoundError: sinon.stub(),
-      SAMLIdentityExistsError: sinon.stub()
-    }
     this.userId = 'user-id-1'
     this.user = {
       _id: this.userId,
@@ -166,7 +162,7 @@ describe('SAMLIdentityManager', function() {
           } catch (e) {
             error = e
           } finally {
-            expect(error).to.be.instanceof(this.Errors.EmailExistsError)
+            expect(error).to.be.instanceof(Errors.EmailExistsError)
             expect(this.User.findOneAndUpdate).to.not.have.been.called
           }
         })
@@ -197,7 +193,7 @@ describe('SAMLIdentityManager', function() {
           } catch (e) {
             error = e
           } finally {
-            expect(error).to.be.instanceof(this.Errors.SAMLIdentityExistsError)
+            expect(error).to.be.instanceof(Errors.SAMLIdentityExistsError)
             expect(this.User.findOneAndUpdate).to.not.have.been.called
           }
         })

services/web/test/unit/src/User/ThirdPartyIdentityManagerTests.js

@@ -26,10 +26,6 @@ describe('ThirdPartyIdentityManager', function() {
         '../../../../app/src/Features/Email/EmailHandler': (this.EmailHandler = {
           sendEmail: sinon.stub().yields()
         }),
-        Errors: (this.Errors = {
-          ThirdPartyIdentityExistsError: sinon.stub(),
-          ThirdPartyUserNotFoundError: sinon.stub()
-        }),
         'logger-sharelatex': (this.logger = {
           error: sinon.stub()
         }),

services/web/test/unit/src/User/UserControllerTests.js

@@ -57,8 +57,6 @@ describe('UserController', function() {
         setUserPassword: sinon.stub()
       }
     }
-    this.ReferalAllocator = { allocate: sinon.stub() }
-    this.SubscriptionDomainHandler = { autoAllocate: sinon.stub() }
     this.UserUpdater = {
       changeEmailAddress: sinon.stub(),
       promises: {
@@ -83,11 +81,21 @@ describe('UserController', function() {
       unprocessableEntity: sinon.stub(),
       legacyInternal: sinon.stub()
     }
+
+    this.UrlHelper = {
+      getSafeRedirectPath: sinon.stub()
+    }
+    this.UrlHelper.getSafeRedirectPath
+      .withArgs('https://evil.com')
+      .returns(undefined)
+    this.UrlHelper.getSafeRedirectPath.returnsArg(0)
+
     this.UserController = SandboxedModule.require(modulePath, {
       globals: {
         console: console
       },
       requires: {
+        '../Helpers/UrlHelper': this.UrlHelper,
         './UserGetter': this.UserGetter,
         './UserDeleter': this.UserDeleter,
         './UserUpdater': this.UserUpdater,
@@ -102,9 +110,6 @@ describe('UserController', function() {
         '../../infrastructure/Features': (this.Features = {
           hasFeature: sinon.stub()
         }),
-        '../Referal/ReferalAllocator': this.ReferalAllocator,
-        '../Subscription/SubscriptionDomainHandler': this
-          .SubscriptionDomainHandler,
         './UserAuditLogHandler': (this.UserAuditLogHandler = {
           promises: {
             addEntry: sinon.stub().resolves()
@@ -123,7 +128,6 @@ describe('UserController', function() {
         '@overleaf/metrics': {
           inc() {}
         },
-        '../Errors/Errors': Errors,
         '@overleaf/o-error': OError,
         '../Email/EmailHandler': (this.EmailHandler = {
           sendEmail: sinon.stub(),
@@ -517,6 +521,16 @@ describe('UserController', function() {
       this.UserController.logout(this.req, this.res)
     })
 
+    it('should redirect after logout, but not to evil.com', function(done) {
+      this.req.body.redirect = 'https://evil.com'
+      this.req.session.destroy = sinon.stub().callsArgWith(0)
+      this.res.redirect = url => {
+        url.should.equal('/login')
+        done()
+      }
+      this.UserController.logout(this.req, this.res)
+    })
+
     it('should redirect to login after logout when no redirect set', function(done) {
       this.req.session.destroy = sinon.stub().callsArgWith(0)
       this.res.redirect = url => {

services/web/test/unit/src/User/UserDeleterTests.js

@@ -102,8 +102,7 @@ describe('UserDeleter', function() {
           log: sinon.stub(),
           warn: sinon.stub(),
           err: sinon.stub()
-        }),
-        '../Errors/Errors': Errors
+        })
       },
       globals: {
         console: console

services/web/test/unit/src/User/UserEmailsConfirmationHandlerTests.js

@@ -35,7 +35,6 @@ describe('UserEmailsConfirmationHandler', function() {
         }),
         'logger-sharelatex': (this.logger = { log: sinon.stub() }),
         '../Security/OneTimeTokenHandler': (this.OneTimeTokenHandler = {}),
-        '../Errors/Errors': Errors,
         './UserUpdater': (this.UserUpdater = {}),
         './UserGetter': (this.UserGetter = {
           getUser: sinon.stub().yields(null, this.mockUser)

services/web/test/unit/src/User/UserEmailsControllerTests.js

@@ -79,7 +79,6 @@ describe('UserEmailsController', function() {
         }),
         '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
         '../Errors/HttpErrorHandler': this.HttpErrorHandler,
-        '../Errors/Errors': Errors,
         'logger-sharelatex': (this.logger = {
           log() {},
           warn: sinon.stub(),

services/web/test/unit/src/User/UserGetterTests.js

@@ -63,8 +63,7 @@ describe('UserGetter', function() {
         },
         '../../infrastructure/Features': {
           hasFeature: sinon.stub().returns(true)
-        },
-        '../Errors/Errors': Errors
+        }
       }
     })
   })

services/web/test/unit/src/User/UserSessionsManagerTests.js

@@ -17,7 +17,6 @@ const should = chai.should()
 const { expect } = chai
 const modulePath = '../../../../app/src/Features/User/UserSessionsManager.js'
 const SandboxedModule = require('sandboxed-module')
-const Async = require('async')
 
 describe('UserSessionsManager', function() {
   beforeEach(function() {
@@ -69,8 +68,7 @@ describe('UserSessionsManager', function() {
       requires: {
         'logger-sharelatex': this.logger,
         'settings-sharelatex': this.settings,
-        './UserSessionsRedis': this.UserSessionsRedis,
-        async: Async
+        './UserSessionsRedis': this.UserSessionsRedis
       }
     }))
   })

services/web/test/unit/src/UserMembership/UserMembershipControllerTests.js

@@ -78,7 +78,6 @@ describe('UserMembershipController', function() {
           '../Authentication/AuthenticationController': this
             .AuthenticationController,
           './UserMembershipHandler': this.UserMembershipHandler,
-          '../Errors/Errors': Errors,
           'logger-sharelatex': {
             log() {},
             err() {}

services/web/test/unit/src/UserMembership/UserMembershipHandlerTests.js

@@ -75,7 +75,6 @@ describe('UserMembershipHandler', function() {
         mongodb: { ObjectId },
         './UserMembershipViewModel': this.UserMembershipViewModel,
         '../User/UserGetter': this.UserGetter,
-        '../Errors/Errors': Errors,
         '../../models/Institution': {
           Institution: this.Institution
         },

services/web/test/unit/src/infrastructure/LockManager/ReleasingTheLock.js

@@ -26,6 +26,16 @@ describe('LockManager - releasing the lock', function() {
     'logger-sharelatex': {
       log() {}
     },
+    'settings-sharelatex': {
+      redis: {},
+      lockManager: {
+        lockTestInterval: 50,
+        maxTestInterval: 1000,
+        maxLockWaitTime: 10000,
+        redisLockExpiry: 30,
+        slowExecutionThreshold: 5000
+      }
+    },
     '@overleaf/metrics': {},
     './RedisWrapper': {
       client() {

services/web/test/unit/src/infrastructure/LockManager/getLockTests.js

@@ -120,6 +120,7 @@ describe('LockManager - getting the lock', function() {
   describe('when the lock times out', function() {
     beforeEach(function(done) {
       const time = Date.now()
+      this.LockManager.LOCK_TEST_INTERVAL = 1
       this.LockManager.MAX_LOCK_WAIT_TIME = 5
       this.LockManager._tryLock = sinon.stub().yields(null, false)
       return this.LockManager._getLock(this.key, this.namespace, (...args) => {
@@ -152,10 +153,10 @@ describe('LockManager - getting the lock', function() {
         }
       }
       // Start ten lock requests in order at 1ms 2ms 3ms...
-      // with them randomly holding the lock for 0-100ms.
+      // with them randomly holding the lock for 0-10ms.
       // Use predefined values for the random delay to make the test
       // deterministic.
-      const randomDelays = [52, 45, 41, 84, 60, 81, 31, 46, 9, 43]
+      const randomDelays = [5, 4, 1, 8, 6, 8, 3, 4, 2, 4]
       let startTime = 0
       return Array.from(randomDelays).map((randomDelay, i) =>
         ((randomDelay, i) => {