[CS2] Fix async tests (#4680)

* Get `coffee` command working again in Node 6, by converting the ‘await’ wrapper in the REPL to use a Promise instead of the ‘await’ keyword; add tests for REPL ‘await’ wrapper, including test to skip async tests if the runtime doesn’t support them

* 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

* Code review

* Unnecessary

* Let's support Node 6+ if we can

* Simplify the returned promise

* Simplify async check

Author: GeoffreyBooth

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,29 +395,35 @@ 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?
-
-  global.supportsAsync = if global.testingBrowser
-    try
+      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 = 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'
 
@@ -442,6 +447,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,21 +456,22 @@ runTests = (CoffeeScript) ->
       CoffeeScript.run code.toString(), {filename, literate}
     catch error
       failures.push {filename, error}
-  return !failures.length
+
+  Promise.all(asyncTests).then ->
+    Promise.reject() if failures.length isnt 0
 
 
 task 'test', 'run the CoffeeScript language test suite', ->
-  testResults = runTests CoffeeScript
-  process.exit 1 unless testResults
+  runTests(CoffeeScript).catch -> process.exit 1
 
 
 task 'test:browser', 'run the test suite against the merged browser script', ->
   source = fs.readFileSync "docs/v#{majorVersion}/browser-compiler/coffeescript.js", 'utf-8'
   result = {}
   global.testingBrowser = yes
   (-> eval source).call result
-  testResults = runTests result.CoffeeScript
-  process.exit 1 unless testResults
+  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

documentation/test.html

@@ -52,16 +52,27 @@ <h1>CoffeeScript Test Suite</h1>
   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 @@ <h1>CoffeeScript Test Suite</h1>
 @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 @@ <h1>CoffeeScript Test Suite</h1>
   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)
 </script>
 
 <%= tests %>

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
 
 

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 = ->