Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple annotations as source for Parameterized Tests #1101

Closed
jochenchrist opened this issue Oct 10, 2017 · 14 comments
Closed

Support multiple annotations as source for Parameterized Tests #1101

jochenchrist opened this issue Oct 10, 2017 · 14 comments

Comments

@jochenchrist
Copy link

jochenchrist commented Oct 10, 2017

Feature Request to simplify Parameterized Tests.

I am looking for an easy way to implement Parameterized Tests with multiple arguments.

While I could use @CsvSource, I'd appreciate multiple annotations, similar to NUnit's TestCase.

Example:

    @ParameterizedTest // still required?
    @Parameter(10, "10000")
    @Parameter(20, "20000")
    @Parameter(30, "30000")
    void calculate(int input, String expected) {
        assertThat(Integer.toString(input*1000)).isEqualTo(expected);
    }

Any thoughts on this?

@sormuras
Copy link
Member

sormuras commented Oct 10, 2017

You could either use a MethodSource

@ParameterizedTest
@MethodSource("stringAndIntProvider")
void testWithMultiArgMethodSource(String first, int second) {
    assertNotNull(first);
    assertNotEquals(0, second);
}

static Stream<Arguments> stringAndIntProvider() {
    return Stream.of(Arguments.of("foo", 1), Arguments.of("bar", 2));
}

or unroll it to distinct (individually startable) test methods.

Why do you want to "program logic" with annotations, anyway?

@jochenchrist
Copy link
Author

Thanks for the quick reply.
Sure, MethodSourcewill work as well, but for me, it is quite hard to read.

So, the feature request with multiple annotations is for readability and simplicity.

@sormuras
Copy link
Member

In addition to the MethodSource solution, how about three more ways to achieve your goals?

This is my preferred way. Clean, simple, basic. Full life-cyle support. Full IDE support for executing a single test:

@Test
void test10() {
	calculate(10, "10000");
}

@Test
void test20() {
	calculate(20, "20000");
}

@Test
void test30() {
	calculate(30, "30000");
}

Use Assertions.assertAll:

@Test
void testAll() {
	assertAll("calculating",
			() -> calculate(10, "10000"),
			() -> calculate(20, "20000"),
			() -> calculate(30, "30000")
	);
}

Use @TestFactory to gain nicer tree nodes in IDEs, but losing life-cycle support:

@TestFactory
DynamicTest[] testDynamic() {
	return new DynamicTest[] {
			dynamicTest("10", () -> calculate(10, "10000")),
			dynamicTest("20", () -> calculate(20, "20000")),
			dynamicTest("30", () -> calculate(30, "30000"))
	};
}

All 3 propsals refer to this "internal" method:

private void calculate(int input, String expected) {
	assertEquals(expected, Integer.toString(input*1000));
}

With CsvSource and MethodSource included, I don't think we need a sixth way. :)

@jochenchrist
Copy link
Author

@sormuras your recent examples are totally correct, but not really regarded to Parameterized Tests ;-)

Surly these are different ways how to express the different cases. My point is simply to make it as easy as possible to read and understand the tests, which is not really the point for current implementation of @ParameterizedTest

So, I'd suggest to keep the feature request open for a while for team and community discussion.

@mkobit
Copy link
Contributor

mkobit commented Oct 16, 2017

You are restricted by the types that can be in an annotation, so keep that in mind when you are trying to model these tests.

