Support JUnit 5 about testparameterinjector HOT 27 CLOSED

Might be an unrelated conversation here, @nymanjens feel free to collapse this.

At https://github.com/junit-pioneer/junit-pioneer/pull/491/files#diff-7b1c59658c736ae3c2131a2425f2f8c88f264a605dd0c0a14dee256d665860c9R18-R21 @jbduncan is double-indirecting the @ExtendWith which is perfectly fine (nice contrib BTW!), because this annotation is meant for a specific parameter (supportsParameter). What I was saying is that one should not just introduce:

@ExtendsWith(SomethingExtension::class)
annotation class Something

for the sake of it, so that you can write

@Something
class MyTest

unless it adds extra value, for example parameters, or more meta-annotations; before then, it's yagni.

Note that TPIT is there to stay (whatever its name) because of the templating constraints (see point 2 in #11 (comment))

from testparameterinjector.

I've just pushed a commit tha adds basic support for JUnit5 in new Maven target.

Notable things that are missing in this first version:

• @nested support
• Support for injecting other types, such as JUnit5's TestInfo

from testparameterinjector.

Good point. Actually, by adding @ExtendWith(...) to @TestParameterInjector.Test, we can avoid a class-level extension altogether.

So this is the new proposed API then:

public class MyTest {

  @TestParameter boolean isDryRun;

  @TestParameterInjector.Test
  public void test1(@TestParameter boolean enableFlag) {
    // ...
  }

  @TestParameterInjector.Test
  public void test2() {
    // ...
  }
}

from testparameterinjector.

https://junit.org/junit5/docs/snapshot/release-notes/#release-notes-5.8.0

5.8 has landed the feature necessary, is there ongoing work?

from testparameterinjector.

This is on my TODO list, but I haven't gotten around to this yet.

from testparameterinjector.

re JUnit 4 v 5, I totally agree testing helpers extending JUnit rules is a plague when it comes to pure JUnit 5 tests.
I think what the approach should be is to have a test-framework agnostic clear API and then a separate junit4 and a jupiter artifact adding the wiring on top of that (this opens the possibility of using the a library with other test frameworks too), an example of such a refactor cashapp/paparazzi#293, sadly usually these are breaking changes because LibraryMainEntry and LibraryMainEntryRule are the same class, and they need to split. I guess in the case of this project, it's quite coupled with JUnit 4 even apart from the API surface?

from testparameterinjector.

I had a play and this is what I learned:

• The meta-annotated @TPIT's @ExtendWith propagates to class from test method, so it actually works without any @ExtendWith. (you said that for templating all test methods have to be annotated.)
• TestInstancePostProcessor works based on @TPIT on method, but not with @Test (good)
• Constructor parameter injection is handled by ParameterResolver if the parameters are annotated rather than the constructor. This means the TestInstanceFactory might not be necessary, unless it's a requirement for templating.
• During implementation mind @Nested and @TestFactory (dynamicTest) features. Also can it be compatible with @RetryingTest (pioneer) and @RepeatedTest (jupiter).
• Have a look at JUnit Pioneers' @CartesianProductTest.
• Jupiter extension APIs are awesome!

annotation class TP

@ExtendWith(TPIE::class)
@Test
annotation class TPIT

import org.junit.jupiter.api.extension.BeforeEachCallback
import org.junit.jupiter.api.extension.ExtensionContext
import org.junit.jupiter.api.extension.ParameterContext
import org.junit.jupiter.api.extension.ParameterResolver
import org.junit.jupiter.api.extension.TestInstanceFactory
import org.junit.jupiter.api.extension.TestInstanceFactoryContext
import org.junit.jupiter.api.extension.TestInstancePostProcessor

class TPIE : BeforeEachCallback, TestInstancePostProcessor, ParameterResolver/*, TestInstanceFactory*/ {
//	override fun createTestInstance(factoryContext: TestInstanceFactoryContext, extensionContext: ExtensionContext): Any? {
//		val ctor = factoryContext.testClass.constructors.single()
//		val outer = factoryContext.outerInstance.map { listOf(it) }.orElse(emptyList())
//		val args = ctor.parameters.drop(outer.size).map { "ctor arg from ext for ${it.name}" }
//		return ctor.newInstance(*(outer + args).toTypedArray())
//	}

	override fun postProcessTestInstance(testInstance: Any, context: ExtensionContext) {
		println("postProcessTestInstance ${testInstance}")
		testInstance::class.java.declaredFields
			.filter { it.isAnnotationPresent(TP::class.java) }
			.forEach { it.set(testInstance, "field from ext") }
	}

	override fun beforeEach(context: ExtensionContext) {
		println("beforeEach ${context.displayName}")
	}

	override fun supportsParameter(parameterContext: ParameterContext, extensionContext: ExtensionContext): Boolean =
		parameterContext.isAnnotated(TP::class.java)

	override fun resolveParameter(parameterContext: ParameterContext, extensionContext: ExtensionContext): Any =
		"param from ext"
}

import org.junit.jupiter.api.Nested

class TPITTests {
	@Nested	inner class OnParam {
		@TPIT fun test(@TP param: String) {
			println(param) // param from ext
		}
	}

	@Nested	inner class OnField {
		@field:TP lateinit var field: String

		@TPIT fun test() {
			println(field) // field from ext
		}
	}

	@Nested inner class OnCtor constructor(
		@param:TP private val ctor: String
	) {
		@TPIT fun test() {
			println(ctor) // param from ext
		}
	}

	@Nested inner class Combo constructor(
		@param:TP private val ctor: String
	) {
		@field:TP lateinit var field: String

		@TPIT fun test(@TP param: String) {
			println("ctor=$ctor field=$field param=$param") // ctor=param from ext field=field from ext param=param from ext
		}
	}

	@Nested inner class Mixed {
		@field:TP lateinit var field: String

		@TPIT fun test(@TP param: String) {
			println("field=$field param=$param") // field=field from ext param=param from ext
		}

		@Test fun normal() {
			println(::field.isInitialized) // false
		}

		@Test fun mismatch(@TP param: String) {
			// error: No ParameterResolver registered for parameter
		}
	}
}

from testparameterinjector.

@TWiStErRob Thanks for your investigation! I confirm your conclusions: The ParameterResolver is indeed also used for constructor parameters, which makes things a lot easier.

TestTemplateInvocationContextProvider and TestInstancePostProcessor seem to be sufficient to cover the following use cases:

class Junit5Test {

    private final boolean constructorParam;

    @TestParameter boolean field;

    public Junit5Test(@TestParameter boolean x){
        this.constructorParam = x;
    }

    @TestParameterInjectorTest
    void withoutParameters() {
        System.out.printf(">>>> JUnit5Test(%s).withoutParameters()\n", constructorParam);
    }

    @TestParameterInjectorTest
    @TestParameters({"{s: false}", "{s: true}"})
    void withParameters_success(boolean s) {
        System.out.printf(">>>> JUnit5Test(%s).withParameters_success(%s)\n", constructorParam, s);
    }

    @TestParameterInjectorTest
    void withParameter_success(@TestParameter boolean b) {
        System.out.printf(">>>> JUnit5Test(%s).withParameter_success(%s)\n", constructorParam, b);
    }
}

Questions I'm still investigating:

• How is this going to work with nested classes? It feels like some API decisions around field parameters and constructors is going to have to be made here. Maybe for a first version this should all be unsupported to keep things manageable.
• How do I set a custom test name? Currently the test names are in the following form, which is terrible:

withParameter_success{boolean}[1]
withParameter_success{boolean}[2]
withParameter_success{boolean}[3]
withParameter_success{boolean}[4]

• What safeguards should I add to reduce the conflicts with other extensions to a minimum?
  • In my local test, I'm always returning true on TestTemplateInvocationContextProvider.supportsTestTemplate() and ParameterResolver.supportsParameter() (see TestParameterInjectorExtension implementation below). It feels like fully claiming TestTemplateInvocationContextProvider for methods annotated with @TestParameterInjectorTest makes sense though.

class TestParameterInjectorExtension implements TestTemplateInvocationContextProvider, TestInstancePostProcessor {

    @Override
    public void postProcessTestInstance(Object testInstance, ExtensionContext context) throws Exception {
        // TODO
    }

    @Override
    public boolean supportsTestTemplate(ExtensionContext context) {
        return true;
    }

    @Override
    public Stream<TestTemplateInvocationContext> provideTestTemplateInvocationContexts(ExtensionContext context) {
        // Returns a stream of TpiTestInvocationContexts, based on the annotated methods
    }


    private static class TpiTestInvocationContext implements TestTemplateInvocationContext {

        private final TestInfo testInfo; // package-private type that holds the parameters and the test name

        @Override
        public String getDisplayName(int invocationIndex) {
            // TODO
        }

        @Override
        public List<Extension> getAdditionalExtensions() {
            return ImmutableList.of(new TpiResolver());
        }

        class TpiResolver implements ParameterResolver {
            @Override
            public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
                return true;
            }

            @Override
            public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
                // TODO
            }
        }
    }

}

from testparameterinjector.

Hi Jonathan,

At Google, we almost exclusively use JUnit4, which is why we haven't felt this need ourselves yet. But we did anticipate this being requested once we open source the project.

Looks like you are the first one requesting this :-). I'll look into how to do this when I get some time.

If anyone has pointers on how this is best done, that would be appreciated :-).

from testparameterinjector.

@nymanjens Thanks for acknowledging this so quickly!

Re. pointers, JUnit 5 provides individual extension points for various parts of the test lifecycle, rather than a super-flexible test runner or rule like JUnit 4 does. There is an official User Guide with a comprehensive reference on the various APIs. But I've found it more helpful to use baeldung's guide first, and then read the User Guide and javadocs or study an existing extension like those in junit-pioneer to get a better feel on what can be done and how to do things.

I hope this helps!

from testparameterinjector.

For those of you looking for a library like TestParameterInjector for JUnit 5 in the meantime, consider using JUnit 5's parameterized tests, junit-pioneer's @CartesianProductTest , or both.

from testparameterinjector.

I've investigated this a bit. My ideal scenario would be to have something like this:

// Old code
@RunWith(TestParameterInjector.class)
public class MyTest {

  @TestParameter boolean isDryRun;

  @Test public void test1(@TestParameter boolean enableFlag) {
    // ...
  }
}

// New code
@ExtendWith(TestParameterInjector.class)
public class MyTest {

  @TestParameter boolean isDryRun;

  @Test public void test1(@TestParameter boolean enableFlag) {
    // ...
  }
}

i.e. replacing @RunWith by @ExtendWith, keeping everything else the same.

It looks like that's not possible because the only extension interface I could find that allows multiple runs is TestTemplateInvocationContextProvider, which seems to need a custom annotation.

So the next best API I can think of would look like this:

@ExtendWith(TestParameterInjector.class)
public class MyTest {

  @TestParameter boolean isDryRun;

  @TestParameterInjector.Test
  public void test1(@TestParameter boolean enableFlag) {
    // ...
  }


  @TestParameterInjector.Test
  public void test2() {
    // ...
  }
}

from testparameterinjector.

@nymanjens Ah, that's a pain.

Alternatively you may be able to make a custom "source" for @ParameterizedTest as per this guide.

public class MyTest {

  @TestParameter boolean isDryRun;

  @ParameterizedTest
  @TestParameterInjectorSource
  public void test1(@TestParameter boolean enableFlag) {
    // ...
  }

  @ParameterizedTest
  @TestParameterInjectorSource
  public void test2() {
    // ...
  }
}

On its own, this wouldn't any better. But you may be able to create a new annotation like @TestParameterInjector.Test that is meta-annotated with @ParameterizedTest and @TestParameterInjectorSource. I've never tried this before, so I don't know if JUnit 5 would recognise it, but if it does, then you could do something like this:

public class MyTest {

  @TestParameter boolean isDryRun;

  @TestParameterInjector.Test
  public void test1(@TestParameter boolean enableFlag) {
    // ...
  }

  @TestParameterInjector.Test
  public void test2() {
    // ...
  }
}

...which looks more similar to the JUnit 4-based API to me.

from testparameterinjector.

The JUnit 5 maintainers will have more insight on this than me, so try raising an issue on the JUnit 5 issue tracker to see if using @ExtendWith and @Test like your ideal scenario is possible, or could be made possible. They're very receptive, in my experience.

from testparameterinjector.

@nymanjens Actually, it looks like we might be able to make this even more concise.

In the next release of JUnit 5 - 5.8 - @ExtendWith will be able to be applied to fields and parameters. (See their ongoing feature branch for more technical info if you're interested.)

So the following annotation will be possible:

@Target({ ElementType.FIELD, ElementType.PARAMETER })
@Retention(RetentionPolicy.RUNTIME)
@ExtendWith(TestParameterInjectorExtension.class)
public @interface TestParameter {
}

And in turn your example API usage can be shortened to:

public class MyTest {

  @TestParameter boolean isDryRun;

  @Test
  public void test1(@TestParameter boolean enableFlag) {
    // ...
  }

  @Test
  public void test2() {
    // ...
  }
}

This makes it just like TestParameterInjector's JUnit 4-based API, minus a @RunWith(TestParameterInjector.class) usage! So I'd say waiting for JUnit 5.8 is the best move.

from testparameterinjector.

Cool, thanks for the info. That sounds ideal :-D

from testparameterinjector.

Also, thanks for letting me know about the feature landing :-)

from testparameterinjector.

I've investigated this some more. My conclusions:

• The following JUnit5 extension interfaces are relevant for this project:
  • TestTemplateInvocationContextProvider: For creating multiple tests for a single method. This is key because it's the only extension mechanism that I could find that can increase the amount of test invocations. So this has to be called for every test method, even if the method itself isn't parameterized (because there might be field/constructor parameterization)
  • TestInstanceFactory: For calling a constructor with parameters
  • TestInstancePostProcessor: For populating parameterized fields (can be omitted if TestInstanceFactory is also used for the default constructor)
• TestTemplateInvocationContextProvider is only called when the test method is annotated with a custom annotation (with the @TestTemplate annotation). This means that the solution proposed in an earlier comment (#11 (comment)) won't work because it uses the generic @Test. Instead, we'll need an @Test variant for this library, e.g. @TestParameterInjectorTest
  • I tested this on version 5.8.1. I think the new feature request doesn't help here because it doesn't seem to apply to TestTemplateInvocationContextProvider (the examples are usually about ParameterResolver)
• TestInstanceFactory and TestInstancePostProcessor only work if the class is annotated with @ExtendWith(TestParameterInjectorExtension.class) (it doesn't matter if all tests are annotated with @TestParameterInjectorTest or fields/parameters/constructors are annotated with @TestParameter)
• It's likely going to be a requirement that the JUnit5 tests don't depend on JUnit4 and vice versa (example issue).
  • So any JUnit5 solution won't be able to use TestParameterInjector because it extends from the JUnit4 runner.
  • Likewise, @TestParameter[s] shouldn't be annotated with @ExtendWith, because that's a JUnit5 dependency. Given the above, that's not really needed anyway.

So given the constraints above, this is the new proposed API:

@ExtendWith(TestParameterInjectorExtension.class)
class MyTest {

    @TestParameter boolean field;

    @TestParameterInjectorTest
    void myTest() { ... }

    @TestParameterInjectorTest
    @TestParameters2({"{a: 1}", "{a: 2}"})
    void withParameters_success(int a) { ... }

    @TestParameterInjectorTest
    void withParameter_success(@TestParameter boolean b, @TestParameter boolean c) { ... }
}

from testparameterinjector.

Nice investigation!

Mind you that "extendswith" is additive and there could be other ParameterResolvers (e.g. get a spring context, or create a DB session). With that, @TestParameterInjectorTest should make sure to work with other extensions.

@ExtendWith(TestParameterInjectorExtension.class)
@ExtendWith(OtherExtension.class)
class MyTest {

    @TestParameterInjectorTest
    @TestParameters2({"{a: 1}", "{a: 2}"})
    void withParameters_success(int a, @Something Type injected) { ... }

(Quick note: TPIT is pretty a mouthful, imagine every developer having to write that every time they write a test.)

from testparameterinjector.

I think what the approach should be is to have a test-framework agnostic clear API and then a separate junit4 and a jupiter artifact

Yep, that's the plan. I'm currently refactoring the implementation to make it JUnit4-agnostic. The public types @TestParameter and @TestParameters can be shared because they don't reference any framework. The public type TestParameterInjector will remain tied to JUnit4 without a breaking change that would have too big of an impact I think.

(Quick note: TPIT is pretty a mouthful, imagine every developer having to write that every time they write a test.)

I agree it's a mouthful, and I'm open to alternative suggestions. I've considered @TpiTest, but we're usually no fans of acronyms and strong abbreviations.

from testparameterinjector.

Yeah, not a fan of abbreviations either, I had some thinking and couldn't come up with any better name 😢.

Just found a new thing, regarding @ExtendWith(TestParameterInjectorExtension.class) on the class. I think that might not be necessary in some cases, for example @ParameterizedTest is annotated with @ExtendsWith and a single annotation carries the necessary setup.

from testparameterinjector.

I think that might not be necessary in some cases

As I understand it, if you don't add @ExtendWith, you can't support constructor or field parameterization, only method parameterization. I think that's OK for @ParameterizedTest because it only supports method parameters.

Method parameterization is probably the most commonly used one, but field parameterization is definitely occasionally useful, and used enough to be confusing if it weren't supported. It could work by only requiring @ExtendWith when using field/constructor parameterization, but that just feels like it's going to lead to a lot of NullPointerException confusion.

from testparameterinjector.

Excellent investigations, both! It's a bit gutting to see that we cannot use @Test directly, but it's good to know.

And hopefully there's a way we can avoid forcing users to use @ExtendsWith(...)... Fingers crossed.🤞 Worst case scenario, we could make TestParameterInjectorExtension it's own meta-annotation annotated with @ExtendsWith(TPIE.class), perhaps. Or even introduce another TestParameterInjector class under the JUnit 5 artifact, separate from the JUnit 4 one.

from testparameterinjector.

As for acronyms for @TestParameterInjectorTest, if we went for my idea of a separate, JUnit 5-specific TestParameterInjector type, we could then introduce @TestParameterInjector.Test (note the dot), so that people can explicitly import it like so:

import com.google.testing.junit.jupiter.testparameterinjector.TestParameterInjector
import com.google.testing.junit.jupiter.testparameterinjector.TestParameterInjector.Test
...

@TestParameterInjector
class MyTest {

    @TestParameter boolean field;

    @Test
    void myTest() { ... }

    @Test
    @TestParameters2({"{a: 1}", "{a: 2}"})
    void withParameters_success(int a) { ... }

    @Test
    void withParameter_success(@TestParameter boolean b, @TestParameter boolean c) { ... }
}

A disadvantage is users could easily get @TestParameterInjector.Test confused with JUnit 5's @Test, I suppose.

from testparameterinjector.

Careful, remember that code is read more than written. It can be easily confused, agreed; also you would get into problems with automatic formatters (forcing qualified usages), or wanting to use parameterized and non-paramterized tests in the same class.

The same applies for a meta-annotated extension annotation. Does it actually bring benefits? Having 4 ExtendsWith on the class is cleaner than 4 random annotations that might or might not extend.

from testparameterinjector.

I think the @ExtendsWith is where we'll have to agree to disagree, as I'm using a meta-annotation to hide my extension class name in my new extension for JUnit Pioneer, which IMO looks neater.

But I agree with everything else you've said. :)

from testparameterinjector.

@TWiStErRob Good argument re. my meta-annotation idea not adding anything extra behaviour-wise, I'm convinced!

from testparameterinjector.