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

throwing SkipException sets iTestResult status to Failure instead of Skip #1632

Closed
3 of 7 tasks
xlenz opened this issue Nov 30, 2017 · 22 comments · Fixed by #2214
Closed
3 of 7 tasks

throwing SkipException sets iTestResult status to Failure instead of Skip #1632

xlenz opened this issue Nov 30, 2017 · 22 comments · Fixed by #2214

Comments

@xlenz
Copy link

xlenz commented Nov 30, 2017

TestNG Version 6.13.1

Expected behavior

previously I had in beforeInvocation:

throw new SkipException("Skipped due to previous failure of: " + failedTestName);

So I have TestResult.SKIP in afterInvocation.

Discussion started here: #1611

Actual behavior

After upgrading to 6.13.1 it stopped working, because iTestResult.getStatus() is now TestResult.FAILURE instead of TestResult.SKIP in afterInvocation.

to workaround this I'm setting status explicitly before throwing SkipException:

iTestResult.setStatus(TestResult.SKIP);
throw new SkipException("Skipping the test case");

Is the issue reproductible on runner?

  • Shell
  • Maven
  • Gradle
  • Ant
  • Eclipse
  • IntelliJ
  • NetBeans

Test case sample

// listener
public class TestNGExecutionListener implements IInvokedMethodListener {
    @Override
    public void beforeInvocation(IInvokedMethod iInvokedMethod, ITestResult iTestResult) {
        throw new SkipException("skip");
    }

    @Override
    public void afterInvocation(IInvokedMethod iInvokedMethod, ITestResult iTestResult) {
        iTestResult.getStatus(); // actual Failure (2), expected Skip (3)
    }
}

// add listener
testng.addListener((ITestNGListener) new TestNGExecutionListener());
@ghost
Copy link

ghost commented Apr 24, 2019

Setting the iTestResult.setStatus(TestResult.SKIP) before throwing the SkipException in the #beforeInvocation method causes me to see failures if the method is a BeforeXXX.

@klubi
Copy link

klubi commented Dec 30, 2019

Is there a plan to fix this, or provide a mechanism that would allow skipping tests via configuration method? Above workaround seems to not work (I'm currently on version 7.0.0).
I'd like to skip tests, not disable them with @Test(enabled=false). Outcome is similar, but in terms of reporting (ex system tests) difference is huge.

krmahadevan added a commit to krmahadevan/testng that referenced this issue Jan 1, 2020
krmahadevan added a commit to krmahadevan/testng that referenced this issue Jan 1, 2020
krmahadevan added a commit that referenced this issue Jan 3, 2020
@klubi
Copy link

klubi commented Jun 9, 2020

Hi

I see this was fixed, and merged to master. Any chance this get's released anytime soon?

@krmahadevan
Copy link
Member

ping @cbeust - Please chime in.

@klubi
Copy link

klubi commented Aug 17, 2020

@krmahadevan Can I get a confirmation this is fixed in 7.3.0 release?
I can still see tests being marked FAILED instead of SKIP when listener throws SkipException.

@krmahadevan
Copy link
Member

@klubi - Are you seeing this not working in 7.3.0 ? This version has been released.

@klubi
Copy link

klubi commented Aug 17, 2020

@krmahadevan I'm on 7.3.0, and when my listeners throw SkipException tests are still marked as Failed.

When gradle logging is enabled I see something like this:

11:47:59  Test Suite >  Tests > com.aaa.bbbTest > shouldDoSomething FAILED
11:47:59      org.testng.SkipException
11:47:59  
11:47:59  Test Suite >  Tests > com.aaa.bbbTest > shouldDoSomething SKIPPED

and then I see two records for same method in html test report
image

@krmahadevan
Copy link
Member

@klubi - Do you have a sample that you can share so that we can investigate on this ?

@klubi
Copy link

klubi commented Aug 17, 2020

@krmahadevan let me prepare one

@klubi
Copy link

klubi commented Aug 17, 2020

@krmahadevan
https://github.com/klubi/vsc-testng7-sample

I re-used sample project I created for issue with VS Code.
I created ShouldSkipTest class, and testSuite.xml, and wrapped it with runSkiptest gradle task.

@klubi
Copy link

klubi commented Jan 14, 2021

@krmahadevan any update on this?
It seems to still be an issue.

@sbrannen
Copy link

I believe this is effectively a duplicate of spring-projects/spring-framework#26387.

See spring-projects/spring-framework#26387 (comment) for further details.

@krmahadevan
Copy link
Member

@klubi - I haven't gotten around to looking at this issue yet. The last few months have been extremely hectic at work front. I hope to get things sorted out at work front in a couple of weeks after which I should have some breather time. Transitioning into a full fledged backend developer at work with zero knowledge on Spring is taking a toll on my time, while still juggling the current role's responsibilities.

@juherr
Copy link
Member

juherr commented Jan 16, 2021

@krmahadevan No need to learn Spring. You can have a look at the test cases I added at #2455. They are based on the sample provided by @sbrannen

@krmahadevan
Copy link
Member

@juherr - Lol.. NO that was not what I meant. I was meaning to say that since at work, I am trying to transition into a backend developer, I am having to invest a lot of time trying to figure out things in Spring etc., and hence I haven't been able to find a lot of time to get to take a look at this issue. btw thanks for adding that sample. I will take it up from there hopefully over this weekend.

@juherr
Copy link
Member

juherr commented Jan 16, 2021

@krmahadevan Oops, sorry I didn't catch it. BTW I've just added some new test cases. I think there are some relations with #2423 too.

krmahadevan added a commit to juherr/testng that referenced this issue Jan 19, 2021
@sbrannen
Copy link

@krmahadevan, thanks for pushing the fix for this!

I don't see a milestone assigned. Is this expected to be included in a 7.3.x release?

@krmahadevan
Copy link
Member

@sbrannen - 7.3.0 is already released. This should be part of 7.4.0 (which would be the next upcoming release for TestNG)

@juherr - I think we should start leveraging the milestone capability in github so that our users exactly know as to when would a fix be available. Any thoughts on how to go about doing this ?

@krmahadevan krmahadevan added this to the 7.4.0 milestone Jan 21, 2021
@krmahadevan
Copy link
Member

@sbrannen - Thank you for bringing up this. I will start adding this milestone for all the issues that would be part of 7.4.0 going forward.

@juherr
Copy link
Member

juherr commented Jan 21, 2021

I think we should start leveraging the milestone capability in github so that our users exactly know as to when would a fix be available. Any thoughts on how to go about doing this ?

@krmahadevan Good idea 👍

@krmahadevan
Copy link
Member

@juherr - As a first step, i went ahead and added 7.4.0 as a milestone to all defects that we have fixed so far after we released 7.3.0. Going forward when we close an issue, we should ensure we update the milestone to whatever is the current upcoming version. That way this mapping would be there all the time.

@sbrannen
Copy link

Glad to you see adopting GitHub milestones.

The community thanks you! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants