New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored steps and hooks improvements #591

Merged
merged 2 commits into from Jul 7, 2017

Conversation

Projects
None yet
2 participants
@DavertMik
Member

DavertMik commented Jul 5, 2017

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

@DavertMik DavertMik requested a review from APshenkin Jul 5, 2017

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));

This comment has been minimized.

@APshenkin

APshenkin Jul 6, 2017

Collaborator

this changes will break our idea to print execution time on the same line as the step in verbose mode. Check the current output

   [1] Queued | return result
   Emitted | step.start (I am on page "https://github.com/404")
 • I am on page "https://github.com/404"
   Emitted | step.passed (I am on page "https://github.com/404")
 (9.716 sec)
   Emitted | step.start (I wait for invisible "#html_banner")
 • I wait for invisible "#html_banner"
   Emitted | step.passed (I wait for invisible "#html_banner")
@APshenkin

APshenkin Jul 6, 2017

Collaborator

this changes will break our idea to print execution time on the same line as the step in verbose mode. Check the current output

   [1] Queued | return result
   Emitted | step.start (I am on page "https://github.com/404")
 • I am on page "https://github.com/404"
   Emitted | step.passed (I am on page "https://github.com/404")
 (9.716 sec)
   Emitted | step.start (I wait for invisible "#html_banner")
 • I wait for invisible "#html_banner"
   Emitted | step.passed (I wait for invisible "#html_banner")

This comment has been minimized.

@DavertMik

DavertMik Jul 6, 2017

Member

Yes, output can get broken in --verbose and --debug mode (if something is printed during debug)
That may happen. And I don't have any solution to that (

Let's have a different format then.

In --debug:

• I am on page "https://github.com/404" (9.716 sec)

in --verbose:

 • I am on page "https://github.com/404"
   Emitted | step.passed (I am on page "https://github.com/404")
   Finished in 9.716 sec
@DavertMik

DavertMik Jul 6, 2017

Member

Yes, output can get broken in --verbose and --debug mode (if something is printed during debug)
That may happen. And I don't have any solution to that (

Let's have a different format then.

In --debug:

• I am on page "https://github.com/404" (9.716 sec)

in --verbose:

 • I am on page "https://github.com/404"
   Emitted | step.passed (I am on page "https://github.com/404")
   Finished in 9.716 sec

This comment has been minimized.

@APshenkin

APshenkin Jul 7, 2017

Collaborator

👍

@APshenkin

APshenkin Jul 7, 2017

Collaborator

👍

Show outdated Hide outdated lib/output.js
@@ -87,9 +87,9 @@ module.exports = {
stepExecutionTime: function (step) {
if (outputLevel < 2) return;
if (!step) return;
step.endTime = new Date();
step.endTime = +new Date();

This comment has been minimized.

@APshenkin

APshenkin Jul 6, 2017

Collaborator

This is unnecessary, if we add endTime in actor.js

@APshenkin

APshenkin Jul 6, 2017

Collaborator

This is unnecessary, if we add endTime in actor.js

Show outdated Hide outdated lib/scenario.js
if (hookName === 'after' || hookName === 'afterSuite') {
event.emit(event.hook[hookName].failed, suite);
}
if (hookName === 'after') event.emit(event.test.after, test);

This comment has been minimized.

@APshenkin

APshenkin Jul 6, 2017

Collaborator

Should beif (hookName === 'after') event.emit(event.test.after, suite);. test is undefined

@APshenkin

APshenkin Jul 6, 2017

Collaborator

Should beif (hookName === 'after') event.emit(event.test.after, suite);. test is undefined

This comment has been minimized.

@APshenkin

APshenkin Jul 6, 2017

Collaborator

Also there is an issue in bdd.js
now

      afterEachHooks.unshift(['After', scenario.injected(fn, suites[0]), 'after']);
    };

should be

context.After = function (fn) {
      afterEachHooks.unshift(['After', scenario.injected(fn, suites[0], 'after')]);
    };

Because of this we don't get name of hook in this place for After

@APshenkin

APshenkin Jul 6, 2017

Collaborator

Also there is an issue in bdd.js
now

      afterEachHooks.unshift(['After', scenario.injected(fn, suites[0]), 'after']);
    };

should be

context.After = function (fn) {
      afterEachHooks.unshift(['After', scenario.injected(fn, suites[0], 'after')]);
    };

Because of this we don't get name of hook in this place for After

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Jul 6, 2017

Member

@APshenkin please check my updates

Member

DavertMik commented Jul 6, 2017

@APshenkin please check my updates

@DavertMik DavertMik merged commit 32e964e into master Jul 7, 2017

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details

@DavertMik DavertMik deleted the refactored-steps branch Jul 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment