From 0c7ae12772f7d284e83d38d41d4fe0c31c0ff090 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 31 Aug 2017 23:08:10 -0700 Subject: [PATCH 1/7] =?UTF-8?q?Get=20`coffee`=20command=20working=20again?= =?UTF-8?q?=20in=20Node=206,=20by=20converting=20the=20=E2=80=98await?= =?UTF-8?q?=E2=80=99=20wrapper=20in=20the=20REPL=20to=20use=20a=20Promise?= =?UTF-8?q?=20instead=20of=20the=20=E2=80=98await=E2=80=99=20keyword;=20ad?= =?UTF-8?q?d=20tests=20for=20REPL=20=E2=80=98await=E2=80=99=20wrapper,=20i?= =?UTF-8?q?ncluding=20test=20to=20skip=20async=20tests=20if=20the=20runtim?= =?UTF-8?q?e=20doesn=E2=80=99t=20support=20them?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Cakefile | 13 +++++++++++++ lib/coffeescript/repl.js | 11 ++++++----- src/repl.coffee | 6 +++--- test/repl.coffee | 7 +++++++ 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/Cakefile b/Cakefile index 0307b6bf0f..f492498ff7 100644 --- a/Cakefile +++ b/Cakefile @@ -409,6 +409,17 @@ runTests = (CoffeeScript) -> description: description if description? source: fn.toString() if fn.toString? + global.supportsAsync = if global.testingBrowser + try + new Function('async () => {}')() + yes + catch exception + no + else + [major, minor, build] = process.versions.node.split('.').map (version) -> + parseInt version, 10 + major >= 8 or (major is 7 and minor >= 6) + helpers.extend global, require './test/support/helpers' # When all the tests have run, collect and print errors. @@ -428,6 +439,8 @@ runTests = (CoffeeScript) -> # Run every test in the `test` folder, recording failures. files = fs.readdirSync 'test' + unless global.supportsAsync # Except for async tests, if async isn’t supported. + files = files.filter (filename) -> filename isnt 'async.coffee' for file in files when helpers.isCoffee file literate = helpers.isLiterate file diff --git a/lib/coffeescript/repl.js b/lib/coffeescript/repl.js index 93019d6b7b..8b16d09b59 100644 --- a/lib/coffeescript/repl.js +++ b/lib/coffeescript/repl.js @@ -26,7 +26,7 @@ } })(), historyMaxInputSize: 10240, - eval: async function(input, context, filename, cb) { + eval: function(input, context, filename, cb) { var Assign, Block, Call, Code, Literal, Value, ast, err, isAsync, js, referencedVars, result, token, tokens; // XXX: multiline hack. input = input.replace(/\uFF00/g, '\n'); @@ -71,10 +71,11 @@ result = runInContext(js, context, filename); // Await an async result, if necessary if (isAsync) { - result = (await result); - if (!sawSIGINT) { - cb(null, result); - } + result.then(function(resolvedResult) { + if (!sawSIGINT) { + return cb(null, resolvedResult); + } + }); return sawSIGINT = false; } else { return cb(null, result); diff --git a/src/repl.coffee b/src/repl.coffee index 7ac2cc1ac3..3193a3e3cd 100644 --- a/src/repl.coffee +++ b/src/repl.coffee @@ -44,9 +44,9 @@ replDefaults = result = runInContext js, context, filename # Await an async result, if necessary if isAsync - result = await result - cb null, result unless sawSIGINT - sawSIGINT = false + result.then (resolvedResult) -> + cb null, resolvedResult unless sawSIGINT + sawSIGINT = no else cb null, result catch err diff --git a/test/repl.coffee b/test/repl.coffee index c0ce1a121f..f1a6091537 100644 --- a/test/repl.coffee +++ b/test/repl.coffee @@ -115,6 +115,13 @@ testRepl "keeps running after runtime error", (input, output) -> input.emitLine 'a' eq 'undefined', output.lastWrite() +testRepl "#4604: wraps an async function", (input, output) -> + return unless global.supportsAsync + input.emitLine 'await new Promise (resolve) -> setTimeout (-> resolve 33), 10' + setTimeout -> + eq '33', output.lastWrite() + , 20 + process.on 'exit', -> try fs.unlinkSync historyFile From be208a5a67423f1befbf1f91901c763c0af3d20f Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 1 Sep 2017 00:37:19 -0700 Subject: [PATCH 2/7] Fix async tests: now if a test function is a Promise (which an `await` function is), we add it to an array of async test functions and wait for them all to resolve before finishing the test run, so that if any async tests fail we see those failures in the output --- Cakefile | 45 ++++++++++++++++++++++++++------------ documentation/test.html | 42 +++++++++++++++++++++++------------ test/async.coffee | 33 +++++++++------------------- test/error_messages.coffee | 9 ++++++++ 4 files changed, 78 insertions(+), 51 deletions(-) diff --git a/Cakefile b/Cakefile index f492498ff7..0c6e90eb81 100644 --- a/Cakefile +++ b/Cakefile @@ -376,7 +376,6 @@ task 'bench', 'quick benchmark of compilation time', -> # Run the CoffeeScript test suite. runTests = (CoffeeScript) -> CoffeeScript.register() unless global.testingBrowser - startTime = Date.now() # These are attached to `global` so that they’re accessible from within # `test/async.coffee`, which has an async-capable version of @@ -396,18 +395,29 @@ runTests = (CoffeeScript) -> global.yellow = yellow global.reset = reset + asyncTests = [] + onFail = (description, fn, err) -> + failures.push + filename: global.currentFile + error: err + description: description + source: fn.toString() if fn.toString? + # Our test helper function for delimiting different test cases. global.test = (description, fn) -> try fn.test = {description, currentFile} - fn.call(fn) - ++passedTests - catch e - failures.push - filename: currentFile - error: e - description: description if description? - source: fn.toString() if fn.toString? + result = fn.call(fn) + if result instanceof Promise # An async test. + asyncTests.push result + result.then -> + passedTests++ + .catch (err) -> + onFail description, fn, err + else + passedTests++ + catch err + onFail description, fn, err global.supportsAsync = if global.testingBrowser try @@ -442,6 +452,7 @@ runTests = (CoffeeScript) -> unless global.supportsAsync # Except for async tests, if async isn’t supported. files = files.filter (filename) -> filename isnt 'async.coffee' + startTime = Date.now() for file in files when helpers.isCoffee file literate = helpers.isLiterate file currentFile = filename = path.join 'test', file @@ -450,12 +461,17 @@ runTests = (CoffeeScript) -> CoffeeScript.run code.toString(), {filename, literate} catch error failures.push {filename, error} - return !failures.length + + asyncTests.push -> + new Promise (resolve) -> resolve failures.length is 0 + Promise.all asyncTests task 'test', 'run the CoffeeScript language test suite', -> - testResults = runTests CoffeeScript - process.exit 1 unless testResults + runTests(CoffeeScript).then -> + # All the async tests passed, but did all the non-async ones? + process.exit 1 unless failures.length is 0 + .catch -> process.exit 1 # At least one async test failed. task 'test:browser', 'run the test suite against the merged browser script', -> @@ -463,8 +479,9 @@ task 'test:browser', 'run the test suite against the merged browser script', -> result = {} global.testingBrowser = yes (-> eval source).call result - testResults = runTests result.CoffeeScript - process.exit 1 unless testResults + runTests(CoffeeScript).then -> + process.exit 1 unless failures.length is 0 + .catch -> process.exit 1 task 'test:integrations', 'test the module integrated with other libraries and environments', -> # Tools like Webpack and Browserify generate builds intended for a browser diff --git a/documentation/test.html b/documentation/test.html index 3254e97574..dc2ad5abef 100644 --- a/documentation/test.html +++ b/documentation/test.html @@ -52,16 +52,27 @@

CoffeeScript Test Suite

stdout.appendChild div msg +asyncTests = [] +onFail = (description, fn, err) -> + failures.push + error: err + description: description + source: fn.toString() if fn.toString? + @test = (description, fn) -> ++total try - fn.call(fn) - ++passedTests - catch error - failures.push - error: error - description: description - source: fn.toString() if fn.toString? + result = fn.call(fn) + if result instanceof Promise # An async test. + asyncTests.push result + result.then -> + passedTests++ + .catch (err) -> + onFail description, fn, err + else + passedTests++ + catch err + onFail description, fn, err @failures = push: (failure) -> # Match function called by regular tests @@ -74,11 +85,11 @@

CoffeeScript Test Suite

@ok = (good, msg = 'Error') -> throw Error msg unless good -# Polyfill Node assert's fail +# Polyfill Node assert’s fail @fail = -> ok no -# Polyfill Node assert's deepEqual with Underscore's isEqual +# Polyfill Node assert’s deepEqual with Underscore’s isEqual @deepEqual = (a, b) -> ok _.isEqual(a, b), "Expected #{JSON.stringify a} to deep equal #{JSON.stringify b}" @@ -114,11 +125,14 @@

CoffeeScript Test Suite

CoffeeScript.run test.innerHTML, options # Finish up -yay = passedTests is total and not failedTests -sec = (new Date - start) / 1000 -msg = "passed #{passedTests} tests in #{sec.toFixed 2} seconds" -msg = "failed #{total - passedTests} tests and #{msg}" unless yay -say msg, (if yay then 'good' else 'bad') +done = -> + yay = passedTests is total and not failedTests + sec = (new Date - start) / 1000 + msg = "passed #{passedTests} tests in #{sec.toFixed 2} seconds" + msg = "failed #{total - passedTests} tests and #{msg}" unless yay + say msg, (if yay then 'good' else 'bad') + +Promise.all(asyncTests).then(done).catch(done) <%= tests %> diff --git a/test/async.coffee b/test/async.coffee index cb71d2f1bd..be1994c589 100644 --- a/test/async.coffee +++ b/test/async.coffee @@ -1,28 +1,15 @@ -# Functions that contain the `await` keyword will compile into async -# functions, which currently only Node 7+ in harmony mode can even -# evaluate, much less run. Therefore we need to prevent runtimes -# which will choke on such code from even loading it. This file is -# only loaded by async-capable environments, so we redefine `test` -# here even though it is based on `test` defined in `Cakefile`. -# It replaces `test` for this file, and adds to the tracked -# `passedTests` and `failures` arrays which are global objects. -test = (description, fn) -> - try - fn.test = {description, currentFile} - await fn.call(fn) - ++passedTests - catch e - failures.push - filename: currentFile - error: e - description: description if description? - source: fn.toString() if fn.toString? - - -# always fulfills +# Functions that contain the `await` keyword will compile into async functions, +# supported by Node 7.6+, Chrome 55+, Firefox 52+, Safari 10.1+ and Edge. +# But runtimes that don’t support the `await` keyword will throw an error, +# even if we put `return unless global.supportsAsync` at the top of this file. +# Therefore we need to prevent runtimes which will choke on such code from even +# parsing it, which is handled in `Cakefile`. + + +# This is always fulfilled. winning = (val) -> Promise.resolve val -# always is rejected +# This is always rejected. failing = (val) -> Promise.reject new Error val diff --git a/test/error_messages.coffee b/test/error_messages.coffee index d5aeda6081..ec9145d444 100644 --- a/test/error_messages.coffee +++ b/test/error_messages.coffee @@ -91,6 +91,9 @@ if require? notEqual error.stack.toString().indexOf(filePath), -1 test "#4418: stack traces for compiled files reference the correct line number", -> + # The browser is already compiling other anonymous scripts (the tests) + # which will conflict. + return if global.testingBrowser filePath = path.join os.tmpdir(), 'StackTraceLineNumberTestFile.coffee' fileContents = """ testCompiledFileStackTraceLineNumber = -> @@ -112,6 +115,9 @@ if require? test "#4418: stack traces for compiled strings reference the correct line number", -> + # The browser is already compiling other anonymous scripts (the tests) + # which will conflict. + return if global.testingBrowser try CoffeeScript.run ''' testCompiledStringStackTraceLineNumber = -> @@ -128,6 +134,9 @@ test "#4418: stack traces for compiled strings reference the correct line number test "#4558: compiling a string inside a script doesn’t screw up stack trace line number", -> + # The browser is already compiling other anonymous scripts (the tests) + # which will conflict. + return if global.testingBrowser try CoffeeScript.run ''' testCompilingInsideAScriptDoesntScrewUpStackTraceLineNumber = -> From f87cc67dc1310f43d7a59c424ba881bd407f13ba Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 1 Sep 2017 00:36:45 -0700 Subject: [PATCH 3/7] Code review --- Cakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cakefile b/Cakefile index 0c6e90eb81..8c2e64f614 100644 --- a/Cakefile +++ b/Cakefile @@ -423,7 +423,7 @@ runTests = (CoffeeScript) -> try new Function('async () => {}')() yes - catch exception + catch no else [major, minor, build] = process.versions.node.split('.').map (version) -> From f309e1718393e8b70136fa1e198d1e2438e087a8 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 1 Sep 2017 00:50:20 -0700 Subject: [PATCH 4/7] Unnecessary --- Cakefile | 2 -- 1 file changed, 2 deletions(-) diff --git a/Cakefile b/Cakefile index 8c2e64f614..ad5831af8a 100644 --- a/Cakefile +++ b/Cakefile @@ -462,8 +462,6 @@ runTests = (CoffeeScript) -> catch error failures.push {filename, error} - asyncTests.push -> - new Promise (resolve) -> resolve failures.length is 0 Promise.all asyncTests From a8c889ebecdcc92ae5195fe7183135f070f80666 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 1 Sep 2017 09:29:24 -0700 Subject: [PATCH 5/7] Let's support Node 6+ if we can --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 16120fafa5..a6212a0d39 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "version": "2.0.0-beta4", "license": "MIT", "engines": { - "node": ">=7.6.0" + "node": ">=6" }, "directories": { "lib": "./lib/coffeescript" From 143c81b719f7702aedb4718001e3a3e62d81bd96 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 1 Sep 2017 09:49:10 -0700 Subject: [PATCH 6/7] Simplify the returned promise --- Cakefile | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Cakefile b/Cakefile index ad5831af8a..818d463a6b 100644 --- a/Cakefile +++ b/Cakefile @@ -462,14 +462,12 @@ runTests = (CoffeeScript) -> catch error failures.push {filename, error} - Promise.all asyncTests + Promise.all(asyncTests).then -> + Promise.reject() if failures.length isnt 0 task 'test', 'run the CoffeeScript language test suite', -> - runTests(CoffeeScript).then -> - # All the async tests passed, but did all the non-async ones? - process.exit 1 unless failures.length is 0 - .catch -> process.exit 1 # At least one async test failed. + runTests(CoffeeScript).catch -> process.exit 1 task 'test:browser', 'run the test suite against the merged browser script', -> @@ -477,9 +475,8 @@ task 'test:browser', 'run the test suite against the merged browser script', -> result = {} global.testingBrowser = yes (-> eval source).call result - runTests(CoffeeScript).then -> - process.exit 1 unless failures.length is 0 - .catch -> process.exit 1 + runTests(CoffeeScript).catch -> process.exit 1 + task 'test:integrations', 'test the module integrated with other libraries and environments', -> # Tools like Webpack and Browserify generate builds intended for a browser From b85b9d6caba95ec8d83db8e276a20b0fa6ec9bee Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 1 Sep 2017 12:24:35 -0700 Subject: [PATCH 7/7] Simplify async check --- Cakefile | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/Cakefile b/Cakefile index f4e9faf1a0..a43239a5f7 100644 --- a/Cakefile +++ b/Cakefile @@ -419,27 +419,11 @@ runTests = (CoffeeScript) -> catch err onFail description, fn, err - global.supportsAsync = if global.testingBrowser - try - new Function('async () => {}')() - yes - catch - no - else - [major, minor, build] = process.versions.node.split('.').map (version) -> - parseInt version, 10 - major >= 8 or (major is 7 and minor >= 6) - - global.supportsAsync = if global.testingBrowser - try + global.supportsAsync = try new Function('async () => {}')() yes catch no - else - [major, minor, build] = process.versions.node.split('.').map (version) -> - parseInt version, 10 - major >= 8 or (major is 7 and minor >= 6) helpers.extend global, require './test/support/helpers'