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

Introduce extension API for accessing arguments passed to tests #1139

Open
rumpfc opened this issue Nov 3, 2017 · 29 comments
Open

Introduce extension API for accessing arguments passed to tests #1139

rumpfc opened this issue Nov 3, 2017 · 29 comments

Comments

@rumpfc
Copy link

rumpfc commented Nov 3, 2017

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

@marcphilipp
Copy link
Member

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?

@rumpfc
Copy link
Author

rumpfc commented Nov 3, 2017

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.

@marcphilipp
Copy link
Member

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?

@sbrannen
Copy link
Member

sbrannen commented Nov 3, 2017

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
Copy link
Member

sbrannen commented Nov 3, 2017

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.

@rumpfc
Copy link
Author

rumpfc commented Nov 3, 2017

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.

@marcphilipp
Copy link
Member

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?

@rumpfc
Copy link
Author

rumpfc commented Nov 3, 2017

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

@rumpfc
Copy link
Author

rumpfc commented Nov 13, 2017

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. 😉

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

@sbrannen sbrannen changed the title Extensions should get access to instances of arguments in a ParameterizedTest Introduce extension API for accessing arguments passed to tests May 23, 2018
@joakime
Copy link

joakime commented Sep 19, 2018

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.

@sormuras
Copy link
Member

sormuras commented Sep 19, 2018

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

Implementation outline as @marcphilipp described above in #1139 (comment)

@sbrannen
Copy link
Member

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.

@joakime
Copy link

joakime commented Sep 19, 2018

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.

@sbrannen
Copy link
Member

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.

@paul-brooks
Copy link

paul-brooks commented Oct 1, 2018

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.

@marcphilipp
Copy link
Member

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.

@foxfortune
Copy link

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

@paul-brooks
Copy link

paul-brooks commented Jan 10, 2020

@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.

@foxfortune
Copy link

foxfortune commented Jan 14, 2020

@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
Copy link

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];
        }

@paul-brooks
Copy link

@foxfortune

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

@bitcoder
Copy link

bitcoder commented Mar 4, 2021

@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

@paul-brooks
Copy link

paul-brooks commented Mar 4, 2021

@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.

@mschechter-bellese
Copy link

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?

@sbrannen
Copy link
Member

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.

@beatngu13
Copy link

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.

@stale
Copy link

stale bot commented Jun 19, 2022

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 stale bot added the status: stale label Jun 19, 2022
@stale
Copy link

stale bot commented Jul 11, 2022

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 stale bot closed this as completed Jul 11, 2022
@sbrannen sbrannen reopened this Jul 12, 2022
@TimeInvestor
Copy link

TimeInvestor commented Jul 29, 2022

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

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