Repository: Codeception/CodeceptJS (js)

Issue:
AfterSuite can't be defined after first Scenario
#### What are you trying to achieve?

I expect `AfterSuite` to run if it is placed at the end of a test file.

#### What do you get instead?

It does not run. It only runs when you define `AfterSuite` before the first `Scenario`.

For some reason, [context.AfterSuite](https://github.com/Codeception/CodeceptJS/blob/master/lib/interfaces/bdd.js#L121-L123) does not register the hooks directly.  Instead, the hooks are only registered when [AddScenario](https://github.com/Codeception/CodeceptJS/blob/master/lib/interfaces/bdd.js#L46-L51) is called for the first time.

This means any AfterSuite defined after the first Scenario will not be registered. This is unexpected and undocumented behavior.

I haven't submitted a fix because there might be a good reason that `AfterSuite` does not register the hook directly. 
  

Pull request description:
[AfterSuite|After] Allow AfterSuite and After test hooks to be defined after the first Scenario
### Highlights

Allow `AfterSuite` and `After` test hooks to be defined after the first Scenario. This allows for a more flexible and natural placement of these two test hooks. Previously these two hooks needed to be placed before the first `Scenario`. Fixes #857

#### Example
```js
const assert = require('assert');

Feature('My new feature');

BeforeSuite((I) => {
  console.log('Before suite');
});

Before((I) => {
  console.log('Before each test');
});

Scenario('Test Case 1', (I) => {
    I.amOnPage('/');
});

Scenario('Test Case 2', (I) => {
  I.amOnPage('/');
});

After((I) => {
  console.log('After each test');
});

AfterSuite((I) => {
  console.log('Bfter suite');
});
```

Patch:
diff --git a/lib/interfaces/bdd.js b/lib/interfaces/bdd.js
index c32c5439..661e7549 100644
--- a/lib/interfaces/bdd.js
+++ b/lib/interfaces/bdd.js
@@ -39,22 +39,6 @@ module.exports = function (suite) {
         fn = opts;
         opts = {};
       }
-      /**
-       * load hooks from arrays to suite to prevent reordering
-       *
-       */
-      if (!afterAllHooksAreLoaded) {
-        afterAllHooks.forEach((hook) => {
-          suites[0].afterAll(hook[0], hook[1]);
-        });
-        afterAllHooksAreLoaded = true;
-      }
-      if (!afterEachHooksAreLoaded) {
-        afterEachHooks.forEach((hook) => {
-          suites[0].afterEach(hook[0], hook[1]);
-        });
-        afterEachHooksAreLoaded = true;
-      }
       if (suite.pending) {
         fn = null;
       }
@@ -159,4 +143,23 @@ module.exports = function (suite) {
 
     addDataContext(context);
   });
+
+  suite.on('post-require', (context, file, mocha) => {
+    /**
+     * load hooks from arrays to suite to prevent reordering
+     */
+    if (!afterEachHooksAreLoaded && Array.isArray(afterEachHooks)) {
+      afterEachHooks.forEach((hook) => {
+        suites[0].afterEach(hook[0], hook[1]);
+      });
+      afterEachHooksAreLoaded = true;
+    }
+
+    if (!afterAllHooksAreLoaded && Array.isArray(afterAllHooks)) {
+      afterAllHooks.forEach((hook) => {
+        suites[0].afterAll(hook[0], hook[1]);
+      });
+      afterAllHooksAreLoaded = true;
+    }
+  });
 };


Test patch:
diff --git a/test/data/helper.js b/test/data/helper.js
index d6eb338a..5cc650b5 100644
--- a/test/data/helper.js
+++ b/test/data/helper.js
@@ -5,6 +5,22 @@ class MyHelper extends Helper {
     return 'hello world';
   }
 
+  _beforeSuite() {
+    console.log('Helper: I\'m simple BeforeSuite hook');
+  }
+
+  _before() {
+    console.log('Helper: I\'m simple Before hook');
+  }
+
+  _after() {
+    console.log('Helper: I\'m simple After hook');
+  }
+
+  _afterSuite() {
+    console.log('Helper: I\'m simple AfterSuite hook');
+  }
+
   _passed() {
     console.log('Event:test.passed (helper)');
   }
@@ -22,15 +38,15 @@ class MyHelper extends Helper {
   }
 
   stringWithHook(hookName) {
-    return `I'm generator ${hookName} hook`;
+    return `Test: I'm generator ${hookName} hook`;
   }
 
   asyncStringWithHook(hookName) {
-    return `I'm async/await ${hookName} hook`;
+    return `Test: I'm async/await ${hookName} hook`;
   }
 
   stringWithScenarioType(type) {
-    return `I'm ${type} test`;
+    return `Test: I'm ${type} test`;
   }
 }
 
