junit5 icon indicating copy to clipboard operation
junit5 copied to clipboard

Support custom description for set of arguments in parameterized tests

Open dmitry-timofeev opened this issue 7 years ago • 14 comments

Overview

This is a feature request and a follow-up to the discussion in #1306. Currently if a user needs to add a description of a set of arguments (e.g., because their string representation is not enough, or the differences between adjacent arguments are subtle), they need to (ab)use the Arguments by adding an additional one. This has a couple of drawbacks:

  • That argument must be specified in the test method signature. Although it may be given a descriptive name to clarify its purpose (e.g., "testDescription"), it will still be highlighted by an IDE as unused (see #1306).
  • A proper workaround is ugly: @SuppressWarnings("unused") // Used in the test name above
  • Such argument is misleading, as it isn't a test parameter, but a meta-data about test parameters. It shall not affect the test results.

Example

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

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

class SampleTest {

  @ParameterizedTest(name = "{1}: {0}")
  @MethodSource("testParameters")
  void test(String s,
            @SuppressWarnings("unused")  // Used in the test name above
            String testDescription) {
    assertThrows(NumberFormatException.class,
        () -> Integer.parseInt(s)
    );
  }

  static Stream<Arguments> testParameters() {
    return Stream.of(
        Arguments.of("1a", "illegal character"),
        Arguments.of("10000000000000", "out of bounds")
    );
  }
}

Possible solutions

As suggested by @sormuras in the discussion, an optional attribute may be added to Arguments.

Alternatives

  • #1306, but it will enable users to write confusing code (= reference some arguments that do not appear in the method parameters by index).

Questions

  • [ ] Q1: Shall CsvSource & CsvFileSource be expanded with a similar attribute (e.g., description column index):
@CsvSource(arguments = {
    "Arg1, Arg2, Test Case #1 Description",
    // …    
}, descriptionColumnIndex = 2)

?

  • [ ] Q2: Shall we change the default value of org.junit.jupiter.params.ParameterizedTest#name or the interpretation of {arguments} to include the description if it's non-empty?
  • [ ] Q3: What a name of the method setting a description should be (consider IDE auto-complete!):
    • ~~desc~~
    • description
    • describedAs
    • withDescription?
  • [x] Q4: What return type of the method returning the description should be:
    • ~~String, with empty string meaning no description.~~
    • Optional<String>, with empty meaning no description. PoC shows, that this one is preferred.
  • [ ] Q5: Shall the method setting a description accept:
    • String.
    • CharSequence
    • Object and format as it does the parameters?
  • [ ] Q6: How to separate parameters and their description:
    • a space:
    • a dot + space: .
    • a colon + space: : (A colon is likely to appear in description itself, e.g. "Regression: Integer.MIN_VALUE, see XYZ-123").

Deliverables

  • [ ] User Guide must be updated
  • [ ] TBD

dmitry-timofeev avatar Feb 24 '18 16:02 dmitry-timofeev

According to the discussion in #1306, the new API would look like Arguments.of(...).describedAs("") or something similar. How would you use it? Via a special {description} placeholder for @ParameterizedTest(name="{description}: ...")? If so, what do we use for Arguments instances without a description?

marcphilipp avatar Feb 24 '18 19:02 marcphilipp

Default to lorem ipsum dolor sit amet... ;)

{description} could default to joined string of all arguments, like Arguments[0={0}, 1={1}, ...]

sormuras avatar Feb 24 '18 19:02 sormuras

I've updated the questions section in the issue description, based on the feedback on a PoC PR.

As a user of this feature, I'd expect the following:

  1. CsvSource & CsvFileSource with an optional attribute and an empty description by default.
  2. I certainly want a sensible default that includes everything that I've put in the Arguments and formats that accessibly:
  • If no description -- keeps as it is: "[{index}] {0} {1} … {n-1}"
  • If there is description -- includes it, probably, like that: "[{index}] {0} {1} … {n-1}: {description}"

I also like to think of Arguments as an optionally described sequence of test parameters. Therefore, I'd expect {arguments} to be expanded into:

  • "{0} {1} … {n-1}" -- when no description
  • "{0} {1} … {n-1}: {description}" -- when there is one. I'd make it a job of the formatter to expand {arguments} properly. I'd also put description in the end for it (a) appears there in the source code and (b) may be quite long.

If I'm not happy with the default, I'd like to reference individual parts that comprise Arguments:

  • Each parameter by index: "{0}".
  • Description: "{description}".

I can't think of a use case when one needs all parameters with no description, since the resulting string is not supposed to be machine-readable.

  1. As a user, I care about readability, unambiguity (esp. to those who see this in a PR infrequently) and precise auto-complete. Therefore, I'd go with such a name that has 0 prefix with another method names. I like @marcphilipp describeAs.

  2. As we documented in #1312 , a user shall generally not access arguments, therefore, this method shall serve well JUnit internal use-cases.

  3. Although Object seems like the most flexible, it might be an overkill. String or CharSequence must be enough for me.

Edit: Q6: I no longer like colon ­— it might appear in the description itself and yield something like: [0] -2^31: Regression: Integer.MIN_VALUE, see XYZ-123

What do you think?

dmitry-timofeev avatar Feb 25 '18 11:02 dmitry-timofeev

Possibly also related to https://github.com/junit-team/junit5/issues/162

