jest-junit icon indicating copy to clipboard operation
jest-junit copied to clipboard

output xml does not contain failed tests if jest.retryTimes was used

Open justinaszid opened this issue 4 years ago • 5 comments

Hello,

I am facing a very strange issue which I cannot get hang of.

I am using jest.retryTimes to rerun failed tests, but in doing so, I noticed, that the output xml file does not contain tests which failed even after retry. I can see in it default ouput, but not in the xml. Am I missing some settings of these two (retry and jest-junit) does not match well?

Thank you in advance!

justinaszid avatar Jan 11 '21 16:01 justinaszid

Hello!

Please create a repository that recreates this issue consistently along with what results you expect to see.

Once you have that I'll take a look.

Thank you

palmerj3 avatar Jan 11 '21 18:01 palmerj3

Hi @palmerj3,

I've created a repo demonstrating this issue.

The context is that jest added support for jest.retryTimes to automatically retry individual tests in the event of failure. This is great for smoothing over flaky tests in large codebases.

The default jest json reporter outputs information on retried tests, e.g. (see output.json)

{
  "testResults": [{
    "assertionResults": [{
      "failureDetails": [],
      "failureMessages": [],
      "retryReasons": ["Error: ..."],
      "status": "passed"
      "title": "flakes"
    }]
  }]
}

However, this information is not present in jest-junit: (see junit.xml)

<testsuites>
  <testsuite failures="0">
    <testcase name="flakes">
    </testcase>
  </testsuite>
</testsuites>

We'd like to ingest the XML for flake detection, but for now we need to disable jest.retryTimes and re-run the entire test suite on failure, then take both XML from both runs. It'd be great if jest-junit interpreted these retryReasons from jest and output them in the XML.

jayrobin avatar Feb 11 '23 00:02 jayrobin

I'm the one who wrote jests test retry feature :) Fun to revisit this.

The primary issue here is that there is no one perfect way to represent test retries in junit. If we added a test case in the junit file for each attempt, then some folks using jest-junit would receive failures in their CI system because of a failing test case even though the actual test in the test runner passed.

This is because a non-zero number of folks have their CI system configured to parse junit.xml to understand if there were failures vs simply relying on exit code.

And the junit spec does not allow testcase properties so we couldn't simply add on a property saying how many executions a test has had. And for that same reason we can't just add an attribute to the testcase type because that is also not in the spec.

Everything jest-junit does is comform to the jenkins junit spec.

I would be open to someone creating a PR which enables any of the approaches above based on a configuration option.

palmerj3 avatar Feb 15 '23 23:02 palmerj3

I'm the one who wrote jests test retry feature :) Fun to revisit this.

Thanks for that, it's an awesome feature!

I agree on your concerns around not wanting to go "off-spec" and the potential impact on CI systems. Putting it behind a config flag (showRetries?) definitely makes sense.

I'm happy to take a stab at this, if we can find an approach that's workable. It sounds like there are a couple of options, each with pros and cons:

  1. Repeat test case entries in the event of retry. This would still conform to the spec in terms of shape, but the testsuite attributes (e.g. tests, failures) would no longer correspond to testcase nodes. Also, as you mentioned, this might cause CI issues when parsing the XML rather than relying on exit code (maybe less of an issue if it's behind a config flag?).
<testsuites>
  <testsuite tests="1" failures="0" errors="0">
    <testcase name="flakes">
      <!-- flaky test failed -->
      <failure>Error: expect(received).toBe(expected)</failure>
    </testcase>
    <testcase name="flakes">
      <!-- flaky test failed -->
      <failure>Error: expect(received).toBe(expected)</failure>
    </testcase>
    <testcase name="flakes">
      <!-- flaky test finally passed -->
    </testcase>
  </testsuite>
</testsuites>
  1. Add custom nodes/attributes within testcase to indicate retries. Wouldn't correspond to the spec, but less impactful to existing systems: the basic nodes and attributes are unaffected.
<testsuites>
  <testsuite tests="1" failures="0" errors="0">
    <testcase name="flakes">
      <retryreasons>
        <retryreason>
          <!-- flaky test failed -->
          Error: expect(received).toBe(expected)
        </retryreason>
        <retryreason>
          <!-- flaky test failed -->
          Error: expect(received).toBe(expected)
        </retryreason>
      </retryreasons>
    </testcase>
  </testsuite>
</testsuites>

Let me know what you think, or if you have a specific approach in mind.

jayrobin avatar Feb 16 '23 00:02 jayrobin

I guess the approach should depend on what information you're trying to get out of a test run.

Do you simply want to know which tests were retried? If so we could just add an attribute to "testcase" so:

<testcase retries=3> Or if you want to know the details of test failures for each run then I would probably go with something like:

<testsuites>
  <testsuite tests="1" failures="0" errors="0">
    <testcase name="flakes">

    </testcase>
    <testcase name="flakes_retry1">

    </testcase>
    <testcase name="flakes_retry2">

    </testcase>
  </testsuite>
</testsuites>

palmerj3 avatar Feb 16 '23 01:02 palmerj3