Introduce extension API for expression language evaluation in display names

[DcortezMeleth]

Overview

I have found that it would be useful if you could use argument fields as a part of parametrized test name. Right now, when using just {0} as described in docs, it results in injecting into a test name string in form of ClassName(fieldName1=fieldValue1, fieldName2=fieldValue2, ...). But that doesn't seen readable at all especially then you have a class with more than just few fields. Now I would like to be able to use some semantics as {0.name} or {0}.name to use just name field of class used as test method argument, so I can write test name as follows:

@BeerTest
class BeerConversionServiceTest {

    @Autowired
    private BeerConversionService beerConversionService;

    private static Stream<Arguments> createBeers() {
        return Stream.of(
                Arguments.of(new Beer(...), 25L),
                Arguments.of(new Beer(...), 50L)
        );
    }

    @DisplayName("Get alcohol content of a beer in mg")
    @ParameterizedTest(name = "Alcohol content of {0.name} should be equal to {1}")
    @MethodSource("createBeers")
    void getAlcInMg(Beer beer, long mg) {
        Assertions.assertEquals(beerConversionService.getAlcInMg(beer), mg);
    }

}

This would give users possibility to keep test names simple, yet still descriptive, as they could reference objects passed as arguments and use only parts of them which identifies test cases.

I didn't look into the source code so I cannot tell which approach would be easier to implement but I would guess that {0.name} seems more reasonable.

Deliverables

