Repository: Codeception/CodeceptJS
Pull request #591 (2017-07-05 22:52:43)

Refactored steps and hooks improvements
Simplified steps improvements and hooks handling by @APshenkin 

* removed additional events for after and afterSuite hooks
* added `step.passed` and `step.failed` sync events similar to test.passed and test.failed
* separated steps time calculation and printing


Patch:
diff --git a/lib/actor.js b/lib/actor.js
index 6c119341..fdf012a7 100644
--- a/lib/actor.js
+++ b/lib/actor.js
@@ -58,20 +58,25 @@ function recordStep(step, args) {
   // run step inside promise
   recorder.add(task, () => {
     event.emit(event.step.started, step);
-    val = step.run.apply(step, args);
+    step.startTime = +new Date();
+    return val = step.run.apply(step, args);
   });
 
-  // ensure step result is passed to next step
-  recorder.add('return step result', () => val);
+  event.emit(event.step.after, step);
+
+  recorder.add(`step passed`, () => {
+    step.endTime = +new Date();
+    event.emit(event.step.passed, step);
+  });
 
-  //return execution time for logging
-  recorder.add(`return execution time`, () => {
-    output.stepExecutionTime(step, global.suiteDetails);
-    return val;
+  recorder.catchWithoutStop((err) => {
+    step.endTime = +new Date();
+    event.emit(event.step.failed, step);
+    throw err;
   });
 
+  recorder.add(`return result`, () => val);
   // run async after step hooks
-  event.emit(event.step.after, step);
 
   return recorder.promise();
 }
diff --git a/lib/command/run.js b/lib/command/run.js
index 2e56a54d..dfec7064 100644
--- a/lib/command/run.js
+++ b/lib/command/run.js
@@ -4,7 +4,7 @@ const getTestRoot = require('./utils').getTestRoot;
 const deepMerge = require('./utils').deepMerge;
 const fileExists = require('../utils').fileExists;
 const path = require('path');
-const fs = require('fs');
+const mkdirp = require('mkdirp');
 const Config = require('../config');
 const Codecept = require('../codecept');
 const output = require('../output');
@@ -23,7 +23,7 @@ module.exports = function (test, options) {
 
   if (!fileExists(outputDir = path.join(testRoot, config.output))) {
     output.print('creating output directory: ' + outputDir);
-    fs.mkdirSync(outputDir);
+    mkdirp.sync(outputDir);
   }
 
   try {
diff --git a/lib/event.js b/lib/event.js
index 0ac56569..b67f3ac8 100644
--- a/lib/event.js
+++ b/lib/event.js
@@ -5,28 +5,22 @@ var log = require('./output').log;
 module.exports = {
   dispatcher,
   test: {
-    started: 'test.start',
-    before: 'test.before',
-    after: 'test.after',
-    passed: 'test.passed',
-    failed: 'test.failed',
-  },
-  hook: {
-    afterSuite: {
-      failed: 'afterSuiteHook.failed',
-    },
-    after: {
-      failed: 'afterTestHook.failed'
-    }
+    started: 'test.start', // sync
+    before: 'test.before', // async
+    after: 'test.after', // async
+    passed: 'test.passed', // sync
+    failed: 'test.failed', // sync
   },
   suite: {
     before: 'suite.before',
     after: 'suite.after'
   },
   step: {
-    before: 'step.before',
-    started: 'step.start',
-    after: 'step.after'
+    before: 'step.before', // async
+    after: 'step.after',  // async
+    started: 'step.start', // sync
+    passed: 'step.passed', // sync
+    failed: 'step.failed' // syn
   },
   all: {
     before: 'global.before',
diff --git a/lib/helper/Appium.js b/lib/helper/Appium.js
index 73c2025a..343b656e 100644
--- a/lib/helper/Appium.js
+++ b/lib/helper/Appium.js
@@ -121,9 +121,13 @@ class Appium extends WebdriverIO {
     }
 
     if (this.options.windowSize === 'maximize') {
-      this.browser.windowHandleMaximize(false);
-    }
-    if (this.options.windowSize && this.options.windowSize.indexOf('x') > 0) {
+      this.browser.execute('return [screen.width, screen.height]').then((res) => {
+        return this.browser.windowHandleSize({
+          width: res.value[0],
+          height: res.value[1]
+        });
+      });
+    } else if (this.options.windowSize && this.options.windowSize.indexOf('x') > 0) {
       let dimensions = this.options.windowSize.split('x');
       this.browser.windowHandleSize({
         width: dimensions[0],
diff --git a/lib/helper/Nightmare.js b/lib/helper/Nightmare.js
index 73669bb1..56f7f0f7 100644
--- a/lib/helper/Nightmare.js
+++ b/lib/helper/Nightmare.js
@@ -192,7 +192,9 @@ class Nightmare extends Helper {
       return this._stopBrowser();
     }
     if (this.options.keepCookies) return;
-    return Promise.all([this.browser.cookies.clearAll(), this.executeScript('localStorage.clear();')]);
+    return Promise.all([this.browser.cookies.clearAll(), this.executeScript(function () {
+      return localStorage.clear();
+    })]);
   }
 
   _afterSuite() {
diff --git a/lib/helper/SeleniumWebdriver.js b/lib/helper/SeleniumWebdriver.js
index f3c105c9..d52eb0fd 100644
--- a/lib/helper/SeleniumWebdriver.js
+++ b/lib/helper/SeleniumWebdriver.js
@@ -124,11 +124,9 @@ class SeleniumWebdriver extends Helper {
 
     if (this.options.windowSize == 'maximize') {
       this.resizeWindow(this.options.windowSize);
-    }
-
-    if (this.options.windowSize) {
+    } else if (this.options.windowSize && this.options.windowSize.indexOf('x') > 0) {
       var size = this.options.windowSize.split('x');
-      this.resizeWindow(parseInt(size[0]), parseInt(size[1]));
+      this.resizeWindow(size[0], size[1]);
     }
 
     return this.browser;
@@ -668,10 +666,12 @@ class SeleniumWebdriver extends Helper {
    * {{> ../webapi/resizeWindow }}
    */
   resizeWindow(width, height) {
+    let client = this.browser;
     if (width === 'maximize') {
-      return this.browser.manage().window().maximize();
-    }
-    return this.browser.manage().window().setSize(width, height);
+      return client.executeScript('return [screen.width, screen.height]').then(function (res) {
+        return client.manage().window().setSize(parseInt(res[0]), parseInt(res[1]));
+      });
+    } else return client.manage().window().setSize(parseInt(width), parseInt(height));
   }
 
   /**
diff --git a/lib/helper/WebDriverIO.js b/lib/helper/WebDriverIO.js
index d0f7265a..eec1b79a 100644
--- a/lib/helper/WebDriverIO.js
+++ b/lib/helper/WebDriverIO.js
@@ -277,9 +277,13 @@ class WebDriverIO extends Helper {
     }
 
     if (this.options.windowSize === 'maximize') {
-      this.browser.windowHandleMaximize(false);
-    }
-    if (this.options.windowSize && this.options.windowSize.indexOf('x') > 0) {
+      this.browser.execute('return [screen.width, screen.height]').then((res) => {
+        return this.browser.windowHandleSize({
+          width: res.value[0],
+          height: res.value[1]
+        });
+      });
+    } else if (this.options.windowSize && this.options.windowSize.indexOf('x') > 0) {
       let dimensions = this.options.windowSize.split('x');
       this.browser.windowHandleSize({
         width: dimensions[0],
@@ -1073,7 +1077,12 @@ class WebDriverIO extends Helper {
    */
   resizeWindow(width, height) {
     if (width === 'maximize') {
-      return this.browser.windowHandleMaximize(false);
+      return this.browser.execute('return [screen.width, screen.height]').then((res) => {
+        return this.browser.windowHandleSize({
+          width: res.value[0],
+          height: res.value[1]
+        });
+      });
     }
     return this.browser.windowHandleSize({
       width,
diff --git a/lib/interfaces/bdd.js b/lib/interfaces/bdd.js
index 3a8e8fa3..3ad82ac6 100644
--- a/lib/interfaces/bdd.js
+++ b/lib/interfaces/bdd.js
@@ -82,7 +82,7 @@ module.exports = function (suite) {
     };
 
     context.After = function (fn) {
-      afterEachHooks.unshift(['After', scenario.injected(fn, suites[0]), 'after']);
+      afterEachHooks.unshift(['After', scenario.injected(fn, suites[0], 'after')]);
     };
 
     /**
diff --git a/lib/listener/helpers.js b/lib/listener/helpers.js
index a8820bfd..01882350 100644
--- a/lib/listener/helpers.js
+++ b/lib/listener/helpers.js
@@ -36,14 +36,6 @@ module.exports = function () {
     runAsyncHelpersHook('_afterSuite', suite, true);
   });
 
-  event.dispatcher.on(event.hook.after.failed, function (suite) {
-    runAsyncHelpersHook('_after', suite, true);
-  });
-
-  event.dispatcher.on(event.hook.afterSuite.failed, function (suite) {
-    runAsyncHelpersHook('_afterSuite', suite, true);
-  });
-
   event.dispatcher.on(event.test.started, function (test) {
     runHelpersHook('_test', test);
     recorder.catch((e) => error(e));
@@ -55,7 +47,7 @@ module.exports = function () {
 
   event.dispatcher.on(event.test.failed, function (test) {
     runAsyncHelpersHook('_failed', test, true);
-    // should not fail test execution, so errors should be catched
+    // should not fail test execution, so errors should be caught
     recorder.catchWithoutStop((e) => error(e));
   });
 
diff --git a/lib/output.js b/lib/output.js
index 58f87c9f..9aa951a6 100644
--- a/lib/output.js
+++ b/lib/output.js
@@ -74,7 +74,7 @@ module.exports = {
     if (outputLevel === 0) return;
     if (!step) return;
     let sym = process.platform === 'win32' ? '*' : '•';
-    if (outputLevel >= 2) {
+    if (outputLevel == 2) {
       newline = false;
       return process.stdout.write(`${' '.repeat(this.stepShift)} ${sym} ${step.toString()}`);
     }
@@ -87,9 +87,16 @@ module.exports = {
   stepExecutionTime: function (step) {
     if (outputLevel < 2) return;
     if (!step) return;
-    step.endTime = new Date();
-    newline = true;
-    process.stdout.write(` ${styles.debug(" (" + (step.endTime - step.startTime) / 1000 + " sec)\n")}`);
+
+    let time = (step.endTime - step.startTime) / 1000 + " sec";
+
+    if (outputLevel == 2) {
+      newline = true;
+      process.stdout.write(`${styles.debug(" (" + time + ")\n")}`);
+      return;
+    }
+
+    this.log("Step finished in " + time);
   },
 
   suite: {
diff --git a/lib/reporter/cli.js b/lib/reporter/cli.js
index 6d41f8de..e6e80b27 100644
--- a/lib/reporter/cli.js
+++ b/lib/reporter/cli.js
@@ -66,9 +66,9 @@ class Cli extends Base {
         }
       });
 
-      event.dispatcher.on(event.step.started, function (step) {
-        output.step(step, global.suiteDetails);
-      });
+      event.dispatcher.on(event.step.started, (step) => output.step(step));
+      event.dispatcher.on(event.step.passed, (step) => output.stepExecutionTime(step));
+      event.dispatcher.on(event.step.failed, (step) => output.stepExecutionTime(step));
     }
 
     runner.on('end', this.result.bind(this));
diff --git a/lib/scenario.js b/lib/scenario.js
index 856cebf3..aae5a8cd 100644
--- a/lib/scenario.js
+++ b/lib/scenario.js
@@ -81,9 +81,8 @@ module.exports.injected = function (fn, suite, hookName) {
     }
     recorder.catch((err) => {
       event.emit(event.test.failed, suite, err); // emit
-      if (hookName === 'after' || hookName === 'afterSuite') {
-        event.emit(event.hook[hookName].failed, suite);
-      }
+      if (hookName === 'after') event.emit(event.test.after, suite);
+      if (hookName === 'afterSuite') event.emit(event.suite.after, suite);
       throw err;
     });
     return recorder.promise();
diff --git a/lib/step.js b/lib/step.js
index 89709858..0c34b6ca 100644
--- a/lib/step.js
+++ b/lib/step.js
@@ -22,7 +22,6 @@ class Step {
   }
 
   run() {
-    this.startTime = new Date();
     this.args = Array.prototype.slice.call(arguments);
     let result;
     try {


Test patch:
diff --git a/test/runner/interface_test.js b/test/runner/interface_test.js
index 70ba75b7..8d510c79 100644
--- a/test/runner/interface_test.js
+++ b/test/runner/interface_test.js
@@ -33,12 +33,12 @@ describe('CodeceptJS Interface', () => {
         "[1] Queued | hook FileSystem._beforeSuite()",
         `[1] Queued | hook FileSystem._before()`,
         `[1] Queued | amInPath: "."`,
-        `[1] Queued | return step result`,
-        `[1] Queued | return execution time`,
+        `[1] Queued | step passed`,
+        `[1] Queued | return result`,
         `[1] Queued | say hello world`,
         `[1] Queued | seeFile: "codecept.json"`,
-        `[1] Queued | return step result`,
-        `[1] Queued | return execution time`,
+        `[1] Queued | step passed`,
+        `[1] Queued | return result`,
         `[1] Queued | fire test.passed`,
         `[1] Queued | finish test`,
         `[1] Queued | hook FileSystem._after()`,
diff --git a/test/unit/actor_test.js b/test/unit/actor_test.js
index 0b233847..61acdc52 100644
--- a/test/unit/actor_test.js
+++ b/test/unit/actor_test.js
@@ -4,6 +4,8 @@ let actor = require('../../lib/actor');
 let container = require('../../lib/container');
 let path = require('path');
 let should = require('chai').should();
+const recorder = require('../../lib/recorder');
+const event = require('../../lib/event');
 global.codecept_dir = path.join(__dirname, '/..');
 let I;
 
@@ -14,6 +16,7 @@ describe('Actor', () => {
       MyHelper: {
         hello: () => 'hello world',
         bye: () => 'bye world',
+        die: () => { throw new Error('ups') },
         _hidden: () => 'hidden'
       },
       MyHelper2: {
@@ -21,17 +24,56 @@ describe('Actor', () => {
       }
     });
     I = actor();
+    event.cleanDispatcher();
   });
 
   it('should take all methods from helpers and built in', () => {
-    I.should.have.keys(['hello', 'bye', 'greeting', 'say']);
+    I.should.have.keys(['hello', 'bye', 'die', 'greeting', 'say']);
   });
 
   it('should return promise', () => {
-    let recorder = require('../../lib/recorder');
     recorder.start();
     let promise = I.hello();
     promise.should.be.instanceOf(Promise);
     return promise.then((val) => val.should.eql('hello world'));
   });
+
+  it('should produce step events', () => {
+    recorder.start();
+    let listeners = 0;
+    event.dispatcher.addListener(event.step.before, () => listeners++);
+    event.dispatcher.addListener(event.step.after, () => listeners++);
+    event.dispatcher.addListener(event.step.passed, (step) => {
+      listeners++;
+      step.endTime.should.not.be.null;
+      step.startTime.should.not.be.null;
+      step.startTime.should.not.eql(step.endTime);
+    });
+
+    return I.hello().then(() => {
+      listeners.should.eql(3);
+    });
+
+  });
+
+  it('should print handle failed steps', () => {
+    recorder.start();
+    let listeners = 0;
+    event.dispatcher.addListener(event.step.before, () => listeners++);
+    event.dispatcher.addListener(event.step.after, () => listeners++);
+    event.dispatcher.addListener(event.step.failed, (step) => {
+      listeners++;
+      step.endTime.should.not.be.null;
+      step.startTime.should.not.be.null;
+      step.startTime.should.not.eql(step.endTime);
+    });
+
+    return I.die()
+      .then(() => listeners = 0)
+      .catch((err) => null)
+      .then(() => {
+      listeners.should.eql(3);
+    });
+
+  });
 });