Fix work with promise chains #568

APshenkin · 2017-06-22T09:35:23Z

Our goals:

Fix After block not properly executed if Scenario fails #543 - After block not properly executed if Scenario fails
Expected behavior in promise chains: _beforeSuite hooks from helpers -> BeforeSuite from test -> _before hooks from helpers -> Before from test - > Test steps -> _failed hooks from helpers (if test failed) -> After from test -> _after hooks from helpers -> AfterSuite from test -> _afterSuite hook from helpers.
if during test we got errors from any hook (in test or in helper) - stop complete this suite and go to another
if during test we got error from Selenium server - stop complete this suite and go to another
if restart option is false - close all tabs expect one in _after (ready for webdriverIO, SeleniumWebdriver and Protractor. For Nightmare it will be hard to add (I found only this way https://github.com/rosshinkley/nightmare-window-manager)
Complete _after, _afterSuite hooks even After/AfterSuite from test was failed
Don't close browser between suites, when restart option is false. We should start browser only one time and close it only after all tests.
Close tabs and clear local storage, if keepCookies flag is enabled

Bonus:

Add step execution time in debug mode
Fix Multiple run and XML report #548

DavertMik · 2017-06-22T20:20:18Z

Please fix test/runner/interface_test.js, it tests the order of events

DavertMik · 2017-06-24T22:36:00Z

Wow, awesome work. We are getting closer and closer to 1.0 release!

DavertMik · 2017-06-25T00:14:38Z

lib/interfaces/bdd.js

@@ -45,6 +49,12 @@ module.exports = function (suite) {
        suites.shift();
      }
      if (!opts) opts = {};
+
+      afterAllHooks = [];


this was already defined at line 27

DavertMik · 2017-06-25T00:17:05Z

Please try to use interactive shell to check that nothing was broken for it.
Also, execute one test with within block to be sure this changes to recorder doesn't break them.

APshenkin · 2017-06-25T19:57:59Z

@DavertMik
During these changes I found one interaction that may be not correct. I want to ask your advice about it.
Now we have clear promises chain as described in topic. It will break only after fail. Also if we get error in BeforeSuite, Before or After hooks we will skip all tests in this suite (looks it's ok, because there is no sense to try repeat test with the sameBeforeSuite/Before/After if it's not working). But there is an interesting ways with After and AfterSuite hooks.
If After hook from test will fail, then _after hooks from helpers will not be executed.
The same thing with AfterSuite - if it fails, then _afterSuite hooks from helpers will not be executed. It looks normal, but this can cause an issue, that after failure in these hooks, browser will not close or we will not clear environment for next tests.

I think, that maybe we can track this and force close/clear browser in this cases.

What do you think? Should we track this, or it should be resolved from user side?

DavertMik · 2017-06-25T22:34:20Z

If After hook from test will fail, then _after hooks from helpers will not be executed.

Yes, this makes a big problem for closing browser sessions...
I have 2 ideas about this:

run _after hook for Helper even test has failed in After. Same for AfterSuite. I think it is better to have all _after hooks to be executed no matter what happened.
add new finalize hook.

But this is important to solve it in one way or another.

APshenkin · 2017-06-26T11:55:44Z

@DavertMik please review the changes. pause() and within block works as expected

DavertMik · 2017-06-26T22:55:29Z

lib/helper/SeleniumWebdriver.js

+   * I.closeTabsExceptForOne();
+   * ```
+   */
+  closeTabsExceptForOne() {


better to rename:

closeOtherTabs

DavertMik · 2017-06-26T23:00:06Z

lib/step.js

@@ -22,6 +22,7 @@ class Step {
  }

  run() {
+    this.startTime = new Date();


I think this date-time counter should be moved out to https://github.com/Codeception/CodeceptJS/blob/master/lib/listener/steps.js

If we place it in steps.js then we can't pass this value properly between states. Or I don't know how to do it. If you have any suggestion, it will be awesome

DavertMik · 2017-06-26T23:01:39Z

lib/scenario.js

  event.emit(event.test.before);
 };

 module.exports.teardown = function () {
+  if (!recorder.isRunning()) {


you repeat this 3 lines too much.
better to make a method recorder->startUnlessRunning()

DavertMik · 2017-06-27T01:55:14Z

lib/event.js

@@ -11,6 +11,14 @@ module.exports = {
    passed: 'test.passed',
    failed: 'test.failed',
  },
+  hook: {


This name is a bit confusing. Ok, these are special events fired by a failing after and afterSuite.

Ok, let's try to clarify their context. Those methods should not fail, so if they do it is an error.
So I propose to have next events: test.after.error, and suite.after.error instead.

we can't use test.after/suite.after, because they are already defined. I suggest test.afterError, suite.afterError

test.afterError, suite.afterError

good

DavertMik · 2017-06-27T01:58:12Z

lib/actor.js

-    val = step.run.apply(step, args);
+    val = step.run.apply(step, args).then(() => {
+      step.endTime = new Date();
+      output.log(`-> ${step.toString()} was executed in ${(step.endTime - step.startTime) / 1000} sec`);


move this code to event listener

DavertMik · 2017-06-27T01:58:59Z

lib/interfaces/bdd.js

@@ -88,7 +95,18 @@ module.exports = function (suite) {
        fn = opts;
        opts = {};
      }
-
+      if (!afterAllHooksAreLoaded) {
+        afterAllHooks.forEach(function (hook) {


add some comments about what issues does it solve

I added comment in test.
FYI, we collect all hooks in right order in array and then push them to suite. We start collecting hooks from helpers and after from testfile. this cause hooks reordering
(expected: After from test -> _after hooks from helpers -> AfterSuite from test -> _afterSuite hook from helpers.
actual: _after hooks from helpers -> After from test -> _afterSuite hook from helpers. -> AfterSuite from test )

This change fix this chain

Run after block after test fails

af69c3e

Pshenkin Andrey added 3 commits June 23, 2017 10:55

fix interface test

62e2005

fix formatting

2d30ea2

refactor work with chains

0c771f5

APshenkin changed the title ~~Run after block after test fails~~ [WIP] Fix work with promise chains Jun 23, 2017

Pshenkin Andrey added 4 commits June 23, 2017 14:50

fix typo

ee83622

remove second after

7b7c0ba

correct chain order

7c69ce5

fix formatting

e9329fe

DavertMik reviewed Jun 25, 2017

View reviewed changes

APshenkin mentioned this pull request Jun 25, 2017

Scenarios fail if restart=false. #564

Closed

Pshenkin Andrey added 10 commits June 26, 2017 10:51

Add compliting _after _afterSuite if after hooks failed

b48f9ac

fix formatting

cf8e60a

Add new hook finishTest

9a67d45

fix formatting

cce7f76

return recorder back

5eb9b3e

fix formatting

98b8358

add closing tabs for Protractor and selenium

55a0482

fix formatting

110d3c1

fix uppercase

79f5c99

fix hooks

b63a20f

Pshenkin Andrey added 3 commits June 26, 2017 15:53

clear localstorage and close tabs with keepCookies flag

68bbe79

fix formatting

206c1b7

Fixes

ce22a6a

remove defenition

db517cc

APshenkin changed the title ~~[WIP] Fix work with promise chains~~ Fix work with promise chains Jun 26, 2017

Pshenkin Andrey added 2 commits June 26, 2017 20:30

add step execution time in Verbose mode

bdd1c64

fix formatting

00c35dd

DavertMik requested changes Jun 27, 2017

View reviewed changes

Pshenkin Andrey added 5 commits June 27, 2017 16:24

Add step execution time to output

c90788c

fix formatting

30d15f5

fixes after review

4871219

fix formatting

58f51ce

fix typo

62adade

APshenkin mentioned this pull request Jun 27, 2017

###How To Calculate Page Load Time in CodeceptJs and How to Print it in Console### #550

Closed

Pshenkin Andrey added 4 commits June 27, 2017 18:00

fix tests with generator

e3b245e

fix formatting

861b47a

add mochaFile override

91a5df9

add report file name in override config for junit reporter

e9ec704

DavertMik approved these changes Jun 28, 2017

View reviewed changes

DavertMik merged commit a79a725 into master Jun 28, 2017

DavertMik deleted the use-after-when-fail branch June 28, 2017 23:02

lisettea mentioned this pull request Jul 25, 2017

Cookies aren't being cleared from local storage (1.0.0) #631

Closed

Uh oh!

Fix work with promise chains #568

Fix work with promise chains #568

Uh oh!

Conversation

APshenkin commented Jun 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DavertMik commented Jun 22, 2017

Uh oh!

DavertMik commented Jun 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DavertMik commented Jun 25, 2017

Uh oh!

APshenkin commented Jun 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DavertMik commented Jun 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

APshenkin commented Jun 26, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

APshenkin commented Jun 22, 2017 •

edited

Loading

APshenkin commented Jun 25, 2017 •

edited

Loading

DavertMik commented Jun 25, 2017 •

edited

Loading