wdio-allure-reporter: capture before each and all hooks #3536

mgrybyk · 2019-02-10T02:20:50Z

Proposed changes

Capture before all and after all hooks in Allure report as tests in case:

hook failed
hook has some user defined steps attached to report with allure.addStep(...)

Capture beforeEach and afterEach hooks in Allure report as test steps.

No global hooks will not appear in report.

fix for #3517

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

I have read the CONTRIBUTING doc
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

Further comments

Allure report example with mentioned changes

example with beforeEach failure and afterEach pass as test steps

example with failure in the very first before

more examples here #3536 (comment)

Reviewers: @webdriverio/technical-committee

codecov · 2019-02-10T02:25:24Z

Codecov Report

Merging #3536 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3536      +/-   ##
==========================================
+ Coverage    97.7%   97.75%   +0.04%     
==========================================
  Files         133      133              
  Lines        2969     2978       +9     
  Branches      651      655       +4     
==========================================
+ Hits         2901     2911      +10     
+ Misses         64       63       -1     
  Partials        4        4

Impacted Files	Coverage Δ
packages/wdio-allure-reporter/src/utils.js	`100% <100%> (ø)`	⬆️
packages/wdio-mocha-framework/src/index.js	`100% <100%> (ø)`	⬆️
packages/wdio-allure-reporter/src/constants.js	`100% <100%> (ø)`	⬆️
packages/wdio-allure-reporter/src/index.js	`98.62% <100%> (+0.85%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a6e5ba...7c8a1c9. Read the comment docs.

packages/wdio-mocha-framework/src/index.js

BorisOsipov · 2019-02-10T08:20:28Z

@mgrybyk what about jasmine adapter? Did you check that reporter works as well with wdio-jasmine?

mgrybyk · 2019-02-10T10:01:58Z

@BorisOsipov no, Jasmine logic should not be affected at all.

Previous logic was only ignoring mocha all and each hooks.

if (!this.allure.getCurrentSuite() || ignoredHooks(hook.title)) {
    return false
// ignoredHooks checks if hook is one of
// mochaIgnoredHooks = ['"before all" hook', '"after all" hook', '"before each" hook', '"after each" hook']

I changed logic to capture only that mocha hooks and only for allure reporter.

If we need to change something in other adapters/reporters fee free to submit issue and assign to me.

mgrybyk · 2019-02-10T23:16:09Z

If there are no comments let's merge! :)

BorisOsipov · 2019-02-11T07:52:44Z

packages/wdio-allure-reporter/src/index.js

 return false
 }

- this.allure.startCase(hook.title)
+ // todo: describe what are we doing here


I suppose, it was backported from v4, as you I am no sure it is required or can be removed

I'm not sure as well. I'd better keep existing logic as is for now.

We can improve it in different PR

@BorisOsipov removed // todo: describe what are we doing here and refactored that code.

BorisOsipov · 2019-02-11T08:18:15Z

There is one issue why hooks are ignored. If you report hooks as test - it will significantly increase end user tests count - and it is bad because hooks aren't tests

mgrybyk · 2019-02-11T11:17:31Z

@BorisOsipov why significantly?
If there are no user defined hooks report will look the same.

At least before hook in testing is called precondition which is a part of test case.

We should always show before hooks, but I can't say the same for after hooks.

We can print after hooks only if there is some error which will be useful to understand why tests have failed.

On the other hand showing before and after hooks will provide better explanation in terms of timing.

mgrybyk · 2019-02-11T11:24:28Z

Let me hide passed after hooks.

P.s.
Ideally it should be on before all hook per suite.

BorisOsipov · 2019-02-11T11:30:02Z

why significantly?
If there are no user defined hooks report will look the same.

In any project I worked we had before\all hooks. So if we have N tests, if we enable reporting than we will have + N tests in report - It isn't good. Also we will have misleading with e.g. spec reporter, who doens't count hooks as tests.

We should always shown before hooks, but I can't say the same for after hooks.
On the other hand showing before and after hooks will provide better explanation in terms of timing.

Yes, I totally agree with you, but it hard to implement, because AFAIK hook:start event appears earlier than test:start, so there is no started test in allure => you can't create step for reporting.

We can print after hooks only if there is some error which will be useful to understand why tests have failed.

yes it will be good. I suppose it already implemented? isn't it?

mgrybyk · 2019-02-11T11:38:29Z

@BorisOsipov

So if we have N tests, if we enable reporting than we will have + N tests in report - It isn't good.

Actually nope, only if there are suite level before/after hooks. beforeEach and afterEach are not added as tests so they won't bother user.

I suppose it already implemented? isn't it?

Not really, have to adjust implementation so that failed after hook will be shown only in case of error.

mgrybyk · 2019-02-11T11:43:21Z

describe
  before
  beforeEach
  it
  it
  it
  after
  afterEach

With current implementation of this PR we have in report something like that. (As you might see each hooks are not spamming report.)

before
it
it
it
after

I want to change it to

before
it
it
it
after (only in case of failure)

mgrybyk · 2019-02-11T11:52:00Z

Is this fine? @BorisOsipov

BorisOsipov · 2019-02-11T12:01:48Z

No, sorry. We must not report hooks as tests. It will cause misleading and question about test counts in different reporters. But I think reporting failed hooks is really important to do.

So can we make in that way?

before (report as test only in case of failure)
beforeEach(report as steps always)
it
it
it
afterEach (report as steps always)
after (report as test only in case of failure)

what do you think?

cc @christian-bromann, maybe you have some ideas

mgrybyk · 2019-02-11T13:20:06Z

I think that part of the test should be reported explicitly.

However I agree that there will be questions to test count, so I should not show hooks as tests.

As a conclusion: if I won't be able to add before as test step properly I'll only shown that hooks in case of failures as you've suggested.

BorisOsipov · 2019-02-11T13:23:18Z

@mgrybyk 👍

mgrybyk · 2019-02-11T22:13:49Z

@BorisOsipov behavior is now simplified:

beforeEach/afterEach hooks are added to test as steps

all the other hooks (mocha before/after or whatever) are added as tests only in case of failure or if hook has steps (added with allure.addStep(...)).

Steps are controlled by user, in order to have passed hook in report user has to trigger allure.addStep(...) in before/after or any other hook (except of mocha beforeEach or afterEach of course).

mgrybyk · 2019-02-11T22:30:12Z

example with added user step in before hook

  before(() => {
    browser.url("https://google.com.ua")
    reporter.addStep('user added step in before', undefined, 'passed')
  })

example with no user steps in before and failed after hook

mgrybyk · 2019-02-14T10:47:40Z

@BorisOsipov so you have some more comments?

BorisOsipov · 2019-02-14T13:26:36Z

I will review at the weekends, I bit busy now

…

On Thu, Feb 14, 2019 at 1:47 PM Mykola Grybyk ***@***.***> wrote: @@BorisOsipov <https://github.com/BorisOsipov> so you have some more comments? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3536 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGNmZFvOmXYlrkKMN2fuBc673-28bUxgks5vNT7PgaJpZM4ay2Td> .

--

-- Best regards, Boris Osipov

BorisOsipov

Awesome 👍

packages/wdio-allure-reporter/src/index.js

packages/wdio-mocha-framework/src/index.js

…3536)

mgrybyk commented Feb 10, 2019

View reviewed changes

packages/wdio-mocha-framework/src/index.js Show resolved Hide resolved

mgrybyk commented Feb 10, 2019

View reviewed changes

packages/wdio-mocha-framework/src/index.js Show resolved Hide resolved

BorisOsipov reviewed Feb 11, 2019

View reviewed changes

wdio-allure-reporter: capture before each and all hooks

7c8a1c9

mgrybyk mentioned this pull request Feb 12, 2019

beforeAll and afterAll (mocha + chai) failures are not captured in report #3517

Closed

BorisOsipov approved these changes Feb 16, 2019

View reviewed changes

packages/wdio-allure-reporter/src/index.js Show resolved Hide resolved

packages/wdio-mocha-framework/src/index.js Show resolved Hide resolved

christian-bromann added the PR: Polish 💅 PRs that contain improvements on existing features label Feb 18, 2019

christian-bromann merged commit 4aae6be into webdriverio:master Feb 18, 2019

mgrybyk deleted the allure-reporter-capture-mocha-hooks branch February 21, 2019 12:20

yamkay pushed a commit to MoveInc/webdriverio that referenced this pull request Jun 13, 2019

wdio-allure-reporter: capture before each and all hooks (webdriverio#…

6a562e8

…3536)

yamkay pushed a commit to MoveInc/webdriverio that referenced this pull request Sep 4, 2019

wdio-allure-reporter: capture before each and all hooks (webdriverio#…

9027c77

…3536)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wdio-allure-reporter: capture before each and all hooks #3536

wdio-allure-reporter: capture before each and all hooks #3536

mgrybyk commented Feb 10, 2019 •

edited

Loading

codecov bot commented Feb 10, 2019 •

edited

Loading

BorisOsipov commented Feb 10, 2019

mgrybyk commented Feb 10, 2019 •

edited

Loading

mgrybyk commented Feb 10, 2019

BorisOsipov Feb 11, 2019

mgrybyk Feb 11, 2019

mgrybyk Feb 11, 2019

BorisOsipov commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

BorisOsipov commented Feb 11, 2019

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019

BorisOsipov commented Feb 11, 2019

mgrybyk commented Feb 11, 2019 •

edited

Loading

BorisOsipov commented Feb 11, 2019

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 14, 2019 •

edited

Loading

BorisOsipov commented Feb 14, 2019 via email

BorisOsipov left a comment

wdio-allure-reporter: capture before each and all hooks #3536

wdio-allure-reporter: capture before each and all hooks #3536

Conversation

mgrybyk commented Feb 10, 2019 • edited Loading

Proposed changes

Types of changes

Checklist

Further comments

Reviewers: @webdriverio/technical-committee

codecov bot commented Feb 10, 2019 • edited Loading

Codecov Report

BorisOsipov commented Feb 10, 2019

mgrybyk commented Feb 10, 2019 • edited Loading

mgrybyk commented Feb 10, 2019

BorisOsipov Feb 11, 2019

Choose a reason for hiding this comment

mgrybyk Feb 11, 2019

Choose a reason for hiding this comment

mgrybyk Feb 11, 2019

Choose a reason for hiding this comment

BorisOsipov commented Feb 11, 2019 • edited Loading

mgrybyk commented Feb 11, 2019 • edited Loading

mgrybyk commented Feb 11, 2019 • edited Loading

BorisOsipov commented Feb 11, 2019

mgrybyk commented Feb 11, 2019 • edited Loading

mgrybyk commented Feb 11, 2019 • edited Loading

mgrybyk commented Feb 11, 2019

BorisOsipov commented Feb 11, 2019

mgrybyk commented Feb 11, 2019 • edited Loading

BorisOsipov commented Feb 11, 2019

mgrybyk commented Feb 11, 2019 • edited Loading

mgrybyk commented Feb 11, 2019 • edited Loading

mgrybyk commented Feb 14, 2019 • edited Loading

BorisOsipov commented Feb 14, 2019 via email

BorisOsipov left a comment

Choose a reason for hiding this comment

mgrybyk commented Feb 10, 2019 •

edited

Loading

codecov bot commented Feb 10, 2019 •

edited

Loading

mgrybyk commented Feb 10, 2019 •

edited

Loading

BorisOsipov commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 11, 2019 •

edited

Loading

mgrybyk commented Feb 14, 2019 •

edited

Loading