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

[!shouldfail] tag does not produce a failure when no assertions fail #620

Closed
gkm4d opened this issue Mar 20, 2016 · 3 comments
Closed

[!shouldfail] tag does not produce a failure when no assertions fail #620

gkm4d opened this issue Mar 20, 2016 · 3 comments
Labels

Comments

@gkm4d
Copy link

gkm4d commented Mar 20, 2016

When the [!shouldfail] tag it utilized a test will be marked as a failure when no assertions fail.

For example, I would expect the below example to be marked as a failure because the lone assertion passes, but a failure was expected:

TEST_CASE("foo", "[!shouldfail]")
{
    REQUIRE(true);
}

But instead I get: All tests passed (1 assertion in 1 test case). This is with v1.3.5.

See discussion at: https://groups.google.com/forum/#!topic/catch-forum/0h4McyyKKz4

@philsquared
Copy link
Collaborator

Thanks, @gkm4d.
Could you try 1.4.0. I jumped the gun and went ahead and fixed it based on your forum report :-)

@gkm4d
Copy link
Author

gkm4d commented Mar 25, 2016

@philsquared appreciate the quick fix! The behavior with 1.4.0 is now as I expected.

I will pass along a couple of observations related to JUnit output. Consider this test:

TEST_CASE("foo", "[foo][!shouldfail]")
{
    REQUIRE(true);
}

which produces:

<testsuites>
  <testsuite name="test_foo.exe" errors="0" failures="1" tests="2" hostname="tbd" time="0.001793" timestamp="tbd">
    <testcase classname="global" name="foo" time="0.000052"/>
    <system-out/>
    <system-err/>
  </testsuite>
</testsuites>

I was a bit confused by 2 tests being reported. Stepping though the code I see this is due to the introduction of deltaTotals.assertions.failed++ in the new logic block in catch_run_context.hpp. Seems this is necessary to ensure at least one assertion failure is noticed. Perhaps you could instead say something like: deltaTotals.assertions.failed = deltaTotals.assertions.passed; deltaTotals.assertions.passed=0 to keep the count correct? Regardless, I think this is a minor issue and tricky since "tests" generally seems to be keeping track of assertions, but here you are marking a test failure regardless of assertion failures.

The second observation is, I think, a little more problematic. It may have actually have been an existing problem, and I can add a new ticket if necessary. Consider the slightly modified test:

TEST_CASE("foo", "[foo][!shouldfail]")
{
    REQUIRE(true);
    throw 1;
}

which produces:

<testsuites>
  <testsuite name="test_foo.exe" errors="1" failures="18446744073709551615" tests="2" hostname="tbd" time="0.002008" timestamp="tbd">
    <testcase classname="global" name="foo" time="0.0">
      <error message="{Unknown expression after the reported line}">
Unknown exception
at ..\src\test_foo.cpp(15)
      </error>
    </testcase>
    <system-out/>
    <system-err/>
  </testsuite>
</testsuites>

Notice the very large number of failures. Here, a failure is counted due to the exception, but due to the [!shouldfail] this gets switched to a failedButOk. in JunitReporter::writeGroup() the failure count is computed as stats.totals.assertions.failed-unexpectedExceptions. Here this is 0-1, which wraps around to the very large unsigned integer. I'm not sure of a robust solution here, other than some obvious check to catch this case and clip the number of failures to 0.

Thanks again for the quick fix!

@horenmar horenmar added Resolved - pending review Issue waiting for feedback from the original author Revisit labels Feb 19, 2017
@horenmar
Copy link
Member

The issue in title is solved, so I am going to close this soon.

I am also going to open up a new issue with the other problems found (hopefully soon 😄 ).

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

No branches or pull requests

3 participants