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

[TestNG] Make "Display disabled tests in report" configurable #369

Closed
1 of 3 tasks
MindaugasMateika opened this issue Jun 7, 2019 · 16 comments
Closed
1 of 3 tasks

Comments

@MindaugasMateika
Copy link

MindaugasMateika commented Jun 7, 2019

I'm submitting a ...

  • bug report
  • feature request
  • support request => Please do not submit support request here, see note at the top of this template.

What is the current behavior?

Disabled tests (@Test(enabled = false)) appear as unknown in the report.

#275

What is the expected behavior?

Make it configurable so there would be an option to allow disabled tests to appear in report or not.

What is the motivation / use case for changing the behavior?

We have a test suite which supports the same application with different features in multiple countries. Some countries do not have features that other countries have thus we're using Annotation Transformer to disable those tests for those countries.

It would be nice to not see those disabled tests in the report as they clutter the report and create a distraction.

Please tell us about your environment:

Allure version 2.8
Test framework testng@6.14.3
Allure integration -
Generate report using allure-gradle@2.5

We are not updating to the newest versions because of that behavior and we would love to avoid forking and changing this behavior in forked Allure projects.

@MindaugasMateika
Copy link
Author

Not relevant to me anymore.

@AutomatedOwl
Copy link

@MindaugasMateika I have faced the same issue as yours. Have you found another solution which does not involve code changes in allure? Thanks

@MindaugasMateika
Copy link
Author

MindaugasMateika commented Jul 2, 2019

@AutomatedOwl I've started using TestNG's method selector. Either way, I was using custom annotation with parameters to tell which tests should not be run during runtime. Something like this:

@Skip(shouldBeSkipped = someCondition)
@Test
fun test() {
...
}

Before I was using annotation transformer to add enabled = false to @Test annotation based on @Skip annotation's parameters. But now I exclude tests which should be skipped from the list of runnable tests.

This is my method selector:

class MethodSelector : IMethodSelector {
    override fun includeMethod(context: IMethodSelectorContext?, method: ITestNGMethod, isTestMethod: Boolean): Boolean {
        var shouldRunTest = true
        val testMethod = method.constructorOrMethod.method

        if (isTestMethod && testMethod.isAnnotationPresent(Skip::class.java)) {
            val skipParams = testMethod.getAnnotation(Skip::class.java)

            if (skipParams.shouldBeSkipped) {
                context!!.isStopped = true
                shouldRunTest = false
            }
        }
        return shouldRunTest
    }

    override fun setTestMethods(testMethods: List<ITestNGMethod>) {}
}

You can achieve the same thing with method interceptor but it does require TestNG 7.0-beta3 to work properly I think. Previous versions have a bug.

Hope this helps. :-)

@snowymike
Copy link

is there a way to reopen this? i'd also like this behavior to avoid seeing skipped tests in the results without needing to add custom annotations.

@snowymike
Copy link

snowymike commented Feb 12, 2020

sharing some details on why this is relevant in my environment and possibly in others.

we use the

@Test(enabled=false)

or

@Test(enabled = runTests)

to support a few scenarios

  1. the test is an example test on how to do something, which shouldn't be run in test jobs but can easily be run by toggling the enabled setting manually in a class file
  2. the tests in a class are only meant to be run manually after some environmental configuration has been completed. once it has, the user can toggle runTests and run the entire class in an IDE

before upgrading allure, these tests never appeared in any report. now they do - the purple in the image reflects 11 tests with enabled=false. that throws off the pass percentage number, which was 100% passing before upgrading allure.

image

if support is added for ignoring these tests completely in test results, we'd switch to that approach. if that support is not added, we'd likely switch all occurrences of the above patterns to

// @Test

we can do that, it's just not ideal, and i suspect we aren't the only ones that liked to set enabled = false and know they'd be ignored in allure reports.

@MindaugasMateika
Copy link
Author

I've reopened the issue. I hope someone will take a look at this and fix it :-)

@snowymike
Copy link

@MindaugasMateika thanks!

@prashanth56
Copy link

@baev Is there any solution to the above mentioned scenario?

@snowymike
Copy link

snowymike commented Nov 4, 2021

any plans to fix this one? at the moment the workarounds i've seen discussed in this thread are

  1. commenting out @Test rather than doing @Test(enabled = false)
  2. creating a custom annotation, like @Skip, as @MindaugasMateika did

i'd still prefer to use enabled = false, which used to work fine with allure, but if there are no plans to ever support this i'll likely switch everything over to // @Test

@Dilshat517
Copy link

is this issue still open? no solutions yet?

@vinipx
Copy link
Contributor

vinipx commented Sep 26, 2022

Any chance of having this issue prioritized and implemented?
Unfortunately, it is affecting my reporting structure and therefore client is not willing to upgrade Allure version.
Note: the recommended approach here is not approved as good permanent solution.

I'd be happy to contribute and help implementing in case support could be provided :)

@baev
Copy link
Member

baev commented Sep 27, 2022

I'd be happy to contribute and help implementing in case support could be provided :)

👍 I'm ok with that. A few thoughts:

  1. Introduce a new configuration POJO that goes to AllureTestNg class as a constructor parameter
  2. For default constructor use io.qameta.allure.util.PropertiesUtils#loadAllureProperties to load properties and required options
  3. New properties should be under allure.testng. namespace.

@vinipx
Copy link
Contributor

vinipx commented Sep 27, 2022

I'd be happy to contribute and help implementing in case support could be provided :)

👍 I'm ok with that. A few thoughts:

  1. Introduce a new configuration POJO that goes to AllureTestNg class as a constructor parameter
  2. For default constructor use io.qameta.allure.util.PropertiesUtils#loadAllureProperties to load properties and required options
  3. New properties should be under allure.testng. namespace.

Cool! I will take a look at it.
@baev by any chance, do we have similar configurable implementation in place to be used as an example to speed-up things?

Thanks, mate.

@baev
Copy link
Member

baev commented Sep 27, 2022

@baev by any chance, do we have similar configurable implementation in place to be used as an example to speed-up things?

not sure. Simply add desired field to AllureTestNG, create a AllureTestNgConfig POJO with the same field, add a constructor to AllureTestNG that accepts AllureTestNgConfig, update default constructor so it load allure properties and init AllureTestNgConfig from property values and passes config to other constructor. That's should be it. And don't forget a test for that

@vinipx
Copy link
Contributor

vinipx commented Sep 27, 2022

@baev there was this #49 issue years ago fixed to display disabled tests as skipped.
Since we're changing this logic to make it configurable, is it fine if I refactor existing test by replacing it with this #369 here?

@vinipx
Copy link
Contributor

vinipx commented Sep 27, 2022

@baev MR is ready. Please, review whenever you have some time.
All changes and suggestions are welcome - including conventions, standards, style etc...

@baev baev closed this as completed in 0ae1c29 Nov 1, 2022
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

No branches or pull requests

7 participants