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

Cucumber 5 Jvm api changes support for allure (fixes #414) #418

Merged
merged 5 commits into from
Feb 18, 2020

Conversation

naveenchandra91
Copy link
Contributor

@naveenchandra91 naveenchandra91 commented Feb 6, 2020

Allure Cucumber 5 Jvm Support module Added all relevant files for support Cucumber 5 jvm changes

Signed-off-by: NRao nrao@flexera.com

Context

Checklist

…port new Cucumber Jvm5 changes

Signed-off-by: NRao <nrao@flexera.com>
@claassistantio
Copy link

claassistantio commented Feb 6, 2020

CLA assistant check
All committers have signed the CLA.

@naveenchandra91
Copy link
Contributor Author

This commit is to fix #414

@naveenchandra91 naveenchandra91 requested a review from baev February 6, 2020 07:26
@baev baev mentioned this pull request Feb 6, 2020
3 tasks
@TomBAMU
Copy link

TomBAMU commented Feb 6, 2020

TestSourceModel Java Class is justed copied from current cucumber-jvm project, making it volatile when cucumber-jvm project changes again.

@naveenchandra91
Copy link
Contributor Author

Hi @TomBAMU : Thanks for reviewing. I did try to use the existing TestSourceModel class. However it was returning an internal gherkin package. I reached out to cucumber developer and they replied the following:

We'll be phasing out Gherkin 5 and the internal html formatter at some point in the future after introducing Gherkin 8. The classes you're looking at are an adaptor to make that seemless in the future.
If you need access to the Gherkin AST you can parse the scenarios yourself based on the TestSourceRead event using:
https://search.maven.org/artifact/io.cucumber/gherkin/5.2.0/jar
There is feature request to provide a minimal representation of all pickles that may meet your needs but it depends a bit on how much detail your reporter has.

So I thought to add the logic of parsing gherkin ast from TestSourceRead event directly in the allure-cucumber5-jvm module. I can refactor the code and just add the basic Ast we need(Feature and ScenarioDefinition Ast). Please let me know your thoughts about what do you think would be the best approach here

@TomBAMU
Copy link

TomBAMU commented Feb 7, 2020

We'll be phasing out Gherkin 5 and the internal html formatter at some point in the future after introducing Gherkin 8. The classes you're looking at are an adaptor to make that seemless in the future.
If you need access to the Gherkin AST you can parse the scenarios yourself based on the TestSourceRead event using:
https://search.maven.org/artifact/io.cucumber/gherkin/5.2.0/jar
There is feature request to provide a minimal representation of all pickles that may meet your needs but it depends a bit on how much detail your reporter has.

That was a missing information on my side, thanks you! 👍

I can refactor the code and just add the basic Ast we need(Feature and ScenarioDefinition Ast)

That sounds like a resonable approach then.
I liked your restructring of the "runFeature" in the tests, thanks.

…port new Cucumber Jvm5 changes. Removed cucumber-core and cucumber-java reference in AllureCucumber5Jvm class and TestSourcesModel class and modified all classes to adhere to checkstyle standards

Signed-off-by: NRao <nrao@flexera.com>
@naveenchandra91
Copy link
Contributor Author

Hi @TomBAMU : I have made the changes to TestSourceModel to keep only basic AST now. @mpkorstanje : I have removed the reference to cucumber-core and cucumber-java package in plugin source(Its still present in the module because its still needed for tests)

Signed-off-by: NRao <nrao@flexera.com>
…nitialization of object to method and simplified instanceof check without null

Signed-off-by: NRao <nrao@flexera.com>
@naveenchandra91
Copy link
Contributor Author

@TomBAMU : Can you please check whether the latest commit is fine?

@naveenchandra91 naveenchandra91 requested review from viclovsky and baev and removed request for baev February 18, 2020 05:50
@baev baev merged commit 040bb0e into allure-framework:master Feb 18, 2020
@naveenchandra91
Copy link
Contributor Author

Thanks @baev for merging this pull request. When can i expect this to be released?

@baev
Copy link
Member

baev commented Feb 19, 2020

thanks to you for the contribution.

When can i expect this to be released?

this week

@ibrahim-beskat
Copy link

Hi @baev,

Has it been released yet? I've been waiting for release to use Allure report for Cucumber 5.5.0.

Thank you.

@baev
Copy link
Member

baev commented Mar 25, 2020

released in 2.13.2

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.

6 participants