gradle / gradle

Adaptable, fast automation for all
https://gradle.org
Apache License 2.0
16.67k stars 4.66k forks source link

JUnit XML report merges unrelated parameterized tests #23324

Open fknittel opened 1 year ago

fknittel commented 1 year ago

Expected Behavior

Given the following JUnit 5 test class with more than one @ParameterizedTests method:

class FailingParameterizedTest {
    static Stream<String> parameters() {
        return Stream.of("firstParameter", "secondParameter");
    }

    @ParameterizedTest
    @MethodSource("parameters")
    void firstMethod(String parameter) {
        fail("first method failing with parameter " + parameter);
    }

    @ParameterizedTest
    @MethodSource("parameters")
    void secondMethod(String parameter) {
        fail("second method failing with parameter " + parameter);
    }
}

The resulting JUnit XML report should contain test results that reference the specific test method. When using junitXml.mergeReruns.set(true) test failures/success from one parameterized test method should not be merged with other parameterized test methods - even if the parameter values used for both methods overlap.

Something along the following lines would be useful:

<?xml version="1.0" encoding="UTF-8"?>
<testsuite name="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" tests="4" skipped="0" failures="4" errors="0">
    <testcase name="firstMethod(String)[1] firstParameter" classname="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" time="0.031">
        <failure message="org.opentest4j.AssertionFailedError: first method failing with parameter firstParameter"
                 type="org.opentest4j.AssertionFailedError"/>
    </testcase>
    <testcase name="firstMethod(String)[2] secondParameter" classname="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" time="0.003">
        <failure message="org.opentest4j.AssertionFailedError: first method failing with parameter secondParameter" type="org.opentest4j.AssertionFailedError"/>
    </testcase>
    <testcase name="secondMethod(String)[1] firstParameter" classname="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" time="0.031">
        <failure message="org.opentest4j.AssertionFailedError: second method failing with parameter firstParameter"
                 type="org.opentest4j.AssertionFailedError"/>
    </testcase>
    <testcase name="secondMethod(String)[2] secondParameter" classname="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" time="0.003">
        <failure message="org.opentest4j.AssertionFailedError: second method failing with parameter secondParameter"
                 type="org.opentest4j.AssertionFailedError"/>
    </testcase>
</testsuite>

The HTML report isn't perfect, but allows the tests to be differentiated based on the "Method name" column: image

A tree-based view would be even better, but I know that's not possible with the current JUnit XML report and Gradle's class-focused reporting (yet): image

Current Behavior

The resulting JUnit XML report appears to only mention the name of the test tree "leaf" - in the case of the parameterized test this cobbles up the test method name, and does not mention the method name. For parameter sets that get re-used for multiple parameterized test methods this is very confusing. When using junitXml.mergeReruns.set(true) this results in test failures/success from one parameterized test method to be merged with other parameterized test methods:

<?xml version="1.0" encoding="UTF-8"?>
<testsuite name="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" tests="4" skipped="0" failures="4" errors="0">
    <testcase name="[1] firstParameter" classname="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" time="0.031">
        <failure message="org.opentest4j.AssertionFailedError: first method failing with parameter firstParameter" type="org.opentest4j.AssertionFailedError"/>
        <rerunFailure message="org.opentest4j.AssertionFailedError: second method failing with parameter firstParameter"
                      type="org.opentest4j.AssertionFailedError"/>
    </testcase>
    <testcase name="[2] secondParameter" classname="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" time="0.003">
        <failure message="org.opentest4j.AssertionFailedError: first method failing with parameter secondParameter" type="org.opentest4j.AssertionFailedError"/>
        <rerunFailure message="org.opentest4j.AssertionFailedError: second method failing with parameter secondParameter"
                      type="org.opentest4j.AssertionFailedError"/>
    </testcase>
</testsuite>

Note that the results from "firstMethod" and "secondMethod" get merged together.

With maven this works fine - the testcase name is filled with the unique firstMethod(String)[1] identifier. This doesn't look nice (the parameter value isn't rendered), but at least it's correct and doesn't potentially misreport the test results.

Context

We have quite a few test classes that use the parameter of parameterized tests to specify an environment context (website names, stage names, URLs, etc.) and have multiple related tests in the same test class that get passed-in the same set of environments (the same parameter values).

An example regarding URLs as parameters would look something like the following:

class ATest {
    static Stream<String> urls() {
        return Stream.of("https://www.example.org/endpoint1", "https://www.exmple.org/endpoint2");
    }

