Junit Reporter breaks when BDD Sections fail

[dbak-invn]

Describe the bug When a failure occurs, all previously-occurring tests within the same top-level section have their names messed up in a particular way. For example, the following test:

SCENARIO("This is a fake test", "[fake]"){
    GIVEN("Prior given"){
        REQUIRE(true);
        THEN("Prior Given Inner"){
            REQUIRE(true);
        }
    }
    GIVEN("Given1"){
        THEN("Beep Boop"){
            REQUIRE(true);
        }
        WHEN("When1"){
            THEN("Success"){
                REQUIRE(true == true);
            }
            THEN("PRIOR WITH INNER"){
                AND_THEN("inner prior"){
                    REQUIRE(true);
                }
            }
            THEN("FAIL"){
                AND_THEN("SUCCEED INNER"){
                    REQUIRE(true);
                }
                AND_THEN("FAIL INNER") {
                    FAIL("FAILURE");
                }
            }
            THEN("THEN AFTER FAILURE"){
                AND_THEN("inner after") {
                    REQUIRE(true);
                }
            }
            THEN("LAST"){
                REQUIRE(true);
            }
        }
    }
    GIVEN("Given 2"){
        REQUIRE(true);
    }
}
  THEN("FAIL"){
                AND_THEN("FAIL INNER") {
                    FAIL("FAILURE");
                }
            }
            THEN("THEN AFTER FAILURE"){
                AND_THEN("inner after") {
                    REQUIRE(true);
                }
            }
            THEN("LAST"){
                REQUIRE(true);
            }
        }
    }
    GIVEN("Given 2"){
        REQUIRE(true);
    }
}

Produces the following junit xml output:

<testsuites>
<testsuite name="flamenco_algorithms_anomaly_stiction_tests" errors="0" failures="1" tests="10" hostname="tbd" time="0.000328" timestamp="2019-06-04T01:09:41Z">
<properties>
<property name="filters" value="[fake]"/>
</properties>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Prior given" time="0.000029"/>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Prior given/Then: Prior Given Inner" time="0.000002"/>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/And: FAIL INNER/Then: Beep Boop" time="0.000001"/>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/And: FAIL INNER/Then: FAIL/Then: Success" time="0.000003"/>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/And: FAIL INNER/Then: FAIL/Then: PRIOR WITH INNER/And: inner prior" time="0.000001"/>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/And: FAIL INNER/Then: FAIL/When: When1/And: SUCCEED INNER" time="0.000001"/>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/And: FAIL INNER/Then: FAIL/When: When1/Given: Given1" time="0.000071">
<failure type="FAIL">
FAILURE at /cygdrive/c/Users/dbak/Projects/firmware/unit-tests/FlaAlgoTests/flamenco_algorithms_anom_stict_tuning_tests.cpp:111
</failure>
</testcase>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given1/When: When1/Then: THEN AFTER FAILURE/And: inner after" time="0.000001"/>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given1/When: When1/Then: LAST" time="0.000001"/>
<testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given 2" time="0.000021"/>
<system-out/>
<system-err/>
</testsuite>
</testsuites>

So everything after the failure is fine, and everything before the root of the failure is okay. But among all of the leafs occurring before and including the failing leaf, there seem to be lots of problems. All of them have the failing leaf's description prepended to their path, but then their own path seems to be reversed or jumbled up - I can't figure out a pattern to it.

This is a big problem for us as it might mean we need to move away from BDD-style tests and change our existing ones - all of our test reports have become totally unreadable because of this.

Platform information:

• OS: Windows 10
• Compiler+version: g++
• Catch version: 2.8.0

[dbak-invn]

When I redefine the BDD-style sections as so:

#define GIVEN( desc )     INTERNAL_CATCH_SECTION( "    Given: "s += std::string(desc) )
#define AND_GIVEN( desc ) INTERNAL_CATCH_SECTION( "And given: "s += std::string(desc) )
#define WHEN( desc )      INTERNAL_CATCH_SECTION( "     When: "s += std::string(desc) )
#define AND_WHEN( desc )  INTERNAL_CATCH_SECTION( " And when: "s += std::string(desc) )
#define THEN( desc )      INTERNAL_CATCH_SECTION( "     Then: "s += std::string(desc) )
#define AND_THEN( desc )  INTERNAL_CATCH_SECTION( "      And: "s += std::string(desc) )

This does not fix the bug - I saw another issue claim that a similar problem was caused by dynamic sections.

Also, neither the XmlReporter nor the console reporter nor the compact reporter have the same issue; so this is an issue with CumulativeReporterBase or junit.

[dbak-invn]

I have found that replacing all REQUIRES with CHECK fixes this issue. I think the problem is that Cumulative Reporter relies on sections ending properly. However, if there is a not-ok fail, then the section doesn't end properly, and is left "hanging", and I think this causes the higher-level node to get the fail data (since the fail data wasn't consumed by the section that failed) - this is based on my limited understanding of the code from debugging, and hopefully it's useful to whoever can actually fix this bug.

I will try to do so if I can find the time, but it seems like a fairly fundamental problem with any cumulative reporter and would require a decent amount of work.

Unfortunately this also means that FAIL(), or any exceptions being thrown inside a test, will still break the test. So replacement with CHECK is not a sufficient workaround.

[dbak-invn]

Here is the xml output of the example test if FAIL is replaced with CHECK_FALSE(true):

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="flamenco_algorithms_anomaly_stiction_tests" errors="0" failures="1" tests="10" hostname="tbd" time="0.00049" timestamp="2019-06-04T22:35:57Z">
    <properties>
      <property name="filters" value="[fake]"/>
    </properties>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Prior given" time="0.000093"/>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Prior given/Then: Prior Given Inner" time="0.000004"/>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given1/Then: Beep Boop" time="0.000003"/>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given1/When: When1/Then: Success" time="0.000008"/>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given1/When: When1/Then: PRIOR WITH INNER/And: inner prior" time="0.000003"/>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given1/When: When1/Then: FAIL/And: SUCCEED INNER" time="0.000002"/>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given1/When: When1/Then: FAIL/And: FAIL INNER" time="0.000003">
      <failure message="!true" type="CHECK_FALSE">
at /cygdrive/c/Users/dbak/Projects/firmware/unit-tests/FlaAlgoTests/flamenco_algorithms_anom_stict_tuning_tests.cpp:126
      </failure>
    </testcase>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given1/When: When1/Then: THEN AFTER FAILURE/And: inner after" time="0.000002"/>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given1/When: When1/Then: LAST" time="0.000002"/>
    <testcase classname="flamenco_algorithms_anomaly_stiction_tests.global" name="Scenario: This is a fake test/Given: Given 2" time="0.000002"/>
    <system-out/>
    <system-err/>
  </testsuite>
</testsuites>