If you do really want this behavior, you could implement your own extension, too.

  • Annotation

    Retention(RetentionPolicy.RUNTIME)
    @Repeatable(Parameters.class)
    @ArgumentsSource(ParameterSource.class)
    public @interIn the cases I can think offace Parameter {
      int first();
      String second();
    }
  • Repeatable annotation

    import java.lang.annotation.Retention;
    import java.lang.annotation.RetentionPolicy;
    
    @Retention(RetentionPolicy.RUNTIME)
    public @interface Parameters {
      Parameter[] value();
    }
  • Extension

    import org.junit.jupiter.api.extension.ExtensionContext;
    import org.junit.jupiter.params.provider.Arguments;
    import org.junit.jupiter.params.provider.ArgumentsProvider;
    import org.junit.platform.commons.support.AnnotationSupport;
    
    import java.util.List;
    import java.util.stream.Stream;
    
    public class ParameterSource implements ArgumentsProvider {
      @Override
      public Stream<? extends Arguments> provideArguments(final ExtensionContext context) throws Exception {
        System.out.println("In arg provider");
        System.out.println("Element " + context.getElement());
        context.getTestMethod().ifPresent(method -> System.out.println("Is method"));
        context.getElement().ifPresent(annotatedElement -> System.out.println("Element (Parameter.class): " + AnnotationSupport.findRepeatableAnnotations(annotatedElement, Parameter.class)));
        return context.getElement()
          .map(annotatedElement -> AnnotationSupport.findRepeatableAnnotations(annotatedElement, Parameter.class))
          .map(List::stream)
          .map(parameterStream -> parameterStream.map(parameter -> Arguments.of(parameter.first(), parameter.second())))
          .orElse(Stream.empty());
      }
    }
  • Test

    import static org.assertj.core.api.Assertions.assertThat;
    
    import org.junit.jupiter.params.ParameterizedTest;
    import org.junit.jupiter.params.provider.ArgumentsSource;
    
    class ParameterExample {
      @ParameterizedTest
      @ArgumentsSource(ParameterSource.class) // Needed due to https://github.com/junit-team/junit5/issues/1112
      @Parameter(first = 10, second = "10000")
      @Parameter(first = 20, second = "20000")
      @Parameter(first = 30, second = "30000")
      void calculate(int input, String expected) {
        System.out.println("In Test");
        assertThat(Integer.toString(input * 1000)).isEqualTo(expected);
      }
    }

What would you like to see built into JUnit 5 to support your use case?

@jochenchrist
Copy link
Author

Thanks for your neat example.
Unfortunately Java is quite limited what is allowed for Annotation values.

The generic solution would a String array for value() similar to @ValueSource

Annotation

@Retention(RetentionPolicy.RUNTIME)
@Repeatable(Parameters.class)
public @interface Parameter {
    String[] value();
}

ParameterSource:

public class ParameterSource implements ArgumentsProvider {

    @Override
    public Stream<? extends Arguments> provideArguments(final ExtensionContext context) throws Exception {
        System.out.println("In arg provider");
        System.out.println("Element " + context.getElement());
        context.getTestMethod().ifPresent(method -> System.out.println("Is method"));
        context.getElement().ifPresent(annotatedElement -> System.out.println("Element (Parameter.class): " + AnnotationSupport.findRepeatableAnnotations(annotatedElement, Parameter.class)));
        return context.getElement()
                .map(annotatedElement -> AnnotationSupport.findRepeatableAnnotations(annotatedElement, Parameter.class))
                .map(List::stream)
                .map(parameterStream -> parameterStream.map(parameter -> Arguments.of(parameter.value())))
                .orElse(Stream.empty());
    }
}

Test:

    @ParameterizedTest
    @Parameter({"10", "10000"})
    @Parameter({"20", "20000"})
    @Parameter({"30", "30000"})
    void calculate(int input, String expected) {
        assertEquals(expected, Integer.toString(input * 1000));
    }

IMHO this is still more readable than CSVSource, which I expect to see often:

    @ParameterizedTest
    @CsvSource({"10, 10000", "20, 20000", "30, 30000"})
    void calculate(int input, String expected) {
        assertEquals(expected, Integer.toString(input * 1000));
    }

@marcphilipp
Copy link
Member

You can also write the above like this:

@ParameterizedTest
@CsvSource({
    "10, 10000",
    "20, 20000",
    "30, 30000"
})
void calculate(int input, String expected) {
    assertEquals(expected, Integer.toString(input * 1000));
}

Another possible API for an extension:

@ParameterizedTest
@ParameterSource({
    @Parameter({"10", "10000"})
    @Parameter({"20", "20000"})
    @Parameter({"30", "30000"})
})
void calculate(int input, String expected) {
    assertEquals(expected, Integer.toString(input * 1000));
}

In my opinion, readability differs only very slightly. Thus, I'm closing this issue for now but encourage you to implement it as an extension. 🙂

@JensPiegsa
Copy link

JensPiegsa commented Apr 30, 2018

Some second thoughts and practical implications:

  1. @CsvSource
    Within @ValueSource I can ctrl+click on a String value that represents a filename to directly jump to the file. This has proven invaluable for me when trying to figure out what's going on. It does not work with @CsvSource, but would work with the proposed @Parameter annotation out of the box.

  2. @MethodSource / own extensions based on @ArgumentsSource
    Isn't providing input and expected output the central use case of @ParameterizedTest? Or at least the one that encourages developers to include simple and meaningful asserts. This shouldn't require all this plumbing or writing and maintaining of additional dependencies...

  3. plain alternatives proposed in Support multiple annotations as source for Parameterized Tests #1101 (comment)
    They don't allow to specify patterns (like with @ParameterizedTest name), so the semantics are lost. The whole concept of parameterized tests is denied here.

@marcphilipp
Copy link
Member

marcphilipp commented May 1, 2018

@jochenchrist @JensPiegsa I could imagine supporting the following:

@ParameterizedTest
@ValueSource({ @Values({"foo", "1"}), @Values({"bar", "2"}), @Values({"baz, qux", "3"}) })
void testWithValuesInValueSource(String first, int second) {
	assertNotNull(first);
	assertNotEquals(0, second);
}

@CsvSource is still much shorter, but I can see some value in it.

@junit-team/junit-lambda Thoughts?

@marcphilipp marcphilipp reopened this May 1, 2018
@marcphilipp marcphilipp added this to the 5.3 Backlog milestone May 1, 2018
@marcphilipp
Copy link
Member

marcphilipp commented May 1, 2018

Added to 5.3 Backlog for team discussion.

@rweisleder
Copy link
Contributor

rweisleder commented Apr 6, 2020

I just came across this issue because of NUnit' TestCase annotation.

[TestCase(12, 3, ExpectedResult=4)]
[TestCase(12, 2, ExpectedResult=6)]
[TestCase(12, 4, ExpectedResult=3)]
public int DivideTest(int n, int d)
{
    return n / d;
}

Especially an equivalent of the ExpectedResult attribute would increase the readability in JUnit's parameterized tests.

NUnit Documentation

@JamesMcGuigan
Copy link

This functionality has been implemented as part of jUnit Pioneer with their @CartesianProductTest

pom.xml

<dependency>
    <groupId>org.junit-pioneer</groupId>
    <artifactId>junit-pioneer</artifactId>
    <version>1.3.0</version>
    <scope>test</scope>
</dependency>

Test.java

import org.junitpioneer.jupiter.CartesianProductTest;
import org.junitpioneer.jupiter.CartesianValueSource;

@CartesianProductTest
@CartesianValueSource(ints = { 1, 2 })
@CartesianValueSource(ints = { 3, 4 })
void myCartesianTestMethod(int x, int y) {
    // passing test code
}

Maybe this is something that should be considered for addition to junit core?

@marcphilipp marcphilipp modified the milestones: 5.8 Backlog, 5.8 M2 May 13, 2021
@marcphilipp
Copy link
Member

Tentatively slated for 5.8 M2 solely for the purpose of team discussion.

@marcphilipp
Copy link
Member

Team decision: Since the above syntax would only support strings, it would not provide much benefit compared to what's possible with @CsvSource. Hence, closing this issue.

@sbrannen sbrannen removed this from the 5.8 M2 milestone May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants