junit5 icon indicating copy to clipboard operation
junit5 copied to clipboard

Introduce extension API for accessing arguments passed to tests

Open rumpfc opened this issue 8 years ago • 29 comments

Issue

Extension methods all have ExtensionContext which provides us many useful information like displayname, method or tags. This helps in writing custom test reports.

What I'm missing is a way to get argument instances of a @ParameterizedTest. When not running tests extended by a ParameterResolver using ExtensionContext.Store you can only make a workaround to get a clue of what arguments were used for the test:

  • Filter displayname (objects need to override toString() for important informations)
  • get values from annotations

If parameters are simple strings, integers etc. there is no problem when parsing displayname. The problem starts when one of the parameters is an object containing lots of informations. A new instance would have to be created when analyzing the test, but this doesn't garanty an object with exact the same content like in the test and might also lead to memory issues.

Most simple way is to reimplement Parameterized Test Template, extend from each class, catch resolved parameters and store them directly in ExtensionContext.Store, but I'm not a fan of writing Wrapper classes for such small extensions in existing codes.

Possible Suggestions

  • Provide a method Arguments[] ExtensionContext.getArguments() or List<Arguments> ExtensionContext.getArguments() which returns a list of all Arguments used for the test (empty list if no arguments were used).
  • Store arguments in ExtensionContext.Store with a predefined namespace in your ParameterizedTestParameterResolver class (then there is no need to do it in custom ArgumentsProvider classes)
  • Provide an Extension interface which is called between BeforeTestExecutionCallback and @Test. As parameters it should have ExtensionContext and Arguments[].

Related Issues

  • #944
  • #1668
  • #1884

rumpfc avatar Nov 03 '17 09:11 rumpfc

This might be useful for more than just @ParameterizedTest methods. Right now the arguments for a test method get resolved immediately before calling it, i.e. after BeforeTestExecutionCallback extensions are called. When would you need the arguments? Would it suffice to get access to them after the test has been executed?

marcphilipp avatar Nov 03 '17 11:11 marcphilipp

Test analysis are mostly done after the test execution, so I think it's ok to just have them in one of the AfterXXX interfaces. Getting arguments before test execution would also be nice, but instead of storing them in an attribute I don't know what someone should do with them before the actual test.

I use an annotation (similar to @ValuesSource) which accepts a String[] of filenames containing data for data-driven tests. An ArgumentsProvider checks if "filename" exists and stores information in an object. The object stores quite a lot of information which can't be displayed in a single String, so getDisplayname() to fetch informations is not an option.

Priority can be set to medium, it is a nice-to-have feature with lots of advantages on test reporting side. If you are able to release JUnit 5.1 before March 2018 I'm more than satisfied.

rumpfc avatar Nov 03 '17 11:11 rumpfc

I think introducing a new extension point that is called between BeforeTestExecutionCallback and @Test would offer the most flexibility.

It could look like this:

interface ArgumentsProcessor extends Extension {
    void processTestMethodArguments(ExtensionContext context, Object[] arguments);
}

I guess it should only be called when arguments is not empty, i.e. the test method has parameters. Changing the array would cause the test method to be called with changed arguments.

Lifecycle methods and test class constructors can also have arguments that get resolved by ParameterResolvers. Since I cannot think of a possible use case for those, I wouldn't call extensions. We could always add additional methods like processConstructorArguments(...) later.

@junit-team/junit-lambda Thoughts?

marcphilipp avatar Nov 03 '17 11:11 marcphilipp

I think that's a reasonable idea.

I guess it should only be called when arguments is not empty, i.e. the test method has parameters.

That would make sense.

Changing the array would cause the test method to be called with changed arguments.

That seems potentially risky. I'll have to ponder that a bit. 😉

sbrannen avatar Nov 03 '17 11:11 sbrannen

Since I cannot think of a possible use case for those, I wouldn't call extensions. We could always add additional methods like processConstructorArguments(...) later.

If we foresee that, we should either change the name of the ArgumentsProcessor API to reflect that or introduce all such methods as no-op default methods.

sbrannen avatar Nov 03 '17 11:11 sbrannen

Will this extension also be called when method just contains TestInfo or TestReporter? According to previous posts I can imagine that this might happen and if it would be critical for test environment.

