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

add 'this' to Step processing (via allure-framework#615) #377

Merged
merged 3 commits into from
Aug 8, 2019

Conversation

Denysss
Copy link
Contributor

@Denysss Denysss commented Jul 17, 2019

fix #615 Add this to Step processing

Checklist

@Denysss
Copy link
Contributor Author

Denysss commented Jul 28, 2019

@baev Could you please review the pull request?

Copy link
Member

@baev baev left a comment

Choose a reason for hiding this comment

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

In general I don't like the idea of having possibility to use this in step annotations (even having in mind that allure 1 allow it)

Your request changes signatures of public methods that can be used by 3rd party integrations (many users use this methods in custom aspects)

@Denysss
Copy link
Contributor Author

Denysss commented Aug 3, 2019

In general I don't like the idea of having possibility to use this in step annotations (even having in mind that allure 1 allow it)

In my case, I use JUnit 5 @TestFactory (DynamicTest) and add @Step annotation for execute methods of the classes which implements org.junit.jupiter.api.function.Executable interface. I found it's very informative for my Allure report consumers to show some values from the fields of these classes. For instance:

...
import org.junit.jupiter.api.function.Executable;
...
public class SomeAction implements Executable {

   private String actionDescription;
   private ActionName actionName;

   public SomeAction(...) {
      ...
   }

   @Step("Action: {this.actionDescription} ({this.actionName})")
   public void execute() throws Throwable {
      ...
   }
   ...
}

@baev Why don't you like to use this? Maybe, we can find a compromise.
I can propose to put all class fields to params instead of the whole this:
params.put("this", joinPoint.getThis()); --> ... params.put("this.field1", value1); ...

@baev
Copy link
Member

baev commented Aug 4, 2019

BTW you can use Allure.step API instead of Step annotation:

public void execute() throws Throwable {
    Allure.step(String.format("Action: %s %s", actionName, actionDescription), () -> {
       ...
    });
}

@baev baev merged commit 9aa3be0 into allure-framework:master Aug 8, 2019
@baev
Copy link
Member

baev commented Aug 8, 2019

@Denysss thanks!

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.

2 participants