mkobit avatar Feb 26 '18 16:02 mkobit

Just thinking out loud a bit here...

We need to keep in mind that a @MethodSource factory may simply return a stream of objects as opposed to a stream of Arguments. Thus, a change to the Arguments API would be of zero benefit in such use cases. Similarly, @ValueSource, @CsvSource, etc. also would not benefit from a custom description that can only be supplied via the Arguments API.

Perhaps we could come up with a generic means for supplying custom descriptions per parameterized test invocation, but I cannot think of anything at the moment, due to the fact that arguments are always supplied via arrays in the corresponding source annotations. We would actually need to supply arrays of tuples in order to achieve that via annotations, but that would be a breaking change or would require parallel sets of attributes in existing source annotations.

sbrannen avatar Mar 02 '18 20:03 sbrannen

CsvSource & CsvFileSource with an optional attribute and an empty description by default.

I think that would be very unnatural in terms of usability.

For @CsvSource you'd end up with two arrays which you'd have to keep in sync.

For @CsvFileSource it would be worse since changes to the file would require changes to an array in an attribute in a separate file (i.e., the source code of the test).

An alternative for @CsvFileSource would be to support a preceding comment line for each line in the CSV file, but I'm not sure we'd want to go that route.

sbrannen avatar Mar 02 '18 20:03 sbrannen

  • If no description -- keeps as it is: "[{index}] {0} {1} … {n-1}"

I agree with that: if the optional description is not present, nothing should change from the status quo:

"[{index}] {arguments}"

  • If there is description -- includes it, probably, like that: "[{index}] {0} {1} … {n-1}: {description}"

If a custom description is present, I think I'd prefer the following:

"{description} :: [{index}] {arguments}"

sbrannen avatar Mar 02 '18 20:03 sbrannen

@sbrannen, Thank you for taking a look at the issue!

CsvSource & CsvFileSource with an optional attribute and an empty description by default.

I think that would be very unnatural in terms of usability.

Certainly, separate arrays would be terrible. I should've described what optional argument I mean (I'll update the issue description to avoid future confusions):

@CsvSource(arguments = {
    "Arg1, Arg2, Test Case #1 Description",
    // …    
}, descriptionColumnIndex = 2)

, same for @CsvFileSource. I believe description support is useful when arguments and their description are adjacent in either source code or test case spreadsheet.

Regarding @ValueSource, I think it's perfectly reasonable to not support custom descriptions for values, because this source can only provide a single argument and of a limited set of types. Definetely someone will need to describe them sometimes — but they can use a two-column @CsvSource and JUnit will perform type conversions automatically (e.g., "5" -> int).

Regarding a generic means of supplying custom descriptions: do you mean a use-case when you want to programmatically generate descriptions, something one currently can do like that:

return Stream.of(
    "Arg 1",
    "Arg 2"
).map(
    o -> FooTest::barArgumentsWithDescription(o)
);

?

dmitry-timofeev avatar Mar 03 '18 11:03 dmitry-timofeev

As an alternative to @CsvXSource#descriptionColumnIndex we may add an enum specifying where to get description from @CsvXSource#description = Column.{FIRST, LAST, NONE}. Less flexible, but easier to deal with in providers and more refactoring-friendly (= if you keep last, you don't have to change the column index when adding or removing arguments).

dmitry-timofeev avatar Mar 03 '18 13:03 dmitry-timofeev

Thanks for explaining your proposed descriptionColumnIndex attribute.

Regarding a generic means of supplying custom descriptions: do you mean

No, I actually meant something general-purpose that could be used in all use cases, but as I hinted at before... I don't think that is actually possible/feasible.

So, if we decide to support custom descriptions per set of arguments, I think it will have to be limited to an additional feature in the Arguments API.

Whether or not we'd want to support something similar for CSV inputs via new annotation attributes is up for debate. In any case, I would rather not use a limited enum for selecting the position of the description.

sbrannen avatar Mar 17 '18 14:03 sbrannen

Moved to General Backlog to see if we receive additional requests for this feature.

marcphilipp avatar Apr 06 '18 10:04 marcphilipp

Additional request is here.

If you put an argument like isDescripted (I think a boolean is enough for this) into @ParameterizedTest, than that can mark that the last argument in the source (I don't think having multiple ones make sense) should be used as/in the description, even if the test does not use it. That should also be compatible with every argument source we can think of.

Frontrider avatar Jun 07 '18 19:06 Frontrider

That method is actually my current workaround: Add an extra string as the last argument, than I ignore it in the test, but use it in the template of the description.

Frontrider avatar Jun 14 '18 07:06 Frontrider

That method is actually my current workaround: Add an extra string as the last argument, than I ignore it in the test, but use it in the template of the description.

~~I'm facing similar problem, i.e. I'd like to have a programmatic way of composing description when I use method source with multiple arguments. What I discovered is that appending extra argument to args list works but comes with side-effect - it invokes toString on parameters that are before the last one - which is unwanted in my case.~~

~~Would it be possible to add something like Arguments.of(description, arg1, arg2, arg3) ?~~

The Named.named() that I found in linked issue above worked for me. Sorry for confusion.

kasobol-msft avatar May 12 '22 18:05 kasobol-msft

Team decision: Given our limited capacity, the team has decided to close issues for which there has not been considerable interest.

marcphilipp avatar Apr 13 '23 14:04 marcphilipp