    @ParameterizedTest
    @MethodSource("urls")
    @DisplayName("contains HSTS header")
    void containsHstsHeader(String url) {
        // some assertions on the url
    }

    @ParameterizedTest
    @MethodSource("urls")
    @DisplayName("rejects invalid HTTP methods")
    void rejectsInvalidHttpMethods(String url) {
        // more assertions on the url
    }
}

We could work around the issue by limiting ourselves to one parameterized test method per class or by repeating the method name / a unique identifier in the ParameterizedTest display name (e.g. @ParameterizedTest(name = "methodName[{index}] {argumentsWithNames}")), but as JUnit and our IDEs can cope just fine with the existing test tree (see further up) this isn't our preferred approach and unnecessarily clutters the IDE tree view. (And our code-base is quite large. And it worked prior to our migration from Maven to Gradle ...)

Side note: @DisplayName("foo") and @ParameterizedTest(name = "foo") can be used on the same method at the same time and influence the naming at different hierarchical levels. As mentioned above, Gradle appears to only look at the leafs and the class.

Steps to Reproduce

https://github.com/fknittel/gradle-junit-xml-limitations contains a reproducer. The unexpected XML report can be seen in the GitHub action named "Show test output".

Your Environment

Gradle 7.6, Java 17

fknittel commented 1 year ago

22534 looks to be related, just for @RepeatedTest instead of @ParameterizedTest

jbartok commented 1 year ago

Thank you for providing a valid reproducer.

The issue is in the backlog of the relevant team but the existence of a workaround makes it non-critical so it might take a while before a fix is made.

dibollinger commented 1 year ago

There is another issue present here that is not mentioned in the original report.

If we alter the provided test class, such that only one of the two provided parameters causes the test to fail:

package org.example.gradle.junit.xml.limitations;

import static org.junit.jupiter.api.Assertions.fail;

import java.util.stream.Stream;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

class FailingParameterizedTest {
    static Stream<String> parameters() {
        return Stream.of("firstParameter", "secondParameter");
    }

    @ParameterizedTest
    @MethodSource("parameters")
    void firstMethod(String parameter) {
        if (parameter.equals("firstParameter")) {
            fail("first method failing with parameter " + parameter);
        }
    }

    @ParameterizedTest
    @MethodSource("parameters")
    void secondMethod(String parameter) {
        if (parameter.equals("secondParameter")) {
            fail("second method failing with parameter " + parameter);
        }
    }
}

Then the resulting XML will report the failing test as a <flakyFailure> element, if and only if a failing execution of the same parameterized test was afterwards followed by a successful one.

This means that for the first test method we receive:

 <testcase name="[1] firstParameter" classname="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" time="0.002">
    <flakyFailure message="org.opentest4j.AssertionFailedError: first method failing with parameter firstParameter" type="org.opentest4j.AssertionFailedError">
      <stackTrace>org.opentest4j.AssertionFailedError: first method failing with parameter firstParameter</stackTrace>
    </flakyFailure>
  </testcase>

While for the second test method, we receive:

  <testcase name="[2] secondParameter" classname="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" time="0.002"/>
  <testcase name="[2] secondParameter" classname="org.example.gradle.junit.xml.limitations.FailingParameterizedTest" time="0.003">
    <failure message="org.opentest4j.AssertionFailedError: second method failing with parameter secondParameter" type="org.opentest4j.AssertionFailedError">org.opentest4j.AssertionFailedError: second method failing with parameter secondParameter</failure>
</testcase>

The former XML report is false, while the latter is correct.

The potential result of this is that certain failing tests can go completely unnoticed, as many build environments simply ignore test failures that are denoted as being flaky.

In our project, we had utilized the @RetryingTest annotation to silence certain known flaky tests which had no easy fix available. However, we later discovered that a number of tests using the @ParameterizedTest and@MethodSource annotations had been consistently failing, and because they were falsely being reported as flaky failures in the XML, they went completely unnoticed during the Nightly Build process.

While a workaround is possible by adding a display name to the affected tests, we believe that this is not a sufficient solution to the problem. Most developers will not be aware of this issue, hence they will likely not add a display name to their parameterized test methods, and just use the annotations "as-is".

If they then also use the junitXml.mergeReruns.set(true) option, they will be at risk of having potentially serious bugs go completely unnoticed, even when the associated productive code is thoroughly tested.

fknittel commented 1 year ago

Good point @dibollinger.

As a user of junitXml.mergeReruns.set(true), unless you already know about this issue and apply one of the work-arounds, you might think your test coverage keeps you safe, while in fact... some tests could be sneakily failing.

ov7a commented 4 months ago

See also: