use displayName instead of legacyReportingName in xml reports

[mguillem]

Allows to have the configured informative test names in the reports instead of the test method signature (like 1: fib(0) = 0, ..., 7: fib(6) = 8 instead of fibonacci(int, int)[1], ..., fibonacci(int, int)[7].

See discussion in code : https://github.com/apache/ant/commit/5981e1bff1e095bf85ee4d6565bc0c7ff93f2bb6#diff-f985f89ce4779eff2cdcf164d09d5396R246

I was not able to run successfully all tests locally as desired. With some hacks I was able to run JUnitLauncherTaskTest with ant junit-single-test -Djunit.testcase=org.apache.tools.ant.taskdefs.optional.junitlauncher.JUnitLauncherTaskTest to prepare this patch but can't be sure that the changes don't break something else.

[jaikiran]

Thank you for creating this PR. Looks like our CI hasn't been picking up PR related runs. I'll run some tests manually and see how it goes.

[mguillem]

@jaikiran Is this PR ok for you? I'm waiting for this to be merged (or rejected) to submit a new PR with some common changes.

[jaikiran]

Hello @mguillem, please give me a few days on this one. I have tested this locally and do see some issues with the changes (not that we won't be able to solve them). However, I am in the middle of few other things these days so in order to get to this code and explain the issue, I will need a few more days. I'm sorry about that.

[jaikiran]

Hello @mguillem, sorry to keep you waiting and thank you for the patience.

Here's more details on why I used legacyReportingName when I fixed https://bz.apache.org/bugzilla/show_bug.cgi?id=63680 and why the current proposed change can cause issues.

Please consider the following testcase:

package org.myapp;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.junit.jupiter.api.RepeatedTest;

public class SimpleTest {

	@ParameterizedTest
	@ValueSource(strings = { "one", "two", "three" })
	public void testIt(String name) throws Exception {
		System.err.println(name);
	}

	@RepeatedTest(value = 3)
	public void testRep() {
		System.err.println("hello");
	}

	@RepeatedTest(value = 3)
	public void testRep2() {
		System.err.println("hello2");
	}
}

Currently in Ant 1.10.7 (which has that fix for bz-63680), where the legacy reporting name gets used, the generated XML report is of the form:

<testcase classname="org.myapp.SimpleTest" name="testIt(String)[3]" time="0.001"></testcase><testcase classname="org.myapp.SimpleTest" name="testIt(String)[2]" time="0.0"></testcase><testcase classname="org.myapp.SimpleTest" name="testRep2()[2]" time="0.001"></testcase><testcase classname="org.myapp.SimpleTest" name="testRep2()[1]" time="0.0"></testcase><testcase classname="org.myapp.SimpleTest" name="testRep()[2]" time="0.001"></testcase><testcase classname="org.myapp.SimpleTest" name="testRep()[1]" time="0.001"></testcase><testcase classname="org.myapp.SimpleTest" name="testRep()[3]" time="0.0"></testcase><testcase classname="org.myapp.SimpleTest" name="testRep2()[3]" time="0.0"></testcase><testcase classname="org.myapp.SimpleTest" name="testIt(String)[1]" time="0.028"></testcase>

and the HTML looks like this image https://home.apache.org/~jaikiran/ant-1.10.7.png

Now with the proposed change in this PR, the generated XML report looks like:

<testcase classname="org.myapp.SimpleTest" name="[3] three" time="0.001"></testcase><testcase classname="org.myapp.SimpleTest" name="[2] two" time="0.0"></testcase><testcase classname="org.myapp.SimpleTest" name="repetition 2 of 3" time="0.001"></testcase><testcase classname="org.myapp.SimpleTest" name="repetition 1 of 3" time="0.0"></testcase><testcase classname="org.myapp.SimpleTest" name="repetition 2 of 3" time="0.001"></testcase><testcase classname="org.myapp.SimpleTest" name="repetition 1 of 3" time="0.0"></testcase><testcase classname="org.myapp.SimpleTest" name="repetition 3 of 3" time="0.001"></testcase><testcase classname="org.myapp.SimpleTest" name="repetition 3 of 3" time="0.0"></testcase><testcase classname="org.myapp.SimpleTest" name="[1] one" time="0.021"></testcase>

and the report HTML looks like https://home.apache.org/~jaikiran/proposed-change.png

Notice that the names no longer contain the method names and have completely lost context of which method is being repeat tested or parameterized. The version in 1.10.7 has the necessary information about the methods being parameterized as well as repeated, so I believe we should continue to use the way it is in 1.10.7.

[mguillem]

Hi @jaikiran, I understand the problems with the usage of displayName.

What do you think of these first ideas to solve this issue without the problems you mention:

• use something like ${legacyReportingName}: ${displayName} as test name, when legacyReportingName and displayName differ (I haven't yet verified when it is the case)
• alternative: add an option to the JUnitLauncher task to specifiy which name should be used

[jaikiran]

alternative: add an option to the JUnitLauncher task to specifiy which name should be used

Yes, I had this in mind but hadn't seen a necessity yet. But I think it makes sense to let users decide what names they want reported (not just for this formatter, but other ones as well). Can you file a bugzilla enhancement request for this?

[simon04]

This PR should resolve the following bug: Bug 64564 - JUnitLauncher Task should support @ParameterizedTest (custom display name)

[don-vip]

@mguillem @jaikiran What needs to be done to see this issue fixed? Can we help?

[mguillem]

@don-vip using display names can't be made the default because it would probably break existing usage. This means that an opt-in possibility needs to be created. I don't know what would be the best way for that. Perhaps a new Test result formatter that differs from legacy-xml only by using displayName? I will probably not work on that as we're not using Ant so much anymore for the execution of our unit tests.

[jaikiran]

@mguillem @jaikiran What needs to be done to see this issue fixed? Can we help?

Hello @don-vip, I think allowing the users to (optionally) decide which name to display is the simplest way to deal with this. I'll add some support to this.

[jaikiran]

I've pushed a commit[1] which adds support to configure which style of names are used for reporting. More specifically, the listener nested element of the junitlauncher can now be set with useLegacyReportingName attribute[2] to either use new style or the legacy style display name.

[1] https://github.com/apache/ant/commit/41c51907a42c5c48435197031ad5f9bea81c1032 [2] https://github.com/apache/ant/commit/41c51907a42c5c48435197031ad5f9bea81c1032#diff-31c99ebd4075f752d6d28ecd2809d7e8f73abec81a0a11a4625b383708a4f0deR405

[don-vip]

@jaikiran awesome thank you so much! We'll try this new feature asap :)

[jaikiran]

Hello @don-vip, we do publish our nightly builds[1], so if you want to give this a try before we do an official release (no date set yet), then please use the nightly build binary and let us know if it solves your issue.

[1] https://ci-builds.apache.org/job/Ant/job/Ant_Nightly/lastSuccessfulBuild/artifact/

[don-vip]

@jaikiran I tried to build Ant myself and face the following error when using it: [junitlauncher] Error: Could not find or load main class org.apache.tools.ant.taskdefs.optional.junitlauncher.StandaloneLauncher

Indeed the built ant-junitlauncher.jar does not contain this class, it only contains classes in the package org/apache/tools/ant/taskdefs/optional/junitlauncher/confined.

I see your build.xml has a lot of code regarding this confined package but I don't understand it, is there something to do when building ant to get the StandaloneLauncher?

[don-vip]

@jaikiran I tried https://ci-builds.apache.org/job/Ant/job/Ant_Nightly/lastSuccessfulBuild/artifact/distribution/binaries/apache-ant-1.10.10alpha-bin.zip and got a different error:

Problem: failed to create task or type junitlauncher Cause: the class org.apache.tools.ant.taskdefs.optional.junitlauncher.JUnitLauncherTask was not found.

Indeed the ant-junitlauncher.jar contains this class in the confined package: org/apache/tools/ant/taskdefs/optional/junitlauncher/confined/JUnitLauncherTask.class

EDIT: I had to force an additional -lib argument to make it work, build is in progress now.

EDIT 2: It works perfectly! Thank you @jaikiran ! I guess you can close https://bz.apache.org/bugzilla/show_bug.cgi?id=64564 as fixed and mention it in release notes.

[don-vip]

@jaikiran I was speaking a bit too fast, there's a minor problem, the test method is missing. For this example test:

class TaggingPresetPreferenceTestIT extends AbstractExtendedSourceEntryTestCase {

    public static List<Object[]> data() throws Exception {
        return getTestParameters(new TaggingPresetPreference.TaggingPresetSourceEditor().loadAndGetAvailableSources());
    }

    @ParameterizedTest(name = "{0} - {1}")
    @MethodSource("data")
    void testPresetsValidity(String displayName, String url, ExtendedSourceEntry source) throws Exception {
    }

The output for an error is:

org.openstreetmap.josm.gui.preferences.map.TaggingPresetPreferenceTestIT.Persian Presets - https://github.com/DearRude/IranianPresets/blob/master/zip/latast.zip

While it was the following for JUnit 4:

org.openstreetmap.josm.gui.preferences.map.TaggingPresetPreferenceTestIT.testPresetsValidity[Persian Presets - https://github.com/DearRude/IranianPresets/blob/master/zip/latast.zip]

Can we please get the same output as JUnit 4? Test.testMethod[displayedName]

[jaikiran]

Hello @don-vip, sorry I've been offline for a few days. I'll get back to this as soon as I'm back online.

[jaikiran]

Hello Vincent @don-vip,

Can we please get the same output as JUnit 4? Test.testMethod[displayedName]

I had forgottent about this. But now that I went backed and looked at it, this should be easily customized using the @ParameterizedTests name attribute. I have now added an example to Ant testsuite to show how it's done. See this https://github.com/apache/ant/blob/master/src/tests/junit/org/example/junitlauncher/jupiter/JupiterSampleTest.java#L80 which uses:

    @ParameterizedTest(name = "{displayName} asserting {0} is an even number")
...
    void testEvenFails(final int val) {

and then in the junitlauncher's task:

<junitlauncher>
...
    <listener type="legacy-xml" ...
                   useLegacyReportingName="false" />
...

which then reports the test as:

[jaikiran]

I'll go ahead and close this PR since this was already delivered in Ant 1.10.10 release as part of https://bz.apache.org/bugzilla/show_bug.cgi?id=64564