• Introduce an SPI (similar to what has been proposed above) and provide a SpEL-based and/or OGNL-based implementation.
• Ensure that display names for @ParameterizedTest and @RepeatedTest can make use of the same facility (see @ParameterizedTest display custom names for arguments #2452).

[marcphilipp]

I agree that this would be useful. Implementation-wise it might be a rabbit hole, though. We should probably check for getters first. If there are none, we could try to find a field traversing the class hierarchy upward.

@junit-team/junit-lambda Thoughts?

[vikaskumar89]

hi @marcphilipp may i please work on this?

[marcphilipp]

Sure!

[sbrannen]

I agree that this would be useful. Implementation-wise it might be a rabbit hole, though. We should probably check for getters first. If there are none, we could try to find a field traversing the class hierarchy upward.

@junit-team/junit-lambda Thoughts?

1. accessor method: name()
2. getter method: getName()
3. field named name (local and then traversing the hierarchy)

[DcortezMeleth]

1. accessor method: name()
2. getter method: getName()
3. field named name (local and then traversing the hierarchy)

Seems like a good idea.

[sormuras]

How about supporting {{ mustache }} notation? We could include this library here https://github.com/spullara/mustache.java

[marcphilipp]

I was also wondering about that. If we do, it needs to be an extension, though. Moreover, I'm not sure we need everything mustache has to offer. 🤔

[DcortezMeleth]

Mustache may be a nice addition but I agree with Mark that if it's going to be used it should be provided as some kind of extension because it will be used by rather small amount of users.

But will we need anything apart of this simple usecase? If not, additional dependecy seems a bit unnecessary.

[marcphilipp]

Do we want to support things like {0.manufacturer.owner.name}?

[sbrannen]

Based on the discussions here, I think it's already starting to get a bit out of hand.

As soon as we support one level of expressions, people will want more, and then they'll want additional features that the JUnit Team should not be implementing (due to scope).

So, in order to avoid feature creep, my proposal is to go back to the original plan (from the JUnit Lambda prototype phase) to add an extension for plugging in a custom expression language.

See also:

• Introduce SPI for generating display names #162
• Support simple variable expansion in display names #432

In summary, I believe it would be better to design a new extension API that receives the String displayName and a Map<String,Object> expressionContextMap and then allow people to pick between Mustache, SpEL, etc.

[sbrannen]

For example...

• With Mustache, it might look like "Owner: {0.manufacturer.owner.name}".
• With SpEL (from Spring), it might look like "Owner: #{#0.manufacturer.owner.name}" or "Owner: #{#beer.manufacturer.owner.name}".

[marcphilipp]

I agree with @sbrannen. That's what I meant with "rabbit hole" in my first comment.

The problem with customizing display names in general is that most of them are determined during test discovery. Only dynamic, parameterized and repeated test display names are computed during test execution. So, such a new extension point would be limited to the latter category of tests.

[sbrannen]

@marcphilipp, the problems you list are certainly applicable for display names that are dynamic based on the run-time arguments supplied to a test method; however, an expression language or display name generator can still be useful in other use cases (e.g., to create human-readable sentences from camel-cased test method names, etc.).

That's why I suggested a "context map" (or similar) so that such an extension can extract information if it's available, and such an extension would likely otherwise throw an exception or use a default value for referenced objects that do not exist in the current context.

[catalin-cretu]

Hello,

I've come across this issue while searching for ways to start contributing to open source projects.
The issue caught my attention and I started thinking about a potential solution that I would like to share here.

It's a bit different from the direction that this discussion went in. Also, please bear in mind that I'm not familiar with junit5 project and this example might not be consistent with the coding style/conventions used here.

The main idea is to use a more declarative approach instead of a reflective approach.
This might avoid bringing in another dependency, but it might not be the best approach to use if we want to expose this type of functionality as an extension.

So here's some example code, based on the initial comment:

class BeerConversionServiceTest {

    @ParameterizedTest
    @DisplayName("Get alcohol content of a beer in mg")
    // maybe add another Source annotation instead
    @MethodSource("createBeers")
    void getAlcInMg(Beer beer, long mg) { /*...*/ }

    private static ParameterizedTestArguments createBeers() {
        return ParameterizedTestArguments.builder()
                // default to @ParameterizedTest(name), if possible
                .withName("Alcohol content of {0} should be equal to {1}")

                .addPlaceholderResolver(
                        new PlaceholderResolverBuilder.<Beer>()
                                // the parameter is a Function<T, Object>
                                // mostly for code completion and compilation errors
                                .withResolver(beer -> beer.manufacturer.owner.name)
                                .build()) // option A
                .addPlaceholderResolver(
                        new PlaceholderResolverBuilder.<Long>()
                                // default resolver
                                .withResolver(Objects::toString)
                                .build())
                // builder API variations instead of the previous methods
                .addPlaceholderResolver(Beer.class, beer -> beer.manufacturer.owner.name) // option B
                .<Beer>addPlaceholderResolver(beer -> beer.manufacturer.owner.name) // option C

                // varargs parameter passed down to Arguments.of()
                .addArguments(new Beer(...), 25L)
                .addArguments(new Beer(...), 50L)
                .build();
    }
}

Basically, the source method returns a wrapper class around the Stream<Arguments> and it contains other metadata, such as the placeholder resolver.

Some things to consider:

• this declarative approach is more involved for users than the expression language that was mentioned before, but I think it should be easier to maintain
• having support for adding the name as part of the wrapper class (instead of @ParameterizedTest) keeps the display name and placeholder logic closer together
• if we don't store the parameter type, then the resolver will most likely be stored as Function<Object, Object>
• adding support for named parameters
  • defaults to the order of .addPlaceholderResolver() calls
  • probably better to add it to PlaceholderResolverBuilder
• adding another source annotation (for example @NamedArgumentsSource)

So, any thoughts?

[marcphilipp]

@catalin-cretu Thanks for the proposal! A clear downside to this approach is that it would only work with @MethodSource.

[marcphilipp]

That's why I suggested a "context map" (or similar) so that such an extension can extract information if it's available, and such an extension would likely otherwise throw an exception or use a default value for referenced objects that do not exist in the current context.

@sbrannen How would you register such an extension? Right now, we only support extension registration during execution, not during discovery. 🤔

[jszklarz-haventec]

Just to add this would be especially handy for using a ValueSource in conjunction with a nested test class where you are passing in classes but do not want to print the fully qualified name.

[eric-labelle]

This issue is old, but is there any progress or workaround with this? I've been trying to do this for hours! The displayName is really unreadable when we use ParametrizedTest with a stream of complex object.

ie.
With the following sealed classes

sealed class MainEvent : Event {
    object Initial : MainEvent()
    data class ChangeEnvironment(val newEnv: Environment) : MainEvent()
}

sealed class MainState : State {
    abstract val data: MainData

    data class Idle(override val data: MainData) : MainState()
    data class EnvironmentChanged(override val data: MainData) : MainState()
}

I'd like to write a test like this

@FlowPreview
@ExperimentalCoroutinesApi
@ExtendWith(MockKExtension::class)
class ReducerTest : BaseUnitTest() {

    @ParameterizedTest(name = "{index} => {0}")
    @MethodSource("listAllMainState")
    fun reduceMainState_onGivenEvent_returnExpectedState(arguments: GivenWhenThenArguments<MainEvent, MainState>) = runBlockingTest {
        val newState = reduceMainState(arguments.previousState, arguments.event)

        assertThat(newState).isEqualTo(arguments.expectedState)
    }

    private companion object {
        @JvmStatic
        fun listAllMainState(): Stream<GivenWhenThenArguments<out Event, out State>> {
            return Stream.of(
                GivenWhenThenArguments(
                    previousState = MainState.Idle(MainData(environment = Environment.DEV)),
                    event = MainEvent.ChangeEnvironment(newEnv = Environment.PROD),
                    expectedState = MainState.EnvironmentChanged(MainData(environment = Environment.PROD))
                )
            )
        }
    }
}

data class GivenWhenThenArguments<E : Event, S : State>(
    val previousState: S,
    val event: E,
    val expectedState: S
)

Being able to do something like

@ParameterizedTest(name = "{index} => given {0.previousState}, when {0.event} -> {0.expectedState}")

would allow me to have a great control over the displayName so it could look like

1 => given Idle(data=MainData(environment=DEV)), when ChangeEnvironment(newEnv=PROD) -> EnvironmentChanged(data=MainData(environment=PROD)))

instead of the actual

1 => GivenWhenThenArguments(previousState=Idle(data=MainData(environment=DEV)), event=ChangeEnvironment(newEnv=PROD), expectedState=EnvironmentChanged(data=MainData(environment=PROD)))

since MainData could have many fields with complex type, it could really get messy. Ideally I'd want to be able to have the following result

1 => given Idle(environment=DEV), when ChangeEnvironment(PROD) -> EnvironmentChanged(environment=PROD))

looking through the Args converter and aggregator I wasn't able to find something as a workaround but another idea could be to add an argumentDecomposer (or whatever the name) in order to manipulate the Stream.of(Arguments) and dynamically add params to the method to be used in the displayName

i.e.

// pseudo-code
@Target(AnnotationTarget.TYPE, AnnotationTarget.VALUE_PARAMETER)
@Retention(AnnotationRetention.RUNTIME)
@ConvertWith(ArgumentsDecomposer::class)
annotation class DecomposeArgs

class ArgumentsDecomposer {
    override fun decompose(source: Any, context: ParameterContext): Any {
        if (source is ReducerArguments<*, *>) {
            return source, source.previousState, source.event, source.expectedState
        }
    }
}

This way, even if the test would look like

    @ParameterizedTest
    @MethodSource("listAllMainState")
    fun desired_reduceMainState_onGivenEvent_returnExpectedState(
        @DecomposeArgs arguments: GivenWhenThenArguments<MainEvent, MainState>
    )

it would actually become the following, allowing us to use args 1, 2, 3

    @ParameterizedTest(name = "{index} => given {1}, when {2} -> {3}")
    @MethodSource("listAllMainState")
    fun desired_reduceMainState_onGivenEvent_returnExpectedState(
        arguments: GivenWhenThenArguments<MainEvent, MainState>,
        decomposedArg1: Any,
        decomposedArg2: Any,
        decomposedArg3: Any,
    )

What do you guys think about the idea? The decomposer could also allow me to get the ideal display name since I could write a custom decomposer to only print what I want and not the full objects

1 => given Idle(environment=DEV), when ChangeEnvironment(PROD) -> EnvironmentChanged(environment=PROD))

Thanks for your time.

[juliette-derancourt]

Tentatively slated for 5.7 M2 solely for the purpose of team discussion

[xenoterracide]

I more or less agree with @sbrannen about SpEL, though I can also see why that would be hard to implement. However, maybe there's a way to register a plugin that defines the parser of the string, with the current behavior being the default parser, but someone, could for example use SpEL or some other parser to parse.

obviously, I don't think anyone meant this should look for {0.name} specifically, I think that was an example.

[marcphilipp]

What do you guys think about the idea?

We think a display name SPI would be more generally useful.

Team decision: Introduce an SPI (similar to what has been proposed above) and provide a SpEL-based and/or OGNL-based implementation.

[delphym]

Hi there, just learning more in details about JUnit5 and whilst practising some simple examples (taken from Vogella) I'm encountering difficulties to display nested argument (arrays of arrays). I wanted to ask for help with it on StackOverflow or similar, but searching around brought me here. I have feeling my case is similar nature of this issue, perhaps a subcase or extension which might be considered whilst working on a suitable solution.

The best would be to provide the sample code and its existing output in contrast what I would like to achieve instead of it.

package net.delphym.unittests;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.stream.Stream;

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

class DynamicTestCreationTestParam {
	private static Stream<int[]> data() {
		return Stream.of(new int[][] { {1, 2, 2}, {5, 3, 15}, {121, 4, 484} });
	}

	@DisplayName("Multiplication test")
	@ParameterizedTest(name = "#{index} for {arguments}: {0} x {1} = {3}")
	@MethodSource("data")
	void testWithStringParameter(int[] data) {
		MyClass tester = new MyClass();
		int m1 = data[0];
		int m2 = data[1];
		int expected = data[2];
		assertEquals(expected, tester.multiply(m1, m2));
	}

	// class to be tested
	class MyClass {
		public int multiply(int i, int j) {
			return i *j;
		}
	}
}

And the existing output with its customised display names:

Obviously as it is now, it is a bit misleading.
This customised display name: @ParameterizedTest(name = "#{index} for {arguments}: {0} x {1} = {2}")

Ideally, I would like that the customised display name would read something like this (for the 1st run):
#1 for [1, 2, 2]: 1 x 2 = 2 or even just this #1 multiply: 1 x 2 = 2

For that I would need to specify the custom display name to handle something like this:
name = "#{index} multiply: {[0][0]} x {[0][1]} = {[0][2]}")
Unfortunately this syntax is not accepted and I haven't figured out a way how to achieve to display individual parameter from the 1st `{0} and only 1 argument which is the array of ints.

[marcphilipp]

Does it have to use int[]?

private static Stream<Arguments> data() {
	return Stream.of(Arguments.of(1, 2, 2), Arguments.of(5, 3, 15), Arguments.of(121, 4, 484));
}

@DisplayName("Multiplication test")
@ParameterizedTest(name = "#{index}: {0} x {1} = {2}")
@MethodSource("data")
void testWithStringParameter(int m1, int m2, int expected) {
	MyClass tester = new MyClass();
	assertEquals(expected, tester.multiply(m1, m2));
}

[sormuras]

What about unrolling the test data to your (display name) needs. To something like:

	@DisplayName("Multiplication test")
	@ParameterizedTest(name = "#{index} multiply: {0} x {1} = {2}")
	@CsvSource({"1,2,2", "3,5,15", "121,4,484"})
	void testWithStringParameter(int m1, int m2, int expected) {
		MyClass tester = new MyClass();
		assertEquals(expected, tester.multiply(m1, m2));
	}

[delphym]

Does it have to use int[]?

No, not at all. It was just the easiest and the most obvious way how to pass those arguments in.... just learning.

What about unrolling the test data to your (display name) needs. Using CsvSource...

I was about to investigate this option and you have provided me a solution already.

Both are actually very interesting and elegant solutions. Thanks heaps @marcphilipp and @sormuras

Even as part of my learning exercise, I'm very flexible what I can use and how, my scenario could be considered in the implementation if this open issue.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

[jean-andre-gauthier]

Hi @marcphilipp , as discussed over slack I'm currently working on preparing a spike PR for this. Please feel free to assign me the issue