junitlauncher - Support useFile attribute for listeners

[azotcsit]

Context

I'm working on migration of Cassandra to JUnit5 (https://issues.apache.org/jira/browse/CASSANDRA-16630). Our test running code is heavily customized and one of the features we rely on is "useFile" attribute (https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/FormatterElement.java#L80). Unfortunately, junitlauncher task does not have any equivalent. The purpose of this change is to introduce a way to write listener's output to stdout instead of a file.

Behavior

This PR does not change any existing behavior.

Summary of the changes

1. Introduced useFile attribute for ListenerDefinition
2. Updated LauncherSupport to supper the new attribute
3. Updated documentation

[jaikiran]

Hello @azotcsit, the listener element is meant to allow any kind of listener implementations as long as they implement the JUnit 5 platform's API org.junit.platform.launcher.TestExecutionListener. As such the current resultFile is only an optional attribute and applicable to only some of our internal (legacy) formatters.
In other words, even in its current form, a custom implementation of org.junit.platform.launcher.TestExecutionListener can decide where to write out the output generated by the listener and as such I don't think a new attribute is necessary for it.

[azotcsit]

Hi @jaikiran, I'm new to this codebase, so I'm sorry if I missed something.

If we talk hypothetically then not only resultFile attribute applicable to formatters, but also outputDir and extension (not mentioning useLegacyReportingName) because TestExecutionListener does not define any output format and a person may want to write test results to other destination than a file (e.g. an external database).

I feel the current design mixes up TestExecutionListener and TestResultFormatter. On the one hand we want to keep TestExecutionListener clean of anything that limits it, on the other hand we want to be able to pass some arguments to the formatter.

a custom implementation of org.junit.platform.launcher.TestExecutionListener can decide where to write out the output generated by the listener

I agree that it can decide, but according to the current design it is supposed to use TestResultFormatter.setDestination(OutputStream os) and any other way seems to be a hack of the system rather than an expected solution.

With that being said, I have some questions. The answers would help me to understand what changes to bring back to ant and what to keep in custom foratters. Also it will probably prevent me of raising controversial tickets like this one.

Architectural questions:

1. Have you considered to introduce LegacyFormatterDefinition? So whoever want to stick to old formatters approach (I guess most of people) they can easily do that. As a result, ListenerDefinition is kept clean.
2. I feel there is a need to decouple output part of the listener and that should solve all the problems. Smth like that:

<listener class="xml-writer" writerClass="com.ant.junitlauncher.FileWriter">
  <outDir>bla-blah<outDir/>
  <extension>extension<extension/>
</listener>
<listener class="brief" writerClass="com.ant.junitlauncher.StdOutWriter" />

That should help to keep listeners and LauncherSupport clean of any output logic. Also it would let customize output in any way the implementation dictates. WDYT?

Other questions:

1. Is it currently possible pass an attribute to a custom formatter (without it being defined on listener level)? Would smth like system properties work?
2. I understand you consider existing formatters being legacy but I believe they are what people actively use right now. So it would be nice to provide at least comparable functionality to what was available for ant-junit. I definitely can customize TestResultFormatter.setDestination(OutputStream os) and that would solve the problem of writing to stdout, but I'm just wondering whether we need to support it for the community use. Maybe we can just put a "legacy" prefix (legacyUseFily) to the new attribute to emphasize it is something not really right. WDYT?
3. Is there a reason why TestPlan field is not a part of AbstractJUnitResultFormatter class? The field is defined in all existing formatters, I'm just wondering why not to move that because it seems to be applicable to all possible formatters.
4. Is there a reason why useLegacyReportingName is not a part of AbstractJUnitResultFormatter? Since TestResultFormatter has setUseLegacyReportingName method it seems to be reasonable to move useLegacyReportingName there as well.

Sorry for bugging you with so many questions 😄