fix rerun before/after hooks on top navigation (#7035)

* cleanup code, add better e2e tests

* fix before/after hooks incorrect test reference

* cleanup, add test for root after hook and test:after:run event

* fix typo

* add link to issue

Co-authored-by: Jennifer Shehane <jennifer@cypress.io>

packages/driver/src/cypress/runner.js

@@ -251,6 +251,18 @@ const getTestFromHook = function (hook, suite, getTestById) {
     return test
   }
 
+  if (hook.hookName === 'after all') {
+    const siblings = getAllSiblingTests(suite, getTestById)
+
+    return _.last(siblings)
+  }
+
+  if (hook.hookName === 'before all') {
+    const siblings = getAllSiblingTests(suite, getTestById)
+
+    return _.first(siblings)
+  }
+
   // if we have a hook id then attempt
   // to find the test by its id
   if (hook != null ? hook.id : undefined) {
@@ -394,7 +406,7 @@ const overrideRunnerHook = function (Cypress, _runner, getTestById, getTest, set
           //    the last test that will run
           if (
             (isRootSuite(this.suite) && isLastTest(t, allTests)) ||
-              (isRootSuite(this.suite.parent) && lastTestThatWillRunInSuite(t, siblings)) ||
+              (isRootSuite(this.suite.parent) && !this.suite.parent._afterAll.length && lastTestThatWillRunInSuite(t, siblings)) ||
               (!isLastSuite(this.suite, allTests) && lastTestThatWillRunInSuite(t, siblings))
           ) {
             changeFnToRunAfterHooks()
@@ -577,7 +589,7 @@ const hookFailed = function (hook, err, hookName, getTestById, getTest) {
   // finds the test by returning the first test from
   // the parent or looping through the suites until
   // it finds the first test
-  const test = getTest() || getTestFromHook(hook, hook.parent, getTestById)
+  const test = getTestFromHook(hook, hook.parent, getTestById)
 
   test.err = err
   test.state = 'failed'
@@ -642,15 +654,15 @@ const _runnerListeners = function (_runner, Cypress, _emissions, getTestById, ge
     // if there is a nested suite with a before, then
     // currentTest will refer to the previous test run
     // and not our current
-    if ((hook.hookName === 'before all') && hook.ctx.currentTest) {
+    if ((hook.hookName === 'before all' || hook.hookName === 'after all') && hook.ctx.currentTest) {
       delete hook.ctx.currentTest
     }
 
     // set the hook's id from the test because
     // hooks do not have their own id, their
     // commands need to grouped with the test
     // and we can only associate them by this id
-    const test = getTest() || getTestFromHook(hook, hook.parent, getTestById)
+    const test = getTestFromHook(hook, hook.parent, getTestById)
 
     hook.id = test.id
     hook.ctx.currentTest = test
@@ -777,6 +789,10 @@ const _runnerListeners = function (_runner, Cypress, _emissions, getTestById, ge
   })
 }
 
+function getOrderFromId (id) {
+  return +id.slice(1)
+}
+
 const create = function (specWindow, mocha, Cypress, cy) {
   let _id = 0
   let _hookId = 0
@@ -889,6 +905,14 @@ const create = function (specWindow, mocha, Cypress, cy) {
     return _testsById[id]
   }
 
+  function hasTestAlreadyRun (test) {
+    if (Cypress._RESUMED_AT_TEST) {
+      return getOrderFromId(test.id) < getOrderFromId(Cypress._RESUMED_AT_TEST)
+    }
+
+    return false
+  }
+
   overrideRunnerHook(Cypress, _runner, getTestById, getTest, setTest, getTests)
 
   return {
@@ -951,7 +975,7 @@ const create = function (specWindow, mocha, Cypress, cy) {
 
       switch (runnable.type) {
         case 'hook':
-          test = getTest() || getTestFromHook(runnable, runnable.parent, getTestById)
+          test = getTestFromHook(runnable, runnable.parent, getTestById)
           break
 
         case 'test':
@@ -962,6 +986,17 @@ const create = function (specWindow, mocha, Cypress, cy) {
           break
       }
 
+      // if this isnt a hook, then the name is 'test'
+      const hookName = runnable.type === 'hook' ? getHookName(runnable) : 'test'
+
+      // extract out the next(fn) which mocha uses to
+      // move to the next runnable - this will be our async seam
+      const _next = args[0]
+
+      if (hasTestAlreadyRun(test)) {
+        return _next()
+      }
+
       // closure for calculating the actual
       // runtime of a runnables fn exection duration
       // and also the run of the runnable:after:run:async event
@@ -986,9 +1021,6 @@ const create = function (specWindow, mocha, Cypress, cy) {
         test.wallClockStartedAt = wallClockStartedAt
       }
 
-      // if this isnt a hook, then the name is 'test'
-      const hookName = runnable.type === 'hook' ? getHookName(runnable) : 'test'
-
       // if we haven't yet fired this event for this test
       // that means that we need to reset the previous state
       // of cy - since we now have a new 'test' and all of the
@@ -997,10 +1029,6 @@ const create = function (specWindow, mocha, Cypress, cy) {
         fire(TEST_BEFORE_RUN_EVENT, test, Cypress)
       }
 
-      // extract out the next(fn) which mocha uses to
-      // move to the next runnable - this will be our async seam
-      const _next = args[0]
-
       const next = function (err) {
         // now set the duration of the after runnable run async event
         afterFnDurationEnd = (wallClockEnd = new Date())

packages/driver/test/cypress/integration/cypress/runner_spec.js

@@ -1,6 +1,10 @@
+const { _ } = Cypress
+
 const pending = []
+const testAfterRunEvents = []
 
 Cypress.on('test:after:run', (test) => {
+  testAfterRunEvents.push(test)
   if (test.state === 'pending') {
     return pending.push(test)
   }
@@ -41,3 +45,18 @@ describe('async timeouts', () => {
     cy.then(() => done())
   })
 })
+
+// NOTE: this test must remain the last test in the spec
+// so we can test the root after hook
+describe('fires test:after:run after root after hook', () => {
+  it('test 1', () => {
+  })
+
+  it('test 2', () => {
+  })
+})
+
+// https://github.com/cypress-io/cypress/issues/2296
+after(() => {
+  expect(_.last(testAfterRunEvents).title, 'test:after:run for test 2 should not have fired yet').eq('test 1')
+})

packages/server/__snapshots__/3_issue_1987.ts.js

@@ -0,0 +1,132 @@
+exports['e2e issue 1987 / can reload during spec run'] = `
+
+====================================================================================================
+
+  (Run Starting)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Cypress:    1.2.3                                                                              │
+  │ Browser:    FooBrowser 88                                                                      │
+  │ Specs:      1 found (beforehook-and-test-navigation.js)                                        │
+  │ Searched:   cypress/integration/beforehook-and-test-navigation.js                              │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+────────────────────────────────────────────────────────────────────────────────────────────────────
+                                                                                                    
+  Running:  beforehook-and-test-navigation.js                                               (1 of 1)
+
+
+  suite
+    ✓ test
+    ✓ causes domain navigation
+
+
+  2 passing
+
+
+  (Results)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Tests:        2                                                                                │
+  │ Passing:      2                                                                                │
+  │ Failing:      0                                                                                │
+  │ Pending:      0                                                                                │
+  │ Skipped:      0                                                                                │
+  │ Screenshots:  0                                                                                │
+  │ Video:        true                                                                             │
+  │ Duration:     X seconds                                                                        │
+  │ Spec Ran:     beforehook-and-test-navigation.js                                                │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+  (Video)
+
+  -  Started processing:  Compressing to 32 CRF                                                     
+  -  Finished processing: /XXX/XXX/XXX/cypress/videos/beforehook-and-test-navigation.     (X second)
+                          js.mp4                                                                    
+
+
+====================================================================================================
+
+  (Run Finished)
+
+
+       Spec                                              Tests  Passing  Failing  Pending  Skipped  
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ ✔  beforehook-and-test-navigation.js        XX:XX        2        2        -        -        - │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+    ✔  All specs passed!                        XX:XX        2        2        -        -        -  
+
+
+`
+
+exports['e2e issue 1987 / can run proper amount of hooks'] = `
+
+====================================================================================================
+
+  (Run Starting)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Cypress:    1.2.3                                                                              │
+  │ Browser:    FooBrowser 88                                                                      │
+  │ Specs:      1 found (afterhooks.spec.js)                                                       │
+  │ Searched:   cypress/integration/afterhooks.spec.js                                             │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+────────────────────────────────────────────────────────────────────────────────────────────────────
+                                                                                                    
+  Running:  afterhooks.spec.js                                                              (1 of 1)
+
+
+  suite 1
+    ✓ test 1
+    ✓ test 2
+    ✓ test 3
+    ✓ test 4
+
+  suite 2
+    ✓ s2t1
+
+  suite 3
+    ✓ s3t1
+
+
+  6 passing
+
+
+  (Results)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Tests:        6                                                                                │
+  │ Passing:      6                                                                                │
+  │ Failing:      0                                                                                │
+  │ Pending:      0                                                                                │
+  │ Skipped:      0                                                                                │
+  │ Screenshots:  0                                                                                │
+  │ Video:        true                                                                             │
+  │ Duration:     X seconds                                                                        │
+  │ Spec Ran:     afterhooks.spec.js                                                               │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+  (Video)
+
+  -  Started processing:  Compressing to 32 CRF                                                     
+  -  Finished processing: /XXX/XXX/XXX/cypress/videos/afterhooks.spec.js.mp4              (X second)
+
+
+====================================================================================================
+
+  (Run Finished)
+
+
+       Spec                                              Tests  Passing  Failing  Pending  Skipped  
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ ✔  afterhooks.spec.js                       XX:XX        6        6        -        -        - │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+    ✔  All specs passed!                        XX:XX        6        6        -        -        -  
+
+
+`

packages/server/test/e2e/3_issue_1987.ts

@@ -0,0 +1,29 @@
+const e2e = require('../support/helpers/e2e')
+const Fixtures = require('../support/helpers/fixtures')
+
+describe('e2e issue 1987', () => {
+  e2e.setup({
+    servers: [{
+      port: 3434,
+      static: true,
+    },
+    {
+      port: 4545,
+      static: true,
+    }],
+  })
+
+  // https://github.com/cypress-io/cypress/issues/1987
+  // before/after hooks should not be rerun on top navigation
+  e2e.it('can reload during spec run', {
+    project: Fixtures.projectPath('hooks-after-rerun'),
+    spec: 'beforehook-and-test-navigation.js',
+    snapshot: true,
+  })
+
+  e2e.it('can run proper amount of hooks', {
+    project: Fixtures.projectPath('hooks-after-rerun'),
+    spec: 'afterhooks.spec.js',
+    snapshot: true,
+  })
+})

packages/server/test/integration/cypress_spec.js

@@ -105,7 +105,7 @@ describe('lib/cypress', () => {
   require('mocha-banner').register()
 
   beforeEach(function () {
-    this.timeout(5000)
+    this.timeout(8000)
 
     cache.__removeSync()
 

packages/server/test/support/fixtures/projects/hooks-after-rerun/cypress.json

@@ -0,0 +1 @@
+{}