rumpfc avatar Nov 03 '17 11:11 rumpfc

Yes, it would be called for any arguments. If you're not interested in TestInfo or TestReporter, you could not report them in your extension implementation or would that be a problem?

marcphilipp avatar Nov 03 '17 12:11 marcphilipp

You can easily ignore these arguments just by using "instanceof TestInfo" or "instanceof TestReporter", no need to create special cases because of that.

rumpfc avatar Nov 03 '17 12:11 rumpfc

Changing the array would cause the test method to be called with changed arguments.

That seems potentially risky. I'll have to ponder that a bit. :wink:

I have a suggestion for this issue: why not calling the interface between @Test and AfterTestExecutionCallback (doesn't matter if before or after TestExecutionExceptionHandler). Then you would avoid a possible change of arguments before the actual test execution

rumpfc avatar Nov 13 '17 07:11 rumpfc

The Eclipse Jetty project would like to have this ability.
To be able to write an Extension that has a fully resolved TestInfo (with parameters) just before the test is actually called.

joakime avatar Sep 19 '18 09:09 joakime

Marking this issue "up-for-grabs". @joakime / @olamy want to give it a shot?

Implementation outline as @marcphilipp described above in https://github.com/junit-team/junit5/issues/1139#issuecomment-341683075

sormuras avatar Sep 19 '18 10:09 sormuras

I've put more thought into this and now have additional feedback.

I think introducing a new extension point that is called between BeforeTestExecutionCallback and @Test would offer the most flexibility.

I agree.

It could look like this:

interface ArgumentsProcessor extends Extension {
    void processTestMethodArguments(ExtensionContext context, Object[] arguments);
}

I think the signature is fine, but I'm not yet convinced regarding the naming. See following points.

I guess it should only be called when arguments is not empty, i.e. the test method has parameters.

I think we might need to always invoke the extension even if the array is empty. Otherwise, extensions that implement the API might have to guess why this API was not honored for particular scenarios.

Changing the array would cause the test method to be called with changed arguments.

I don't think that is a good idea. As far as I can tell, nobody has requested to be able to change the arguments provided to a test method. Thus, I would rather pass a copy of the arguments array to such an extension.

If the community clamors for such support, we could always change that aspect at a later date.

This leads to me wonder what the best name for such an extension API would be.

What would such an extension implementation actually do with the arguments?

Maybe "processing" is generic enough, but I'm wondering if there's something better out there. 😉

Lifecycle methods and test class constructors can also have arguments that get resolved by ParameterResolvers. Since I cannot think of a possible use case for those, I wouldn't call extensions. We could always add additional methods like processConstructorArguments(...) later.

That's true; however, we'll have to introduce a flag for methods in our ExecutableInvoker that specifies whether or not the method being invoked is a test method, and only if the flag is true would we want to invoke the extension API proposed here.

sbrannen avatar Sep 19 '18 12:09 sbrannen

The passed in arguments might be useful for some, but for Eclipse Jetty, just having ExtensionContext.getDisplayName() be valid/sane would be sufficient. Similar to what TestInfo.getDisplayName() shows while in the test itself.

joakime avatar Sep 19 '18 12:09 joakime

just having ExtensionContext.getDisplayName() be valid/sane would be sufficient.

When does that return something "invalid" for you?

Similar to what TestInfo.getDisplayName() shows while in the test itself.

The display name returned by ExtensionContext and TestInfo must actually be the same String, unmodified.

sbrannen avatar Sep 19 '18 13:09 sbrannen

If it's ok, I'd like to submit a WIP pull request for this after looking at it in relation to an extension I'm writing...

I wasn't too keen on the naming as ArgumentsProcessor either so set it up as follows for now:

public interface BeforeParameterizedTestExecutionCallback extends Extension {
	void beforeParameterizedTestExecution(ExtensionContext context, Object[] arguments);
}

On a related note, I also have a version of this that puts the parameter array into the extension's Store with method scope. Here, any existing callback can get to the parameters if required. With this method, a new callback obviously wouldn't be required.

paul-brooks avatar Oct 01 '18 07:10 paul-brooks

We talked about this issue in our team call today and agreed that we need to put some more thought into how the extension/API should look before continuing with the PR.

marcphilipp avatar Jul 19 '19 10:07 marcphilipp

An update on this would be great. Use case is custom reporting application where I want to aggregate based on the parameterized tests name and also pass the param.

The workaround is to have a pattern for how parametrized tests display names and then parsing it which is definitely suboptimal

foxfortune avatar Jan 09 '20 20:01 foxfortune

@foxfortune I abandoned this and wrote a quick bit of reflection to get the test parameters in the extension. Sure, it's brittle but works for me at the moment (on 5.5.2).

I call the following from beforeTestExecution. It returns an empty array if the test isn't parameterised :

private Object[] argumentsFrom(ExtensionContext context) {
        try {
            TestMethodTestDescriptor testDescriptor = invokeMethod(context, "getTestDescriptor", TestMethodTestDescriptor.class);
            TestTemplateInvocationContext invocationContext = fieldValue(testDescriptor, "invocationContext", TestTemplateInvocationContext.class);
            return fieldValue(invocationContext, "arguments", Object[].class);
        } catch (Exception e) {
            return new Object[0];
        }
    }

You'll need to replace invokeMethod and fieldValue with calls to your preferred reflection lib (or use the one built in to JUnit5 if you like it).

I'd prefer it if the parameters (if any) were just made available in the ExtensionContext subclass passed to the existing callback methods. I don't really see the point of having a new specific callback method as it doesn't feel like a lifecycle callback and not really where I want access to the parameters.

paul-brooks avatar Jan 10 '20 09:01 paul-brooks

@paul-brooks

Thanks for the pointer, what library reflection lib are you using? I don't really have a preference and am floundering trying to implement the JUnit5 built in one.

Also not sure I am reading the invokeMethod properly, however I dont see a getTestDescriptor method in ExtensionContext class which is where I think I am having trouble.

Also agree I would love the params to just be in the ExtensionContext

foxfortune avatar Jan 14 '20 01:01 foxfortune

I spoke too soon, I was able to finally get it to work! I used the JUnit 5 reflection in case anyone else would like to use it, here is the code, which I am sure could also be tightened up:

try {
            Method method = ReflectionUtils.findMethod(context.getClass(),"getTestDescriptor").orElse(null);
            TestMethodTestDescriptor descriptor = (TestMethodTestDescriptor) ReflectionUtils.invokeMethod(method, context);

            //Get the TestTemplateInvocationContext
            Field templateField = descriptor.getClass().getDeclaredField("invocationContext");
            templateField.setAccessible(true);
            TestTemplateInvocationContext template = (TestTemplateInvocationContext) templateField.get(descriptor);

            //Get the params finally
            Field argumentsField = template.getClass().getDeclaredField("arguments");
            argumentsField.setAccessible(true);
            Object[] params = (Object[]) argumentsField.get(template);

            return params;

        }catch(Exception e)
        {
            return new Object[0];
        }

foxfortune avatar Jan 14 '20 03:01 foxfortune

@foxfortune

Glad you got it working. My extension has it's own little reflection util as I find it easier to work with.

paul-brooks avatar Jan 14 '20 11:01 paul-brooks

@foxfortune and @paul-brooks ,thanks for sharing first of all. The code works fine for getting the argument values; however, how could we obtain the argument names? btw, @marcphilipp I also think that we need a clean way to obtain parameters and their names so external tools can eventually process them for reporting purposes

bitcoder avatar Mar 04 '21 13:03 bitcoder

@foxfortune and @paul-brooks ,thanks for sharing first of all. The code works fine for getting the argument values; however, how could we obtain the argument names? btw, @marcphilipp I also think that we need a clean way to obtain parameters and their names so external tools can eventually process them for reporting purposes

@bitcoder You can now use the InvocationInterceptor to obtain the parameter value array more easily, without using (your own) reflection, for example in Kotlin:

    // Called for @ParameterizedTest methods
    override fun interceptTestTemplateMethod(
        invocation: InvocationInterceptor.Invocation<Void>,
        invocationContext: ReflectiveInvocationContext<Method>,
        context: ExtensionContext
    ) {
        doSomethingWith(context, invocationContext.arguments.toTypedArray())
        invocation.proceed()
    }

    // Called for @Test methods
    override fun interceptTestMethod(
        invocation: InvocationInterceptor.Invocation<Void>,
        invocationContext: ReflectiveInvocationContext<Method>,
        context: ExtensionContext
    ) {
        doSomethingWith(context, invocationContext.arguments.toTypedArray())
        invocation.proceed()
    }

As for parameter names, I think this is only possible via reflection if you have compiled with -g to include debug information.

For my extension, I'm parsing the source files with Antlr so have access to the parameter names that way.

paul-brooks avatar Mar 04 '21 13:03 paul-brooks

Just to add another use case, it would be helpful to also make the parameterized invocation information available to the methods implemented for @EnabledIf / @DisabledIf, so that I can skip specific indexes when running a parameterized test.

Would the InvocationInterceptor pattern also work there?

mschechter-bellese avatar May 13 '21 13:05 mschechter-bellese

As for parameter names, I think this is only possible via reflection if you have compiled with -g to include debug information.

-g will include debug symbols in the .class files, which can be retrieved via libraries such as ASM that parse the actual bytecode, but that's more challenging than using the built-in java.lang.reflect.Parameter reflection APIs.

To obtain parameter names via the Parameter API, the -parameters flag must be supplied to the Java compiler.

sbrannen avatar May 13 '21 14:05 sbrannen

Just to add another use case, it would be helpful to also make the parameterized invocation information available to the methods implemented for @EnabledIf / @DisabledIf, so that I can skip specific indexes when running a parameterized test.

Would the InvocationInterceptor pattern also work there?

There is a pending PR in JUnit Pioneer which basically does this. From the PR's docs:

This extension can be used to selectively disable parameterized tests based on their parameter values (converted with toString()). The extension comes with three annotations, covering different use-cases:

  • @DisableIfAnyParameter, non-repeatable
  • @DisableIfAllParameters, non-repeatable
  • @DisableIfParameter, repeatable

The underyling DisableIfParameterExtension implements InvocationInterceptor.

beatngu13 avatar May 14 '21 22:05 beatngu13

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.

stale[bot] avatar Jun 19 '22 20:06 stale[bot]

This issue has been automatically closed due to inactivity. If you have a good use case for this feature, please feel free to reopen the issue.

stale[bot] avatar Jul 11 '22 20:07 stale[bot]

I have a case: I would like to implement an extension based on ExecutionCondition in which I would check if a test (case) from ParameterizedTest which has a parameter targetTestEnv should be run for the current test environment.

Sample code

@ParameterizedTest
@CsvSource(textBlock = """
           TC1,    targetTestEnv1,    testData1ForTestEnv1
           TC2,    targetTestEnv2,    testData1ForTestEnv2
""")
void test(String tcId, String targetTestEnv, String testData) {
    ...
}
public class AutoTargetTestEnvCheck implements ExecutionCondition {

    @Override
    public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext context) {
        String testEnv = System.getProperty("testEnv");        
        String testCaseTargetEnv = <need a way to get it>

        ...

        if (testCaseTargetEnv .equalsIgnoreCase(testEnv)) {
            return ConditionEvaluationResult.enabled("Current test env matches the target test env of the test case.");
        }

        return ConditionEvaluationResult
                .disabled("Current test env DOES NOT match the target test env of the test case.");
    }

}

BTW, the Disable Parameterized Test extensions from JUnit Pioneer mentioned by @beatngu13 do not work for my case. Because the filtering value i.e. testEnv will be provided through annotation attribute which must be a constant expression.

For example:

class MyTestClass {

    static final String testEnv = System.getProperty("testEnv");

    @DisableIfAnyArgument(contains = testEnv)
    @ParameterizedTest
    @CsvSource(textBlock = """
                    TC1,    targetTestEnv1,    testDataForEnv1,
                    TC2,    targetTestEnv2,    testDataForEnv2,
            """)
    void myTest(String tcId, String targetTestEnv, String testData) {
        ....
    }
}

The line @DisableIfAnyArgument(contains = testEnv ) will throw error The value for annotation attribute DisableIfAnyArgument.contains must be a constant expression

TimeInvestor avatar Jul 29 '22 06:07 TimeInvestor