Skip to content

Conversation

@rosolko
Copy link

@rosolko rosolko 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

* 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();
Copy link

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

@asolntsev
Copy link
Contributor

@eroshenkoam Houston, we need your help!

@BorisOsipov
Copy link
Contributor

Hi @baev,

Could you please help us with this?

@baev
Copy link
Member

baev 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
Copy link
Member

baev commented May 14, 2019

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

@baev
Copy link
Member

baev 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 adopt-selenide-plugin branch May 15, 2019 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants