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

Update selenide plugin related to latest selenide changes: #351

Closed
wants to merge 1 commit into from

Conversation

5 participants
@rosolko
Copy link

commented May 8, 2019

Changes:

  • Split onEvent to beforeEvent and afterEvent blocks
  • Start step in beforeEvent block
  • Update and stop step in afterEvent block
  • Remove lifecycle.getCurrentTestCase wrap because uuid is not used and all of the time step receive a new uuid in line stepUUID = UUID.randomUUID().toString();

Checklist

Update selenide plugin related to latest selenide changes:
* Split `onEvent` to `beforeEvent` and `afterEvent` blocks
* Start step in `beforeEvent` block
* Update and stop step in `afterEvent` block
* Remove `lifecycle.getCurrentTestCase` wrap because `uuid` is not used
.setName(event.toString())
.setStatus(Status.PASSED));
public void beforeEvent(final LogEvent event) {
stepUUID = UUID.randomUUID().toString();

This comment has been minimized.

Copy link
@pavelpp

pavelpp May 9, 2019

should it really set status to PASSED? Unlike in the old implementation, this is logged before event even happened.

This comment has been minimized.

Copy link
@rosolko

rosolko May 9, 2019

Author

Previous logic not changed. Except lifecycle.getCurrentTestCase unwrap that was unused before.

@asolntsev

This comment has been minimized.

Copy link

commented May 11, 2019

@eroshenkoam Houston, we need your help!

@BorisOsipov

This comment has been minimized.

Copy link

commented May 14, 2019

Hi @baev,

Could you please help us with this?

@baev

This comment has been minimized.

Copy link
Member

commented May 14, 2019

yep, I reviewed the code and have it may have some problems related to concurrent test execution. I'm planning to have a deeper look at it this week

@baev

This comment has been minimized.

Copy link
Member

commented May 14, 2019

I can promise you a release with this update included by the end of the week tho

@baev

This comment has been minimized.

Copy link
Member

commented May 15, 2019

In order to make it fast I created separate pull request with suggested changes. Comparing to this one it has additional changes:

  1. Fixed potential issue with concurrent test execution since now it stores step uuid in Allure internals (with thread locals storage under the hood)
  2. WebDriver errors on takeScreenshot and getPageSource methods are handled correctly
  3. Now It only sets step status after step finished. Unfinished steps (in case on unexpected behaviour) will have Unknown status
  4. Tests for integration added

See #355

Thanks @rosolko for initial contribution tho 👍

@baev baev closed this May 15, 2019

@rosolko rosolko deleted the rosolko:adopt-selenide-plugin branch May 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.