Skip to content
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

Jump to bottom

wdio-allure-reporter: add tags cucumber support for scenario #4819

Merged
merged 4 commits into from Nov 27, 2019
Merged

wdio-allure-reporter: add tags cucumber support for scenario #4819

merged 4 commits into from Nov 27, 2019

Conversation

lacell75
Copy link
Contributor

Proposed changes

Fixed #4815

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)
  • Documentation update

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)
  • I have added proper type definitions for new commands (if appropriate)

Further comments

Reviewers: @webdriverio/technical-committee

@codecov
Copy link

codecov bot commented Nov 26, 2019 •
edited

Codecov Report

Merging #4819 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4819      +/-   ##
==========================================
+ Coverage   99.42%   99.42%   +<.01%     
==========================================
  Files         210      210              
  Lines        5425     5435      +10     
  Branches     1185     1187       +2     
==========================================
+ Hits         5394     5404      +10     
  Misses         28       28              
  Partials        3        3
Impacted Files Coverage Δ
...dio-allure-reporter/tests/__fixtures__/cucumber.js 100% <ø> (ø) ⬆️
packages/wdio-allure-reporter/src/index.js 99.14% <100%> (+0.03%) ⬆️

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 e4745ca...992a739. Read the comment docs.

christian-bromann
christian-bromann requested changes Nov 26, 2019
View reviewed changes
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have tests for it?

@lacell75
Copy link
Contributor Author

Yes I can. I will add tests today or tomorrow.

@lacell75
Copy link
Contributor Author

tests added

CrispusDH
CrispusDH requested changes Nov 26, 2019
View reviewed changes
packages/wdio-allure-reporter/src/index.js Outdated Show resolved Hide resolved
packages/wdio-allure-reporter/src/index.js Outdated Show resolved Hide resolved
packages/wdio-allure-reporter/src/index.js Outdated Show resolved Hide resolved
packages/wdio-allure-reporter/src/index.js Outdated Show resolved Hide resolved
packages/wdio-allure-reporter/src/index.js Outdated Show resolved Hide resolved
packages/wdio-allure-reporter/src/index.js Show resolved Hide resolved
packages/wdio-allure-reporter/src/index.js Outdated Show resolved Hide resolved
packages/wdio-allure-reporter/src/index.js Outdated Show resolved Hide resolved
@christian-bromann christian-bromann mentioned this pull request Nov 27, 2019
Closed
@lacell75
Improve quality of code and move add tags to suite level instead of t…
b431b3f 
…est(for cucumber suite=scenario and test=step)
@lacell75
Copy link
Contributor Author

lacell75 commented Nov 27, 2019 •
edited

In the latest commit, I took into account @CrispusDH's remarks and moved the code to the "suite" level instead of "test" level. I realized that for cucumber , suite = scenario and test = step. (The labels were added at each steps instead of at each tests)

CrispusDH
CrispusDH approved these changes Nov 27, 2019
View reviewed changes
Copy link
Contributor

@CrispusDH CrispusDH left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one additional comment to fix. Also I'm not understand about suite and test. You use getLabel with suite but in other places use test.tags...

packages/wdio-allure-reporter/src/index.js Outdated Show resolved Hide resolved
packages/wdio-reporter/src/stats/test.js Outdated Show resolved Hide resolved
christian-bromann
christian-bromann approved these changes Nov 27, 2019
View reviewed changes
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

thanks for reviewing @CrispusDH

@christian-bromann christian-bromann added the PR: Polish 💅 PRs that contain improvements on existing features label Nov 27, 2019
@christian-bromann christian-bromann merged commit fb15729 into webdriverio:master Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christian-bromann christian-bromann christian-bromann approved these changes

@CrispusDH CrispusDH CrispusDH approved these changes

Assignees
No one assigned
Labels
PR: Polish 💅 PRs that contain improvements on existing features
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

@issue, @severity and @tmsLink in cucumber tags are not taken into account by @wdio/allure-reporter
3 participants
@lacell75 @christian-bromann @CrispusDH