diff --git a/test/data/sandbox/codecept.testhooks.different.order.json b/test/data/sandbox/codecept.testhooks.different.order.json
new file mode 100644
index 00000000..54ac4b11
--- /dev/null
+++ b/test/data/sandbox/codecept.testhooks.different.order.json
@@ -0,0 +1,14 @@
+{
+  "tests": "./*_test.testhooks.different.order.js",
+  "timeout": 10000,
+  "output": "./output",
+  "helpers": {
+    "FakeDriver": {
+      "require": "../helper"
+    }
+  },
+  "include": {},
+  "bootstrap": false,
+  "mocha": {},
+  "name": "sandbox"
+}
diff --git a/test/data/sandbox/testhooks_test.testhooks.different.order.js b/test/data/sandbox/testhooks_test.testhooks.different.order.js
new file mode 100644
index 00000000..71f4a0d7
--- /dev/null
+++ b/test/data/sandbox/testhooks_test.testhooks.different.order.js
@@ -0,0 +1,25 @@
+Feature('Test hooks');
+
+BeforeSuite(async (I) => {
+  const text = await I.asyncStringWithHook('BeforeSuite');
+  console.log(text);
+});
+
+Before(async (I) => {
+  const text = await I.asyncStringWithHook('Before');
+  console.log(text);
+});
+
+Scenario('Simple test 1', () => {
+  console.log('Scenario: It\'s first test');
+});
+
+After(async (I) => {
+  const text = await I.asyncStringWithHook('After');
+  console.log(text);
+});
+
+AfterSuite(async (I) => {
+  const text = await I.asyncStringWithHook('AfterSuite');
+  console.log(text);
+});
diff --git a/test/data/sandbox/testhooks_test.testhooks.js b/test/data/sandbox/testhooks_test.testhooks.js
index 775f2f89..5c8cdb2e 100644
--- a/test/data/sandbox/testhooks_test.testhooks.js
+++ b/test/data/sandbox/testhooks_test.testhooks.js
@@ -1,19 +1,19 @@
 Feature('Test hooks');
 
 BeforeSuite(() => {
-  console.log('I\'m simple BeforeSuite hook');
+  console.log('Test: I\'m simple BeforeSuite hook');
 });
 
 Before(() => {
-  console.log('I\'m simple Before hook');
+  console.log('Test: I\'m simple Before hook');
 });
 
 After(() => {
-  console.log('I\'m simple After hook');
+  console.log('Test: I\'m simple After hook');
 });
 
 AfterSuite(() => {
-  console.log('I\'m simple AfterSuite hook');
+  console.log('Test: I\'m simple AfterSuite hook');
 });
 
 BeforeSuite(function* (I) {
@@ -57,5 +57,5 @@ AfterSuite(async (I) => {
 });
 
 Scenario('Simple test 1', () => {
-  console.log('It\'s first test');
+  console.log('Scenario: It\'s first test');
 });
diff --git a/test/runner/codecept_test.js b/test/runner/codecept_test.js
index 8998e476..06b9f7d5 100644
--- a/test/runner/codecept_test.js
+++ b/test/runner/codecept_test.js
@@ -87,19 +87,44 @@ describe('CodeceptJS Runner', () => {
   it('should run hooks from suites', (done) => {
     exec(codecept_run_config('codecept.testhooks.json'), (err, stdout) => {
       const lines = stdout.match(/\S.+/g);
+
+      expect(lines).to.include.members([
+        'Helper: I\'m simple BeforeSuite hook',
+        'Test: I\'m simple BeforeSuite hook',
+        'Test: I\'m generator BeforeSuite hook',
+        'Test: I\'m async/await BeforeSuite hook',
+        'Helper: I\'m simple Before hook',
+        'Test: I\'m simple Before hook',
+        'Test: I\'m generator Before hook',
+        'Test: I\'m async/await Before hook',
+        'Test: I\'m generator After hook',
+        'Test: I\'m simple After hook',
+        'Test: I\'m async/await After hook',
+        'Helper: I\'m simple After hook',
+        'Test: I\'m generator AfterSuite hook',
+        'Test: I\'m simple AfterSuite hook',
+        'Test: I\'m async/await AfterSuite hook',
+        'Helper: I\'m simple AfterSuite hook',
+      ]);
+      stdout.should.include('OK  | 1 passed');
+      assert(!err);
+      done();
+    });
+  });
+
+  it('should run hooks from suites (in different order)', (done) => {
+    exec(codecept_run_config('codecept.testhooks.different.order.json'), (err, stdout) => {
+      const lines = stdout.match(/\S.+/g);
+
       expect(lines).to.include.members([
-        'I\'m simple BeforeSuite hook',
-        'I\'m generator BeforeSuite hook',
-        'I\'m async/await BeforeSuite hook',
-        'I\'m simple Before hook',
-        'I\'m generator Before hook',
-        'I\'m async/await Before hook',
-        'I\'m generator After hook',
-        'I\'m simple After hook',
-        'I\'m async/await After hook',
-        'I\'m generator AfterSuite hook',
-        'I\'m simple AfterSuite hook',
-        'I\'m async/await AfterSuite hook',
+        'Helper: I\'m simple BeforeSuite hook',
+        'Test: I\'m async/await BeforeSuite hook',
+        'Helper: I\'m simple Before hook',
+        'Test: I\'m async/await Before hook',
+        'Test: I\'m async/await After hook',
+        'Helper: I\'m simple After hook',
+        'Test: I\'m async/await AfterSuite hook',
+        'Helper: I\'m simple AfterSuite hook',
       ]);
       stdout.should.include('OK  | 1 passed');
       assert(!err);
@@ -113,9 +138,9 @@ describe('CodeceptJS Runner', () => {
       expect(lines).to.include.members([
         'Test scenario types --',
         'It\'s usual test',
-        'I\'m generator test',
-        'I\'m async/await test',
-        'I\'m asyncbrackets test',
+        'Test: I\'m generator test',
+        'Test: I\'m async/await test',
+        'Test: I\'m asyncbrackets test',
       ]);
       stdout.should.include('OK  | 4 passed');
       assert(!err);