Skip to content

Added @Disabled tests handling#222

Merged
baev merged 6 commits into
allure-framework:masterfrom
sidelnikovmike:disabled_fix
May 29, 2018
Merged

Added @Disabled tests handling#222
baev merged 6 commits into
allure-framework:masterfrom
sidelnikovmike:disabled_fix

Conversation

@sidelnikovmike
Copy link
Copy Markdown
Contributor

@sidelnikovmike sidelnikovmike commented May 18, 2018

Context

This pull request fixes problem, that tests with annotation @Disabled not shown in report.
Allure shows skipped test only in case of assumption.

Checklist

@sskorol
Copy link
Copy Markdown
Contributor

sskorol commented May 18, 2018

@sidelnikovmike seems like there're couple of PMD violations... I'd assume it failed on AllureJunit5.java - line 87 (missing final keyword on input args)... please run gradlew check locally to detect and fix them...

}

@Override
public void executionSkipped(TestIdentifier testIdentifier, String reason) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

quality plugin will fail on missing final for inputs

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll look at checks, thanks

.withStage(Stage.FINISHED)
.withStatus(SKIPPED);

methodSource.ifPresent(source -> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just wondering, why do you need to create a local variable methodSource, if it's used only once?.. 89 line could be merged with this one I believe...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Don't be so wondering :) such things sometime happens.
Thanks, I'll fix this

@sidelnikovmike
Copy link
Copy Markdown
Contributor Author

I'll now look at fail

// "ISSUE-1", "ISSUE-2", "ISSUE-3",
// "TMS-1", "TMS-2", "TMS-3"
// );
// }
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry :)

@sskorol
Copy link
Copy Markdown
Contributor

sskorol commented May 18, 2018

Summon @baev

@sidelnikovmike
Copy link
Copy Markdown
Contributor Author

@eroshenkoam or @baev could you look at PR?

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

Code looks ok.

As far as I know some people likely will not like to see disabled tests in the report.

In case you run only one test group will other tests reported as skipped?

@sidelnikovmike
Copy link
Copy Markdown
Contributor Author

@baev I think no. Group - another class? If you run only group - other tests will not run at all

@baev
Copy link
Copy Markdown
Member

baev commented May 28, 2018

Group - another class?

I mean running tests using tags

@baev baev merged commit 5fd6fce into allure-framework:master May 29, 2018
@baev
Copy link
Copy Markdown
Member

baev commented May 29, 2018

@sidelnikovmike @sskorol 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.

3 participants