From da7250c6f2b3f1bc9fb60ab03dfcee92a970c018 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 20 May 2015 11:44:54 +0100 Subject: [PATCH 1/6] disable logger in tests --- services/spelling/test/unit/coffee/ASpellTests.coffee | 5 ++++- .../test/unit/coffee/LearnedWordsManagerTests.coffee | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/services/spelling/test/unit/coffee/ASpellTests.coffee b/services/spelling/test/unit/coffee/ASpellTests.coffee index 1170f6baa1..a569282982 100644 --- a/services/spelling/test/unit/coffee/ASpellTests.coffee +++ b/services/spelling/test/unit/coffee/ASpellTests.coffee @@ -6,7 +6,10 @@ assert = require("chai").assert describe "ASpell", -> beforeEach -> - @ASpell = SandboxedModule.require "../../../app/js/ASpell", requires:{} + @ASpell = SandboxedModule.require "../../../app/js/ASpell", requires: + "logger-sharelatex": + log:-> + err:-> describe "a correctly spelled word", -> beforeEach (done) -> diff --git a/services/spelling/test/unit/coffee/LearnedWordsManagerTests.coffee b/services/spelling/test/unit/coffee/LearnedWordsManagerTests.coffee index 984beb37b8..6cbe6af9c9 100644 --- a/services/spelling/test/unit/coffee/LearnedWordsManagerTests.coffee +++ b/services/spelling/test/unit/coffee/LearnedWordsManagerTests.coffee @@ -19,6 +19,10 @@ describe "LearnedWordsManager", -> @LearnedWordsManager = SandboxedModule.require modulePath, requires: "./DB" : @db "./MongoCache":@cache + "logger-sharelatex": + log:-> + err:-> + info:-> describe "learnWord", -> beforeEach -> From 6e0d72a3b188676a6b8727c6fe7b234457bf0a24 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 20 May 2015 11:45:17 +0100 Subject: [PATCH 2/6] timeout does not need async test here --- services/spelling/test/unit/coffee/ASpellTests.coffee | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/spelling/test/unit/coffee/ASpellTests.coffee b/services/spelling/test/unit/coffee/ASpellTests.coffee index a569282982..c7426c74cf 100644 --- a/services/spelling/test/unit/coffee/ASpellTests.coffee +++ b/services/spelling/test/unit/coffee/ASpellTests.coffee @@ -61,7 +61,6 @@ describe "ASpell", -> # Note that this test fails on OS X, due to differing pipe behaviour # on killing the child process. It can be tested successfully on Travis # or the CI server. - it "should return in reasonable time", (done) -> + it "should return in reasonable time", () -> delta = Date.now()-@start delta.should.be.below(@ASpell.ASPELL_TIMEOUT + 100) - done() From 6a241cba28fbc2a18272bc4d0e128bdedada3305 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 20 May 2015 11:45:32 +0100 Subject: [PATCH 3/6] add grep option to grunt test:unit --- services/spelling/Gruntfile.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/spelling/Gruntfile.coffee b/services/spelling/Gruntfile.coffee index 9bf2f98681..2b1e797db5 100644 --- a/services/spelling/Gruntfile.coffee +++ b/services/spelling/Gruntfile.coffee @@ -31,6 +31,7 @@ module.exports = (grunt) -> unit: options: reporter: grunt.option('reporter') or 'spec' + grep: grunt.option("grep") src: ["test/unit/js/**/*.js"] grunt.loadNpmTasks 'grunt-contrib-coffee' From f70476284ba80aab362457aa4c49992759a1499f Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 20 May 2015 11:53:28 +0100 Subject: [PATCH 4/6] move callback _.once down to Worker, add handler for process close event should fix timeout errors in tests --- services/spelling/app/coffee/ASpell.coffee | 2 -- services/spelling/app/coffee/ASpellWorker.coffee | 15 ++++++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/services/spelling/app/coffee/ASpell.coffee b/services/spelling/app/coffee/ASpell.coffee index 92437ebecf..392b543706 100644 --- a/services/spelling/app/coffee/ASpell.coffee +++ b/services/spelling/app/coffee/ASpell.coffee @@ -1,5 +1,4 @@ async = require "async" -_ = require "underscore" ASpellWorkerPool = require "./ASpellWorkerPool" LRU = require "lru-cache" logger = require 'logger-sharelatex' @@ -87,7 +86,6 @@ module.exports = ASpell = # http://aspell.net/man-html/Through-A-Pipe.html checkWords: (language, words, callback = (error, result) ->) -> runner = new ASpellRunner() - callback = _.once callback runner.checkWords language, words, callback ASPELL_TIMEOUT : 4000 diff --git a/services/spelling/app/coffee/ASpellWorker.coffee b/services/spelling/app/coffee/ASpellWorker.coffee index 168be52ee6..05e4a93adb 100644 --- a/services/spelling/app/coffee/ASpellWorker.coffee +++ b/services/spelling/app/coffee/ASpellWorker.coffee @@ -1,6 +1,7 @@ child_process = require("child_process") logger = require 'logger-sharelatex' metrics = require('metrics-sharelatex') +_ = require "underscore" BATCH_SIZE = 100 @@ -15,16 +16,24 @@ class ASpellWorker @state = 'killed' logger.log process: @pipe.pid, lang: @language, "aspell worker has exited" metrics.inc "aspellWorker-exit-" + @language + @pipe.on 'close', () => + @state = 'closed' unless @state == 'killed' + if @callback? + logger.err process: @pipe.pid, lang: @language, "aspell worker closed output streams with uncalled callback" + @callback new Error("aspell worker closed output streams with uncalled callback"), [] + @callback = null @pipe.on 'error', (err) => @state = 'error' unless @state == 'killed' logger.log process: @pipe.pid, error: err, stdout: output.slice(-1024), stderr: error.slice(-1024), lang: @language, "aspell worker error" if @callback? @callback err, [] + @callback = null @pipe.stdin.on 'error', (err) => @state = 'error' unless @state == 'killed' logger.log process: @pipe.pid, error: err, stdout: output.slice(-1024), stderr: error.slice(-1024), lang: @language, "aspell worker error on stdin" if @callback? @callback err, [] + @callback = null output = "" endMarker = new RegExp("^[a-z][a-z]", "m") @@ -33,6 +42,7 @@ class ASpellWorker # We receive the language code from Aspell as the end of data marker if chunk.toString().match(endMarker) @callback(null, output.slice()) + @callback = null # only allow one callback in use @state = 'ready' output = "" error = "" @@ -52,7 +62,10 @@ class ASpellWorker # we will now send data to aspell, and be ready again when we # receive the end of data marker @state = 'busy' - @callback = callback + if @callback? # only allow one callback in use + logger.err process: @pipe.pid, lang: @language, "CALLBACK ALREADY IN USE" + return @callback new Error("Aspell callback already in use - SHOULD NOT HAPPEN") + @callback = _.once callback # extra defence against double callback @setTerseMode() @write(words) @flush() From 81fb8de1db686dbe9d70513e0e26550edee696a6 Mon Sep 17 00:00:00 2001 From: Brian Date: Wed, 20 May 2015 20:41:51 +0100 Subject: [PATCH 5/6] allow mocha timeout to be specified on grunt command line --- services/spelling/Gruntfile.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/spelling/Gruntfile.coffee b/services/spelling/Gruntfile.coffee index 2b1e797db5..038057c3ad 100644 --- a/services/spelling/Gruntfile.coffee +++ b/services/spelling/Gruntfile.coffee @@ -32,6 +32,7 @@ module.exports = (grunt) -> options: reporter: grunt.option('reporter') or 'spec' grep: grunt.option("grep") + timeout: grunt.option("timeout") src: ["test/unit/js/**/*.js"] grunt.loadNpmTasks 'grunt-contrib-coffee' From 9a41def9214b51f39e4cc1add9fa11dfc865ae68 Mon Sep 17 00:00:00 2001 From: Brian Date: Wed, 20 May 2015 20:42:49 +0100 Subject: [PATCH 6/6] increase time allowed for exiting ASpellWorker test from 100ms to 1s --- services/spelling/test/unit/coffee/ASpellTests.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/spelling/test/unit/coffee/ASpellTests.coffee b/services/spelling/test/unit/coffee/ASpellTests.coffee index c7426c74cf..742a5aadbb 100644 --- a/services/spelling/test/unit/coffee/ASpellTests.coffee +++ b/services/spelling/test/unit/coffee/ASpellTests.coffee @@ -63,4 +63,4 @@ describe "ASpell", -> # or the CI server. it "should return in reasonable time", () -> delta = Date.now()-@start - delta.should.be.below(@ASpell.ASPELL_TIMEOUT + 100) + delta.should.be.below(@ASpell.ASPELL_TIMEOUT + 1000)