Skip to content

feat: display command name for webdriver calls in allure reporter#12982

Merged
christian-bromann merged 4 commits intowebdriverio:mainfrom
sudharsan-selvaraj:fix/12927
Jun 10, 2024
Merged

feat: display command name for webdriver calls in allure reporter#12982
christian-bromann merged 4 commits intowebdriverio:mainfrom
sudharsan-selvaraj:fix/12927

Conversation

@sudharsan-selvaraj
Copy link
Copy Markdown
Contributor

Proposed changes

Currently the allure reporter will display only the endpoint and method name for all webdriver api calls. As part of the change, i have enabled the command name to be also displayed along with the endpoint.
Fixes #12926

Types of changes

  • Polish (an improvement for an existing feature)
  • 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 (improvements to the project's docs)
  • Specification changes (updates to WebDriver command specifications)
  • Internal updates (everything related to internal scripts, governance documentation and CI files)

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

Further comments

Reviewers: @webdriverio/project-committers

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Jun 5, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: sudharsan-selvaraj / name: Sudharsan Selvaraj (5fb5c7f, 69e7276, 93d14b3)
  • ✅ login: christian-bromann / name: Christian Bromann (39f9296)

@christian-bromann
Copy link
Copy Markdown
Member

@sudharsan-selvaraj thanks for taking a step at this. Mind adding a unit test so we can be sure we don't regress?

@sudharsan-selvaraj
Copy link
Copy Markdown
Contributor Author

@sudharsan-selvaraj thanks for taking a step at this. Mind adding a unit test so we can be sure we don't regress?

Sure @christian-bromann. Will raise a patch with the unit tests.

@sudharsan-selvaraj
Copy link
Copy Markdown
Contributor Author

@christian-bromann I have added the required unit test. Kindly look into it and let me know your thoughts. Thank you!

@BorisOsipov
Copy link
Copy Markdown
Member

@sudharsan-selvaraj let's look at things straight, who need /session/:sessionid/ info? it is useless, let's remove it. it will make step name shorter

@sudharsan-selvaraj
Copy link
Copy Markdown
Contributor Author

@BorisOsipov sounds good. The step name is now defaulted just to display the command name and added the endpoints as a fallback incase of missing command name.

Copy link
Copy Markdown
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.

Thank you @sudharsan-selvaraj , changes look good to me 👍
Mind raising a PR for the v8 branch?

@BorisOsipov can you approve?

@sudharsan-selvaraj
Copy link
Copy Markdown
Contributor Author

@christian-bromann sure will create a new PR for v8 branch.

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

Labels

PR: Polish 💅 PRs that contain improvements on existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants