From 22b8c114a2f8667e9e7dc6ab701ca07ac4946573 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Fri, 23 Feb 2018 17:00:29 +0000 Subject: [PATCH 01/19] Upgrade mocha to latest --- services/web/npm-shrinkwrap.json | 133 +++++++++++++++++++++++-------- services/web/package.json | 2 +- 2 files changed, 100 insertions(+), 35 deletions(-) diff --git a/services/web/npm-shrinkwrap.json b/services/web/npm-shrinkwrap.json index ccb2e2c467..4ee7207abc 100644 --- a/services/web/npm-shrinkwrap.json +++ b/services/web/npm-shrinkwrap.json @@ -1142,6 +1142,11 @@ } } }, + "browser-stdout": { + "version": "1.3.0", + "from": "browser-stdout@1.3.0", + "resolved": "https://registry.npmjs.org/browser-stdout/-/browser-stdout-1.3.0.tgz" + }, "browserify": { "version": "14.5.0", "from": "browserify@>=14.5.0 <15.0.0", @@ -4651,7 +4656,59 @@ "version": "0.9.0", "from": "grunt-mocha-test@0.9.0", "resolved": "https://registry.npmjs.org/grunt-mocha-test/-/grunt-mocha-test-0.9.0.tgz", - "dev": true + "dev": true, + "dependencies": { + "commander": { + "version": "2.0.0", + "from": "commander@2.0.0", + "resolved": "https://registry.npmjs.org/commander/-/commander-2.0.0.tgz", + "dev": true + }, + "glob": { + "version": "3.2.3", + "from": "glob@3.2.3", + "resolved": "https://registry.npmjs.org/glob/-/glob-3.2.3.tgz", + "dev": true + }, + "graceful-fs": { + "version": "2.0.3", + "from": "graceful-fs@>=2.0.0 <2.1.0", + "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-2.0.3.tgz", + "dev": true + }, + "jade": { + "version": "0.26.3", + "from": "jade@0.26.3", + "resolved": "https://registry.npmjs.org/jade/-/jade-0.26.3.tgz", + "dev": true, + "dependencies": { + "commander": { + "version": "0.6.1", + "from": "commander@0.6.1", + "resolved": "https://registry.npmjs.org/commander/-/commander-0.6.1.tgz", + "dev": true + }, + "mkdirp": { + "version": "0.3.0", + "from": "mkdirp@0.3.0", + "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.3.0.tgz", + "dev": true + } + } + }, + "mkdirp": { + "version": "0.3.5", + "from": "mkdirp@0.3.5", + "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.3.5.tgz", + "dev": true + }, + "mocha": { + "version": "1.17.1", + "from": "mocha@>=1.17.1 <1.18.0", + "resolved": "https://registry.npmjs.org/mocha/-/mocha-1.17.1.tgz", + "dev": true + } + } }, "grunt-newer": { "version": "1.3.0", @@ -4831,6 +4888,11 @@ "from": "hawk@>=6.0.2 <6.1.0", "resolved": "https://registry.npmjs.org/hawk/-/hawk-6.0.2.tgz" }, + "he": { + "version": "1.1.1", + "from": "he@1.1.1", + "resolved": "https://registry.npmjs.org/he/-/he-1.1.1.tgz" + }, "heapdump": { "version": "0.3.9", "from": "heapdump@>=0.3.7 <0.4.0", @@ -7463,46 +7525,49 @@ "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz" }, "mocha": { - "version": "1.17.1", - "from": "https://registry.npmjs.org/mocha/-/mocha-1.17.1.tgz", - "resolved": "https://registry.npmjs.org/mocha/-/mocha-1.17.1.tgz", + "version": "5.0.1", + "from": "mocha@5.0.1", + "resolved": "https://registry.npmjs.org/mocha/-/mocha-5.0.1.tgz", "dependencies": { "commander": { - "version": "2.0.0", - "from": "commander@2.0.0", - "resolved": "https://registry.npmjs.org/commander/-/commander-2.0.0.tgz" + "version": "2.11.0", + "from": "commander@2.11.0", + "resolved": "https://registry.npmjs.org/commander/-/commander-2.11.0.tgz" + }, + "debug": { + "version": "3.1.0", + "from": "debug@3.1.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz" + }, + "diff": { + "version": "3.3.1", + "from": "diff@3.3.1", + "resolved": "https://registry.npmjs.org/diff/-/diff-3.3.1.tgz" }, "glob": { - "version": "3.2.3", - "from": "glob@3.2.3", - "resolved": "https://registry.npmjs.org/glob/-/glob-3.2.3.tgz" + "version": "7.1.2", + "from": "glob@7.1.2", + "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.2.tgz" }, - "graceful-fs": { - "version": "2.0.3", - "from": "graceful-fs@>=2.0.0 <2.1.0", - "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-2.0.3.tgz" + "growl": { + "version": "1.10.3", + "from": "growl@1.10.3", + "resolved": "https://registry.npmjs.org/growl/-/growl-1.10.3.tgz" }, - "jade": { - "version": "0.26.3", - "from": "jade@0.26.3", - "resolved": "https://registry.npmjs.org/jade/-/jade-0.26.3.tgz", - "dependencies": { - "commander": { - "version": "0.6.1", - "from": "commander@0.6.1", - "resolved": "https://registry.npmjs.org/commander/-/commander-0.6.1.tgz" - }, - "mkdirp": { - "version": "0.3.0", - "from": "mkdirp@0.3.0", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.3.0.tgz" - } - } + "has-flag": { + "version": "2.0.0", + "from": "has-flag@>=2.0.0 <3.0.0", + "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-2.0.0.tgz" }, - "mkdirp": { - "version": "0.3.5", - "from": "mkdirp@0.3.5", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.3.5.tgz" + "minimatch": { + "version": "3.0.4", + "from": "minimatch@>=3.0.4 <4.0.0", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.4.tgz" + }, + "supports-color": { + "version": "4.4.0", + "from": "supports-color@4.4.0", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-4.4.0.tgz" } } }, diff --git a/services/web/package.json b/services/web/package.json index 4b33913be7..3853c0bb3d 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -53,7 +53,7 @@ "method-override": "^2.3.3", "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.7.1", "mimelib": "0.2.14", - "mocha": "1.17.1", + "mocha": "^5.0.1", "mongojs": "2.4.0", "mongoose": "4.11.4", "multer": "^0.1.8", From e6f624c7a068bfabea411f1a7bbb365314072298 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Fri, 23 Feb 2018 17:16:46 +0000 Subject: [PATCH 02/19] Make sure mocha exits --- services/web/bin/unit_test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/bin/unit_test b/services/web/bin/unit_test index da13a441fa..b7e14f514f 100755 --- a/services/web/bin/unit_test +++ b/services/web/bin/unit_test @@ -1,7 +1,7 @@ #!/bin/bash set -e; -MOCHA="node_modules/.bin/mocha --recursive --reporter spec" +MOCHA="node_modules/.bin/mocha --exit --recursive --reporter spec" $MOCHA "$@" test/unit/js From d79e226a20e3666b03b1d61900b1d035afd29978 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 10:58:20 +0000 Subject: [PATCH 03/19] Fix callback defined in wrong describe block scope --- .../coffee/References/ReferencesControllerTests.coffee | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/services/web/test/unit/coffee/References/ReferencesControllerTests.coffee b/services/web/test/unit/coffee/References/ReferencesControllerTests.coffee index 37a448d34a..7af553d5ec 100644 --- a/services/web/test/unit/coffee/References/ReferencesControllerTests.coffee +++ b/services/web/test/unit/coffee/References/ReferencesControllerTests.coffee @@ -142,13 +142,15 @@ describe "ReferencesController", -> describe 'index', -> + beforeEach -> + @call = (callback) => + @controller.index @req, @res + callback() + describe 'with docIds as an array and shouldBroadcast as false', -> beforeEach -> @ReferencesHandler.index.callsArgWith(2, null, @fakeResponseData) - @call = (callback) => - @controller.index @req, @res - callback() it 'should call ReferencesHandler.index', (done) -> @call () => From 192eb8b44fe530ad3b3aad185c90a8dd438ce90f Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 11:16:04 +0000 Subject: [PATCH 04/19] Fix error not being defined in wrong describe block --- .../coffee/Authentication/AuthenticationControllerTests.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee index a014b28098..92d2a7dbdb 100644 --- a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee @@ -99,6 +99,7 @@ describe "AuthenticationController", -> @req.session.save = sinon.stub().callsArgWith(0, null) @req.sessionStore = {generate: sinon.stub()} @passport.authenticate.callsArgWith(1, null, @user, @info) + @err = new Error('woops') it 'should call passport.authenticate', () -> @AuthenticationController.passportLogin @req, @res, @next @@ -107,7 +108,6 @@ describe "AuthenticationController", -> describe 'when authenticate produces an error', -> beforeEach -> - @err = new Error('woops') @passport.authenticate.callsArgWith(1, @err) it 'should return next with an error', () -> From 34b53726d4319e4b3704a4761d5ee2b54d594688 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 11:19:52 +0000 Subject: [PATCH 05/19] Fix error not being defined in wrong describe block --- .../Collaborators/CollaboratorsInviteControllerTests.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee b/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee index 0adff748c0..cdfff78249 100644 --- a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee +++ b/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee @@ -108,6 +108,7 @@ describe "CollaboratorsInviteController", -> } @LimitationsManager.canAddXCollaborators = sinon.stub().callsArgWith(2, null, true) @CollaboratorsInviteHandler.inviteToProject = sinon.stub().callsArgWith(4, null, @invite) + @err = new Error('woops') @callback = sinon.stub() @next = sinon.stub() @@ -163,7 +164,6 @@ describe "CollaboratorsInviteController", -> beforeEach -> @CollaboratorsInviteController._checkShouldInviteEmail = sinon.stub().callsArgWith(1, null, true) @CollaboratorsInviteController._checkRateLimit = sinon.stub().yields(null, true) - @err = new Error('woops') @LimitationsManager.canAddXCollaborators = sinon.stub().callsArgWith(2, @err) @CollaboratorsInviteController.inviteToProject @req, @res, @next From cfc9dbdbb3eca88995e959cbe7ea3e6b5ad489a3 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 11:34:08 +0000 Subject: [PATCH 06/19] Fix bad scoping of user agent --- .../coffee/FileStore/FileStoreControllerTests.coffee | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/services/web/test/unit/coffee/FileStore/FileStoreControllerTests.coffee b/services/web/test/unit/coffee/FileStore/FileStoreControllerTests.coffee index 3b3f546e9f..7c59be92f0 100644 --- a/services/web/test/unit/coffee/FileStore/FileStoreControllerTests.coffee +++ b/services/web/test/unit/coffee/FileStore/FileStoreControllerTests.coffee @@ -74,11 +74,10 @@ describe "FileStoreController", -> describe "with a '#{extension}' file extension", -> beforeEach -> - @user_agent = 'A generic browser' @file.name = "bad#{extension}" @req.get = (key) => if key == 'User-Agent' - @user_agent + return 'A generic browser' describe "from a non-ios browser", -> @@ -91,7 +90,9 @@ describe "FileStoreController", -> describe "from an iPhone", -> beforeEach -> - @user_agent = "An iPhone browser" + @req.get = (key) => + if key == 'User-Agent' + return "An iPhone browser" it "should set Content-Type to 'text/plain'", (done) -> @stream.pipe = (des) => @@ -102,7 +103,9 @@ describe "FileStoreController", -> describe "from an iPad", -> beforeEach -> - @user_agent = "An iPad browser" + @req.get = (key) => + if key == 'User-Agent' + return "An iPad browser" it "should set Content-Type to 'text/plain'", (done) -> @stream.pipe = (des) => From 5d017beac5f82d25b58e1d38ac9f53b3542e859c Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 11:46:06 +0000 Subject: [PATCH 07/19] Fix stub incorrectly overriding top level beforeEach --- .../unit/coffee/FileStore/FileStoreHandlerTests.coffee | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/services/web/test/unit/coffee/FileStore/FileStoreHandlerTests.coffee b/services/web/test/unit/coffee/FileStore/FileStoreHandlerTests.coffee index 90a3e870d1..5d21a98671 100644 --- a/services/web/test/unit/coffee/FileStore/FileStoreHandlerTests.coffee +++ b/services/web/test/unit/coffee/FileStore/FileStoreHandlerTests.coffee @@ -11,7 +11,7 @@ describe "FileStoreHandler", -> @fs = createReadStream : sinon.stub() lstat: sinon.stub().callsArgWith(1, null, { - isFile:=> @isSafeOnFileSystem + isFile:=> true isDirectory:-> return false }) @writeStream = @@ -35,7 +35,6 @@ describe "FileStoreHandler", -> describe "uploadFileFromDisk", -> beforeEach -> @request.returns(@writeStream) - @isSafeOnFileSystem = true it "should create read stream", (done)-> @fs.createReadStream.returns @@ -91,8 +90,13 @@ describe "FileStoreHandler", -> done() describe "symlink", -> + beforeEach -> + @fs.lstat = sinon.stub().callsArgWith(1, null, { + isFile:=> false + isDirectory:-> return false + }) + it "should not read file if it is symlink", (done)-> - @isSafeOnFileSystem = false @handler.uploadFileFromDisk @project_id, @file_id, @fsPath, => @fs.createReadStream.called.should.equal false done() From fd8c61985f1001d82a6a45921334baf67cbc1af4 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 12:19:56 +0000 Subject: [PATCH 08/19] Fix bug where stubs were attached to undefined variables --- .../Subscription/SubscriptionViewModelBuilderTests.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionViewModelBuilderTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionViewModelBuilderTests.coffee index 029deec629..1f2c3f8808 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionViewModelBuilderTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionViewModelBuilderTests.coffee @@ -38,8 +38,8 @@ describe 'SubscriptionViewModelBuilder', -> @builder = SandboxedModule.require modulePath, requires: "settings-sharelatex": { apis: { recurly: { subdomain: "example.com" }}} "./RecurlyWrapper": @RecurlyWrapper - "./PlansLocator": @PlansLocator - "./SubscriptionLocator": @SubscriptionLocator + "./PlansLocator": @PlansLocator = {} + "./SubscriptionLocator": @SubscriptionLocator = {} "./SubscriptionFormatters": @SubscriptionFormatters "./LimitationsManager": {} "logger-sharelatex": From 2529ed756abab0f174ddd43f1f7aa3a71d343b22 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 12:27:54 +0000 Subject: [PATCH 09/19] Fix callback being defined in wrong describe block --- .../unit/coffee/SudoMode/SudoModeMiddlewearTests.coffee | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/web/test/unit/coffee/SudoMode/SudoModeMiddlewearTests.coffee b/services/web/test/unit/coffee/SudoMode/SudoModeMiddlewearTests.coffee index 00e8d0ae57..d17c397d3c 100644 --- a/services/web/test/unit/coffee/SudoMode/SudoModeMiddlewearTests.coffee +++ b/services/web/test/unit/coffee/SudoMode/SudoModeMiddlewearTests.coffee @@ -105,6 +105,12 @@ describe 'SudoModeMiddlewear', -> describe 'when external auth is being used', -> beforeEach -> @externalAuth = true + @call = (cb) => + @req = {externalAuthenticationSystemUsed: sinon.stub().returns(@externalAuth)} + @res = {redirect: sinon.stub()} + @next = sinon.stub() + @SudoModeMiddlewear.protectPage @req, @res, @next + cb() it 'should immediately return next with no args', (done) -> @call () => From ec7237b7e83ddbd3c9a30df25f94f502b5a8aaa6 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 12:33:48 +0000 Subject: [PATCH 10/19] Fix callback stub being defined in wrong scope --- .../web/test/unit/coffee/Uploads/ArchiveManagerTests.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/test/unit/coffee/Uploads/ArchiveManagerTests.coffee b/services/web/test/unit/coffee/Uploads/ArchiveManagerTests.coffee index 61efbccce7..5cabf944b0 100644 --- a/services/web/test/unit/coffee/Uploads/ArchiveManagerTests.coffee +++ b/services/web/test/unit/coffee/Uploads/ArchiveManagerTests.coffee @@ -26,12 +26,12 @@ describe "ArchiveManager", -> "metrics-sharelatex": @metrics "fs": @fs = {} "fs-extra": @fse = {} + @callback = sinon.stub() describe "extractZipArchive", -> beforeEach -> @source = "/path/to/zip/source.zip" @destination = "/path/to/zip/destination" - @callback = sinon.stub() @ArchiveManager._isZipTooLarge = sinon.stub().callsArgWith(1, null, false) describe "successfully", -> From be6fa346d5ed97842c91c687f4157e725a5d9fd7 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 13:37:17 +0000 Subject: [PATCH 11/19] Fix bug where incorrect variable was used --- .../unit/coffee/Uploads/ProjectUploadControllerTests.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/test/unit/coffee/Uploads/ProjectUploadControllerTests.coffee b/services/web/test/unit/coffee/Uploads/ProjectUploadControllerTests.coffee index 4480f15536..9c1d115416 100644 --- a/services/web/test/unit/coffee/Uploads/ProjectUploadControllerTests.coffee +++ b/services/web/test/unit/coffee/Uploads/ProjectUploadControllerTests.coffee @@ -102,7 +102,7 @@ describe "ProjectUploadController", -> @project_id = "project-id-123" @folder_id = "folder-id-123" @path = "/path/to/file/on/disk.png" - @filename = "filename.png" + @name = "filename.png" @req.files = qqfile: path: @path From 136fd8481003dceccbe04395f589bab229e95b02 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 13:52:58 +0000 Subject: [PATCH 12/19] Fix module stub in incorrect scope --- .../infrastructure/RateLimterTests.coffee | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/services/web/test/unit/coffee/infrastructure/RateLimterTests.coffee b/services/web/test/unit/coffee/infrastructure/RateLimterTests.coffee index 06efac5d8b..ab1f544b96 100644 --- a/services/web/test/unit/coffee/infrastructure/RateLimterTests.coffee +++ b/services/web/test/unit/coffee/infrastructure/RateLimterTests.coffee @@ -24,21 +24,16 @@ describe "RateLimiter", -> @RedisWrapper = client: sinon.stub().returns(@rclient) - @limiterFn = sinon.stub() - @RollingRateLimiter = (opts) => - return @limiterFn - - @limiter = SandboxedModule.require modulePath, requires: - "rolling-rate-limiter": @RollingRateLimiter - "settings-sharelatex":@settings - "logger-sharelatex" : @logger = {log:sinon.stub(), err:sinon.stub()} - "./RedisWrapper": @RedisWrapper - @endpointName = "compiles" @subject = "some-project-id" @timeInterval = 20 @throttleLimit = 5 + @requires = + "settings-sharelatex":@settings + "logger-sharelatex" : @logger = {log:sinon.stub(), err:sinon.stub()} + "./RedisWrapper": @RedisWrapper + @details = endpointName: @endpointName subjectName: @subject @@ -52,7 +47,9 @@ describe "RateLimiter", -> describe 'when action is permitted', -> beforeEach -> - @limiterFn = sinon.stub().callsArgWith(1, null, 0, 22) + @requires["rolling-rate-limiter"] = (opts) => + return sinon.stub().callsArgWith(1, null, 0, 22) + @limiter = SandboxedModule.require modulePath, requires: @requires it 'should not produce and error', (done) -> @limiter.addCount {}, (err, should) -> @@ -67,7 +64,9 @@ describe "RateLimiter", -> describe 'when action is not permitted', -> beforeEach -> - @limiterFn = sinon.stub().callsArgWith(1, null, 4000, 0) + @requires["rolling-rate-limiter"] = (opts) => + return sinon.stub().callsArgWith(1, null, 4000, 0) + @limiter = SandboxedModule.require modulePath, requires: @requires it 'should not produce and error', (done) -> @limiter.addCount {}, (err, should) -> @@ -82,7 +81,9 @@ describe "RateLimiter", -> describe 'when limiter produces an error', -> beforeEach -> - @limiterFn = sinon.stub().callsArgWith(1, new Error('woops')) + @requires["rolling-rate-limiter"] = (opts) => + return sinon.stub().callsArgWith(1, new Error('woops')) + @limiter = SandboxedModule.require modulePath, requires: @requires it 'should produce and error', (done) -> @limiter.addCount {}, (err, should) -> From abf53625fe7972009562acece4fd6d14db671d86 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 14:02:08 +0000 Subject: [PATCH 13/19] Fix scope not being applied through callback --- .../coffee/ide/editor/directives/cmEditorTests.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/test/unit_frontend/coffee/ide/editor/directives/cmEditorTests.coffee b/services/web/test/unit_frontend/coffee/ide/editor/directives/cmEditorTests.coffee index 1200d6105c..f7667e74cc 100644 --- a/services/web/test/unit_frontend/coffee/ide/editor/directives/cmEditorTests.coffee +++ b/services/web/test/unit_frontend/coffee/ide/editor/directives/cmEditorTests.coffee @@ -13,12 +13,12 @@ define ['ide/editor/directives/cmEditor'], () -> } it 'inits Rich Text', () -> - inject ($compile, $rootScope) -> + inject ($compile, $rootScope) => $compile('
')($rootScope) expect(@richTextInit).to.have.been.called it 'attaches to CM', () -> - inject ($compile, $rootScope, $browser) -> + inject ($compile, $rootScope, $browser) => getSnapshot = sinon.stub() detachFromCM = sinon.stub() attachToCM = sinon.stub() @@ -40,7 +40,7 @@ define ['ide/editor/directives/cmEditor'], () -> expect(attachToCM).to.have.been.called it 'detaches from CM when destroyed', () -> - inject ($compile, $rootScope) -> + inject ($compile, $rootScope) => @richTextInit.returns({ setValue: sinon.stub() }) detachFromCM = sinon.stub() $rootScope.sharejsDoc = { From 2247e4d4651fed7eb8971428ef5fe2e2df528ce3 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 14:21:14 +0000 Subject: [PATCH 14/19] Fix scoping issues where stubs were defined in wrong describe blocks --- .../LimitationsManagerTests.coffee | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/services/web/test/unit/coffee/Subscription/LimitationsManagerTests.coffee b/services/web/test/unit/coffee/Subscription/LimitationsManagerTests.coffee index f08c8669e1..3e0cf4c499 100644 --- a/services/web/test/unit/coffee/Subscription/LimitationsManagerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/LimitationsManagerTests.coffee @@ -78,19 +78,16 @@ describe "LimitationsManager", -> @callback.calledWith(null, @user.features.collaborators).should.equal true describe "canAddXCollaborators", -> - beforeEach -> - @CollaboratorsHandler.getInvitedCollaboratorCount = (project_id, callback) => callback(null, @current_number) - @CollaboratorsInviteHandler.getInviteCount = (project_id, callback) => callback(null, @invite_count) - sinon.stub @LimitationsManager, - "allowedNumberOfCollaboratorsInProject", - (project_id, callback) => callback(null, @allowed_number) - @callback = sinon.stub() - describe "when the project has fewer collaborators than allowed", -> beforeEach -> @current_number = 1 @allowed_number = 2 @invite_count = 0 + @CollaboratorsHandler.getInvitedCollaboratorCount = (project_id, callback) => callback(null, @current_number) + @CollaboratorsInviteHandler.getInviteCount = (project_id, callback) => callback(null, @invite_count) + sinon.stub @LimitationsManager, "allowedNumberOfCollaboratorsInProject", (project_id, callback) => + callback(null, @allowed_number) + @callback = sinon.stub() @LimitationsManager.canAddXCollaborators(@project_id, 1, @callback) it "should return true", -> @@ -101,6 +98,11 @@ describe "LimitationsManager", -> @current_number = 1 @allowed_number = 4 @invite_count = 1 + @CollaboratorsHandler.getInvitedCollaboratorCount = (project_id, callback) => callback(null, @current_number) + @CollaboratorsInviteHandler.getInviteCount = (project_id, callback) => callback(null, @invite_count) + sinon.stub @LimitationsManager, "allowedNumberOfCollaboratorsInProject", (project_id, callback) => + callback(null, @allowed_number) + @callback = sinon.stub() @LimitationsManager.canAddXCollaborators(@project_id, 1, @callback) it "should return true", -> @@ -111,6 +113,11 @@ describe "LimitationsManager", -> @current_number = 1 @allowed_number = 2 @invite_count = 0 + @CollaboratorsHandler.getInvitedCollaboratorCount = (project_id, callback) => callback(null, @current_number) + @CollaboratorsInviteHandler.getInviteCount = (project_id, callback) => callback(null, @invite_count) + sinon.stub @LimitationsManager, "allowedNumberOfCollaboratorsInProject", (project_id, callback) => + callback(null, @allowed_number) + @callback = sinon.stub() @LimitationsManager.canAddXCollaborators(@project_id, 2, @callback) it "should return false", -> @@ -121,6 +128,11 @@ describe "LimitationsManager", -> @current_number = 3 @allowed_number = 2 @invite_count = 0 + @CollaboratorsHandler.getInvitedCollaboratorCount = (project_id, callback) => callback(null, @current_number) + @CollaboratorsInviteHandler.getInviteCount = (project_id, callback) => callback(null, @invite_count) + sinon.stub @LimitationsManager, "allowedNumberOfCollaboratorsInProject", (project_id, callback) => + callback(null, @allowed_number) + @callback = sinon.stub() @LimitationsManager.canAddXCollaborators(@project_id, 1, @callback) it "should return false", -> @@ -131,6 +143,11 @@ describe "LimitationsManager", -> @current_number = 100 @allowed_number = -1 @invite_count = 0 + @CollaboratorsHandler.getInvitedCollaboratorCount = (project_id, callback) => callback(null, @current_number) + @CollaboratorsInviteHandler.getInviteCount = (project_id, callback) => callback(null, @invite_count) + sinon.stub @LimitationsManager, "allowedNumberOfCollaboratorsInProject", (project_id, callback) => + callback(null, @allowed_number) + @callback = sinon.stub() @LimitationsManager.canAddXCollaborators(@project_id, 1, @callback) it "should return true", -> @@ -141,6 +158,11 @@ describe "LimitationsManager", -> @current_number = 0 @allowed_number = 2 @invite_count = 2 + @CollaboratorsHandler.getInvitedCollaboratorCount = (project_id, callback) => callback(null, @current_number) + @CollaboratorsInviteHandler.getInviteCount = (project_id, callback) => callback(null, @invite_count) + sinon.stub @LimitationsManager, "allowedNumberOfCollaboratorsInProject", (project_id, callback) => + callback(null, @allowed_number) + @callback = sinon.stub() @LimitationsManager.canAddXCollaborators(@project_id, 1, @callback) it "should return false", -> @@ -151,6 +173,11 @@ describe "LimitationsManager", -> @current_number = 1 @allowed_number = 2 @invite_count = 1 + @CollaboratorsHandler.getInvitedCollaboratorCount = (project_id, callback) => callback(null, @current_number) + @CollaboratorsInviteHandler.getInviteCount = (project_id, callback) => callback(null, @invite_count) + sinon.stub @LimitationsManager, "allowedNumberOfCollaboratorsInProject", (project_id, callback) => + callback(null, @allowed_number) + @callback = sinon.stub() @LimitationsManager.canAddXCollaborators(@project_id, 1, @callback) it "should return false", -> From 46ac74a16034fc879453262ddb963cf14fbc3d41 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 14:26:04 +0000 Subject: [PATCH 15/19] Fix incorrectly scoped variable --- .../web/test/unit/coffee/Project/ProjectLocatorTests.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/test/unit/coffee/Project/ProjectLocatorTests.coffee b/services/web/test/unit/coffee/Project/ProjectLocatorTests.coffee index fe5464fc86..e19c481d8a 100644 --- a/services/web/test/unit/coffee/Project/ProjectLocatorTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectLocatorTests.coffee @@ -296,7 +296,7 @@ describe 'ProjectLocator', -> it "should not crash with a null", (done)-> path = "/other.tex" - @locator.findElementByPath {project_id: @project._id, path}, (err, element)-> + @locator.findElementByPath {project_id: project._id, path}, (err, element)-> expect(err).to.exist done() From bd7e4908a22a32d56effe0f8f1228cccfb796be8 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 15:30:56 +0000 Subject: [PATCH 16/19] Force mocha to exit after tests run --- services/web/bin/acceptance_test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/bin/acceptance_test b/services/web/bin/acceptance_test index fd2e5137b5..2479328076 100755 --- a/services/web/bin/acceptance_test +++ b/services/web/bin/acceptance_test @@ -1,4 +1,4 @@ #!/bin/bash set -e; -MOCHA="node_modules/.bin/mocha --recursive --reporter spec --timeout 15000" +MOCHA="node_modules/.bin/mocha --exit --recursive --reporter spec --timeout 15000" $MOCHA "$@" From 3217a3fbf0a9665c906dd561de36fffe6c5bc804 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 15:31:34 +0000 Subject: [PATCH 17/19] Fix mocha complaining about returning Promise-like object --- .../test/acceptance/coffee/ProjectStructureMongoLockTest.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/test/acceptance/coffee/ProjectStructureMongoLockTest.coffee b/services/web/test/acceptance/coffee/ProjectStructureMongoLockTest.coffee index 8e39e7e2f2..02019b74dd 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureMongoLockTest.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureMongoLockTest.coffee @@ -34,6 +34,7 @@ describe "ProjectStructureMongoLock", -> namespace = ProjectEntityMongoUpdateHandler.LOCK_NAMESPACE @lock_key = "lock:web:#{namespace}:#{project._id}" LockManager._getLock @lock_key, namespace, done + return after (done) -> LockManager._releaseLock @lock_key, done From 19cec9451db4a64184613a432ed0fe172be6a82e Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 15:31:41 +0000 Subject: [PATCH 18/19] Switch to using scoped variable instead of variables on context Changes to mocha mean that a new context is passed to each describe block instead of it persisting between them. This means that this test cannot be parallelised, however this was the case beforehand (subsequent tests are dependent on earlier tests). --- .../coffee/ProjectStructureTests.coffee | 83 ++++++++++--------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index 7e9801176b..77106e20a3 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -16,6 +16,12 @@ request = require "./helpers/request" User = require "./helpers/User" describe "ProjectStructureChanges", -> + example_project_id = null + example_doc_id = null + example_file_id = null + example_folder_id_1 = null + example_folder_id_2 = null + before (done) -> @owner = new User() @owner.login done @@ -25,11 +31,11 @@ describe "ProjectStructureChanges", -> MockDocUpdaterApi.clearProjectStructureUpdates() @owner.createProject "example-project", {template: "example"}, (error, project_id) => throw error if error? - @example_project_id = project_id + example_project_id = project_id done() it "should version creating a doc", -> - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).docUpdates expect(updates.length).to.equal(2) _.each updates, (update) => expect(update.userId).to.equal(@owner._id) @@ -38,7 +44,7 @@ describe "ProjectStructureChanges", -> expect(_.where(updates, pathname: "/references.bib").length).to.equal 1 it "should version creating a file", -> - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -48,8 +54,9 @@ describe "ProjectStructureChanges", -> describe "duplicating a project", -> before (done) -> MockDocUpdaterApi.clearProjectStructureUpdates() + console.log(example_project_id) @owner.request.post { - uri: "/Project/#{@example_project_id}/clone", + uri: "/Project/#{example_project_id}/clone", json: projectName: 'new.tex' }, (error, res, body) => @@ -80,10 +87,10 @@ describe "ProjectStructureChanges", -> before (done) -> MockDocUpdaterApi.clearProjectStructureUpdates() - ProjectGetter.getProject @example_project_id, (error, project) => + ProjectGetter.getProject example_project_id, (error, project) => throw error if error? @owner.request.post { - uri: "project/#{@example_project_id}/doc", + uri: "project/#{example_project_id}/doc", json: name: 'new.tex' parent_folder_id: project.rootFolder[0]._id @@ -91,11 +98,11 @@ describe "ProjectStructureChanges", -> throw error if error? if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to add doc #{res.statusCode}") - @example_doc_id = body._id + example_doc_id = body._id done() it "should version the doc added", -> - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).docUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -137,7 +144,7 @@ describe "ProjectStructureChanges", -> describe "uploading a file", -> before (done) -> - ProjectGetter.getProject @example_project_id, (error, project) => + ProjectGetter.getProject example_project_id, (error, project) => throw error if error? @root_folder_id = project.rootFolder[0]._id.toString() done() @@ -149,7 +156,7 @@ describe "ProjectStructureChanges", -> image_file = fs.createReadStream(Path.resolve(__dirname + '/../files/1pixel.png')) req = @owner.request.post { - uri: "project/#{@example_project_id}/upload", + uri: "project/#{example_project_id}/upload", qs: folder_id: @root_folder_id formData: @@ -163,9 +170,9 @@ describe "ProjectStructureChanges", -> if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to upload file #{res.statusCode}") - @example_file_id = JSON.parse(body).entity_id + example_file_id = JSON.parse(body).entity_id - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -179,7 +186,7 @@ describe "ProjectStructureChanges", -> image_file = fs.createReadStream(Path.resolve(__dirname + '/../files/2pixel.png')) req = @owner.request.post { - uri: "project/#{@example_project_id}/upload", + uri: "project/#{example_project_id}/upload", qs: folder_id: @root_folder_id formData: @@ -193,7 +200,7 @@ describe "ProjectStructureChanges", -> if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to upload file #{res.statusCode}") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -205,12 +212,12 @@ describe "ProjectStructureChanges", -> describe "moving entities", -> before (done) -> @owner.request.post { - uri: "project/#{@example_project_id}/folder", + uri: "project/#{example_project_id}/folder", formData: name: 'foo' }, (error, res, body) => throw error if error? - @example_folder_id_1 = JSON.parse(body)._id + example_folder_id_1 = JSON.parse(body)._id done() beforeEach () -> @@ -218,15 +225,15 @@ describe "ProjectStructureChanges", -> it "should version moving a doc", (done) -> @owner.request.post { - uri: "project/#{@example_project_id}/Doc/#{@example_doc_id}/move", + uri: "project/#{example_project_id}/Doc/#{example_doc_id}/move", json: - folder_id: @example_folder_id_1 + folder_id: example_folder_id_1 }, (error, res, body) => throw error if error? if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to move doc #{res.statusCode}") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).docUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -237,15 +244,15 @@ describe "ProjectStructureChanges", -> it "should version moving a file", (done) -> @owner.request.post { - uri: "project/#{@example_project_id}/File/#{@example_file_id}/move", + uri: "project/#{example_project_id}/File/#{example_file_id}/move", json: - folder_id: @example_folder_id_1 + folder_id: example_folder_id_1 }, (error, res, body) => throw error if error? if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to move file #{res.statusCode}") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -256,30 +263,30 @@ describe "ProjectStructureChanges", -> it "should version moving a folder", (done) -> @owner.request.post { - uri: "project/#{@example_project_id}/folder", + uri: "project/#{example_project_id}/folder", formData: name: 'bar' }, (error, res, body) => throw error if error? - @example_folder_id_2 = JSON.parse(body)._id + example_folder_id_2 = JSON.parse(body)._id @owner.request.post { - uri: "project/#{@example_project_id}/Folder/#{@example_folder_id_1}/move", + uri: "project/#{example_project_id}/Folder/#{example_folder_id_1}/move", json: - folder_id: @example_folder_id_2 + folder_id: example_folder_id_2 }, (error, res, body) => throw error if error? if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to move folder #{res.statusCode}") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).docUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) expect(update.pathname).to.equal("/foo/new.tex") expect(update.newPathname).to.equal("/bar/foo/new.tex") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -294,7 +301,7 @@ describe "ProjectStructureChanges", -> it "should version renaming a doc", (done) -> @owner.request.post { - uri: "project/#{@example_project_id}/Doc/#{@example_doc_id}/rename", + uri: "project/#{example_project_id}/Doc/#{example_doc_id}/rename", json: name: 'new_renamed.tex' }, (error, res, body) => @@ -302,7 +309,7 @@ describe "ProjectStructureChanges", -> if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to move doc #{res.statusCode}") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).docUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -313,7 +320,7 @@ describe "ProjectStructureChanges", -> it "should version renaming a file", (done) -> @owner.request.post { - uri: "project/#{@example_project_id}/File/#{@example_file_id}/rename", + uri: "project/#{example_project_id}/File/#{example_file_id}/rename", json: name: '1pixel_renamed.png' }, (error, res, body) => @@ -321,7 +328,7 @@ describe "ProjectStructureChanges", -> if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to move file #{res.statusCode}") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -332,7 +339,7 @@ describe "ProjectStructureChanges", -> it "should version renaming a folder", (done) -> @owner.request.post { - uri: "project/#{@example_project_id}/Folder/#{@example_folder_id_1}/rename", + uri: "project/#{example_project_id}/Folder/#{example_folder_id_1}/rename", json: name: 'foo_renamed' }, (error, res, body) => @@ -340,14 +347,14 @@ describe "ProjectStructureChanges", -> if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to move folder #{res.statusCode}") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).docUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) expect(update.pathname).to.equal("/bar/foo/new_renamed.tex") expect(update.newPathname).to.equal("/bar/foo_renamed/new_renamed.tex") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) @@ -362,20 +369,20 @@ describe "ProjectStructureChanges", -> it "should version deleting a folder", (done) -> @owner.request.delete { - uri: "project/#{@example_project_id}/Folder/#{@example_folder_id_2}", + uri: "project/#{example_project_id}/Folder/#{example_folder_id_2}", }, (error, res, body) => throw error if error? if res.statusCode < 200 || res.statusCode >= 300 throw new Error("failed to delete folder #{res.statusCode}") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).docUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).docUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) expect(update.pathname).to.equal("/bar/foo_renamed/new_renamed.tex") expect(update.newPathname).to.equal("") - updates = MockDocUpdaterApi.getProjectStructureUpdates(@example_project_id).fileUpdates + updates = MockDocUpdaterApi.getProjectStructureUpdates(example_project_id).fileUpdates expect(updates.length).to.equal(1) update = updates[0] expect(update.userId).to.equal(@owner._id) From 2a2eb23c78c959ab5f4c3b0a5927eb6ec693a018 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Mon, 26 Feb 2018 16:56:04 +0000 Subject: [PATCH 19/19] Fix bug where tests from new ES code being included in requirejs wrapped code --- services/web/test/unit_frontend/coffee/test-main.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/test/unit_frontend/coffee/test-main.coffee b/services/web/test/unit_frontend/coffee/test-main.coffee index e20acba4d7..c1a511a894 100644 --- a/services/web/test/unit_frontend/coffee/test-main.coffee +++ b/services/web/test/unit_frontend/coffee/test-main.coffee @@ -3,7 +3,7 @@ tests = [] for file of window.__karma__.files if window.__karma__.files.hasOwnProperty(file) - if /Tests\.js$/.test(file) + if /test\/unit_frontend\/js.+Tests.js$/.test(file) tests.push(file) requirejs.config