Upgrade mocha to latest version (5.2.0) (#2703)

Closes #2528

packages/driver/package.json

@@ -49,7 +49,7 @@
     "method-override": "3.0.0",
     "minimatch": "3.0.4",
     "minimist": "1.2.0",
-    "mocha": "cypress-io/mocha#58f6eac05e664fc6b69aa9fba70f1f6b5531a900",
+    "mocha": "5.2.0",
     "moment": "2.24.0",
     "morgan": "1.9.1",
     "npm-install-version": "6.0.2",

packages/driver/src/cypress.coffee

@@ -143,10 +143,6 @@ class $Cypress
     @action("cypress:config", config)
 
   initialize: ($autIframe) ->
-    ## push down the options
-    ## to the runner
-    @mocha.options(@runner)
-
     @cy.initialize($autIframe)
 
   run: (fn) ->

packages/driver/src/cypress/cy.coffee

@@ -1054,9 +1054,15 @@ create = (specWindow, Cypress, Cookies, state, config, log) ->
 
           ## if we're cy or we've enqueued commands
           if isCy(ret) or (queue.length > currentLength)
-            ## the run should already be kicked off
-            ## by now and return this promise
-            return state("promise")
+            if fn.length
+              ## if user has passed done callback
+              ## don't return anything so we don't get an
+              ## 'overspecified' error from mocha
+              return
+            else
+              ## otherwise, return the 'queue promise'
+              ## so mocha awaits it
+              return state("promise")
 
           ## else just return ret
           return ret

packages/driver/src/cypress/error_messages.coffee

@@ -547,6 +547,17 @@ module.exports = {
     async_timed_out: "Timed out after '{{ms}}ms'. The done() callback was never invoked!"
     invalid_interface: "Invalid mocha interface '{{name}}'"
     timed_out: "Cypress command timeout of '{{ms}}ms' exceeded."
+    overspecified: """
+    Cypress detected that you returned a promise in a test, but also invoked a done callback. Return a promise -or- invoke a done callback, not both.
+
+    Read  more here: https://on.cypress.io/returning-promise-and-invoking-done-callback
+
+    #{divider(60, '-')}
+
+    Original mocha error:
+
+    {{error}}
+    """
 
   navigation:
     cross_origin: """

packages/driver/src/cypress/mocha.coffee

@@ -99,6 +99,9 @@ patchRunnerFail = ->
   ## matching the current Runner.prototype.fail except
   ## changing the logic for determing whether this is a valid err
   Runner::fail = (runnable, err) ->
+    if err?.message?.indexOf("Resolution method is overspecified") > -1
+      err.message = $utils.errMessageByPath("mocha.overspecified", { error: err.stack })
+
     ## if this isnt a correct error object then just bail
     ## and call the original function
     if Object.prototype.toString.call(err) isnt "[object Error]"
@@ -189,9 +192,6 @@ create = (specWindow, Cypress, reporter) ->
 
     getRootSuite: ->
       _mocha.suite
-
-    options: (runner) ->
-      runner.options(_mocha.options)
   }
 
 module.exports = {

packages/driver/src/cypress/runner.coffee

@@ -6,7 +6,6 @@ Pending = require("mocha/lib/pending")
 $Log = require("./log")
 $utils = require("./utils")
 
-defaultGrepRe   = /.*/
 mochaCtxKeysRe  = /^(_runnable|test)$/
 betweenQuotesRe = /\"(.+?)\"/
 
@@ -193,7 +192,7 @@ getAllSiblingTests = (suite, getTestById) ->
     ## iterate through each of our suites tests.
     ## this will iterate through all nested tests
     ## as well.  and then we add it only if its
-    ## in our grepp'd tests array
+    ## in our filtered tests array
     if getTestById(test.id)
       tests.push test
 
@@ -212,7 +211,7 @@ getTestFromHook = (hook, suite, getTestById) ->
     return found if found
 
   ## returns us the very first test
-  ## which is in our grepped tests array
+  ## which is in our filtered tests array
   ## based on walking down the current suite
   ## iterating through each test until it matches
   found = onFirstTest suite, (test) =>
@@ -227,11 +226,11 @@ getTestFromHook = (hook, suite, getTestById) ->
 
 ## we have to see if this is the last suite amongst
 ## its siblings.  but first we have to filter out
-## suites which dont have a grep'd test in them
+## suites which dont have a filtered test in them
 isLastSuite = (suite, tests) ->
   return false if suite.root
 
-  ## grab all of the suites from our grep'd tests
+  ## grab all of the suites from our filtered tests
   ## including all of their ancestor suites!
   suites = _.reduce tests, (memo, test) ->
     while parent = test.parent
@@ -296,17 +295,17 @@ overrideRunnerHook = (Cypress, _runner, getTestById, getTest, setTest, getTests)
       when "afterEach"
         t = getTest()
 
-        ## find all of the grep'd _tests which share
+        ## find all of the filtered _tests which share
         ## the same parent suite as our current _test
         tests = getAllSiblingTests(t.parent, getTestById)
 
         ## make sure this test isnt the last test overall but also
-        ## isnt the last test in our grep'd parent suite's tests array
+        ## isnt the last test in our filtered parent suite's tests array
         if @suite.root and (t isnt _.last(allTests)) and (t isnt _.last(tests))
           changeFnToRunAfterHooks()
 
       when "afterAll"
-        ## find all of the grep'd allTests which share
+        ## find all of the filtered allTests which share
         ## the same parent suite as our current _test
         if t = getTest()
           siblings = getAllSiblingTests(t.parent, getTestById)
@@ -327,17 +326,6 @@ overrideRunnerHook = (Cypress, _runner, getTestById, getTest, setTest, getTests)
 
     _runnerHook.call(@, name, fn)
 
-matchesGrep = (runnable, grep) ->
-  ## we have optimized this iteration to the maximum.
-  ## we memoize the existential matchesGrep property
-  ## so we dont regex again needlessly when going
-  ## through tests which have already been set earlier
-  if (not runnable.matchesGrep?) or (not _.isEqual(runnable.grepRe, grep))
-    runnable.grepRe      = grep
-    runnable.matchesGrep = grep.test(runnable.fullTitle())
-
-  runnable.matchesGrep
-
 getTestResults = (tests) ->
   _.map tests, (test) ->
     obj = _.pick(test, "id", "duration", "state")
@@ -347,7 +335,12 @@ getTestResults = (tests) ->
       obj.state = "skipped"
     obj
 
-normalizeAll = (suite, initialTests = {}, grep, setTestsById, setTests, onRunnable, onLogsById, getTestId) ->
+hasOnly = (suite) ->
+  suite._onlyTests.length or
+  suite._onlySuites.length or
+  _.some(suite.suites, hasOnly)
+
+normalizeAll = (suite, initialTests = {}, setTestsById, setTests, onRunnable, onLogsById, getTestId) ->
   hasTests = false
 
   ## only loop until we find the first test
@@ -361,10 +354,8 @@ normalizeAll = (suite, initialTests = {}, grep, setTestsById, setTests, onRunnab
   ## we hand back a normalized object but also
   ## create optimized lookups for the tests without
   ## traversing through it multiple times
-  tests         = {}
-  grepIsDefault = _.isEqual(grep, defaultGrepRe)
-
-  obj = normalize(suite, tests, initialTests, grep, grepIsDefault, onRunnable, onLogsById, getTestId)
+  tests = {}
+  normalizedSuite = normalize(suite, tests, initialTests, onRunnable, onLogsById, getTestId)
 
   if setTestsById
     ## use callback here to hand back
@@ -375,10 +366,10 @@ normalizeAll = (suite, initialTests = {}, grep, setTestsById, setTests, onRunnab
     ## same pattern here
     setTests(_.values(tests))
 
-  return obj
+  return normalizedSuite
 
-normalize = (runnable, tests, initialTests, grep, grepIsDefault, onRunnable, onLogsById, getTestId) ->
-  normalizer = (runnable) =>
+normalize = (runnable, tests, initialTests, onRunnable, onLogsById, getTestId) ->
+  normalizeRunnable = (runnable) =>
     runnable.id = getTestId()
 
     ## tests have a type of 'test' whereas suites do not have a type property
@@ -402,57 +393,50 @@ normalize = (runnable, tests, initialTests, grep, grepIsDefault, onRunnable, onL
   push = (test) =>
     tests[test.id] ?= test
 
-  obj = normalizer(runnable)
+  normalizedRunnable = normalizeRunnable(runnable)
 
-  ## if we have a default grep then avoid
-  ## grepping altogether and just push
-  ## tests into the array of tests
-  if grepIsDefault
+  if runnable.type isnt "suite" or not hasOnly(runnable)
     if runnable.type is "test"
       push(runnable)
 
-    ## and recursively iterate and normalize all other _runnables
-    _.each {tests: runnable.tests, suites: runnable.suites}, (_runnables, key) =>
-      if runnable[key]
-        obj[key] = _.map _runnables, (runnable) =>
-          normalize(runnable, tests, initialTests, grep, grepIsDefault, onRunnable, onLogsById, getTestId)
-  else
-    ## iterate through all tests and only push them in
-    ## if they match the current grep
-    obj.tests = _.reduce runnable.tests ? [], (memo, test) =>
-      ## only push in the test if it matches
-      ## our grep
-      if matchesGrep(test, grep)
-        memo.push(normalizer(test))
-        push(test)
-      memo
-    , []
-
-    ## and go through the suites
-    obj.suites = _.reduce runnable.suites ? [], (memo, suite) =>
-      ## but only add them if a single nested test
-      ## actually matches the grep
-      any = anyTestInSuite suite, (test) =>
-        matchesGrep(test, grep)
-
-      if any
-        memo.push(
-          normalize(
-            suite,
-            tests,
-            initialTests,
-            grep,
-            grepIsDefault,
-            onRunnable,
-            onLogsById,
-            getTestId
-          )
-        )
-
-      memo
-    , []
-
-  return obj
+    ## recursively iterate and normalize all other _runnables
+    _.each {tests: runnable.tests, suites: runnable.suites}, (_runnables, type) =>
+      if runnable[type]
+        normalizedRunnable[type] = _.map _runnables, (runnable) =>
+          normalize(runnable, tests, initialTests, onRunnable, onLogsById, getTestId)
+
+    return normalizedRunnable
+
+  ## this follows how mocha filters onlys. its runner#filterOnly
+  ## is pretty much the same minus the normalization part
+  filterOnly = (normalizedSuite, suite) ->
+    if suite._onlyTests.length
+      suite.tests = suite._onlyTests
+      normalizedSuite.tests = _.map suite._onlyTests, (test) =>
+        normalizedTest = normalizeRunnable(test, initialTests, onRunnable, onLogsById, getTestId)
+        push(normalizedTest)
+        normalizedTest
+      suite.suites = []
+      normalizedSuite.suites = []
+    else
+      suite.tests = []
+      normalizedSuite.tests = []
+      _.each suite._onlySuites, (onlySuite) ->
+        normalizedOnlySuite = normalizeRunnable(onlySuite, initialTests, onRunnable, onLogsById, getTestId)
+        if hasOnly(onlySuite)
+          filterOnly(normalizedOnlySuite, onlySuite)
+
+      suite.suites = _.filter suite.suites, (childSuite) ->
+        normalizedChildSuite = normalizeRunnable(childSuite, initialTests, onRunnable, onLogsById, getTestId)
+        suite._onlySuites.indexOf(childSuite) isnt -1 or filterOnly(normalizedChildSuite, childSuite)
+      normalizedSuite.suites = _.map suite.suites, (childSuite) ->
+        normalize(childSuite, tests, initialTests, onRunnable, onLogsById, getTestId)
+
+    return suite.tests.length or suite.suites.length
+
+  filterOnly(normalizedRunnable, runnable)
+
+  return normalizedRunnable
 
 afterEachFailed = (Cypress, test, err) ->
   test.state = "failed"
@@ -717,23 +701,6 @@ create = (specWindow, mocha, Cypress, cy) ->
   overrideRunnerHook(Cypress, _runner, getTestById, getTest, setTest, getTests)
 
   return {
-    grep: (re) ->
-      if arguments.length
-        _runner._grep = re
-      else
-        ## grab grep from the mocha _runner
-        ## or just set it to all in case
-        ## there is a mocha regression
-        _runner._grep ?= defaultGrepRe
-
-    options: (options = {}) ->
-      ## TODO
-      ## need to handle
-      ## ignoreLeaks, asyncOnly, globals
-
-      if re = options.grep
-        @grep(re)
-
     normalizeAll: (tests) ->
       ## if we have an uncaught error then slice out
       ## all of the tests and suites and just generate
@@ -750,7 +717,6 @@ create = (specWindow, mocha, Cypress, cy) ->
       normalizeAll(
         _runner.suite,
         tests,
-        @grep(),
         setTestsById,
         setTests,
         onRunnable,

packages/driver/test/cypress/integration/cypress/cypress_spec.coffee

@@ -29,7 +29,7 @@ describe "driver/src/cypress/index", ->
         expect($el.get(0)).to.eq($foo.get(0))
 
   context "#backend", ->
-    it "sets __stackCleaned__ on errors", (done) ->
+    it "sets __stackCleaned__ on errors", ->
       cy.stub(@Cypress, "emit")
       .withArgs("backend:request")
       .yieldsAsync({
@@ -75,4 +75,4 @@ describe "driver/src/cypress/index", ->
       fn = ->
         Cypress.log({ message: 'My Log' })
 
-      expect(fn).to.not.throw()
+      expect(fn).to.not.throw()

packages/driver/test/cypress/integration/e2e/promises_spec.coffee

@@ -106,7 +106,7 @@ describe "promises", ->
 
     cy.foo()
 
-  it "can return a promise that throws on its own without warning", (done) ->
+  it "can return a promise that throws on its own without warning", ->
     Cypress.Promise
     .delay(10)
     .then ->
@@ -116,7 +116,6 @@ describe "promises", ->
 
       throw new Error("foo")
     .catch ->
-      done()
 
   it "can still fail cypress commands", (done) ->
     cy.on "fail", (err) ->
@@ -128,3 +127,4 @@ describe "promises", ->
     .then ->
       cy.wrap({}).then ->
         throw new Error("foo")
+    return

packages/driver/test/cypress/integration/e2e/return_value_spec.coffee

@@ -16,6 +16,10 @@ describe "return values", ->
 
     return undefined
 
+  it "can return cy and have done callback", (done) ->
+    cy.wrap({}).then ->
+      done()
+
   it "throws when returning a non promise and invoking cy commands", (done) ->
     cy.on "fail", (err) ->
       expect(err.message).to.include("> foo")
@@ -91,3 +95,23 @@ describe "return values", ->
         return "bar"
 
     cy.foo()
+
+  describe "without invoking cy", ->
+    it "handles returning undefined", ->
+      return undefined
+
+    it "handles synchronously invoking and returning done callback", (done) ->
+      return done()
+
+    it "handles synchronously invoking done callback and returning undefined", (done) ->
+      done()
+      return undefined
+
+    it "handles synchronously invoking done callback and returning a value", (done) ->
+      done()
+      return "foo"
+
+    it "handles asynchronously invoking done callback", (done) ->
+      setTimeout ->
+        done()
+      return "foo"