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

Use test scoped ExtensionContext for test specific callbacks in ParameterResolver and TestInstancePreConstructCallback #3445

Open
5 tasks
JojOatXGME opened this issue Sep 1, 2023 · 3 comments

Comments

@JojOatXGME
Copy link

JojOatXGME commented Sep 1, 2023

I am currently writing a small extension which implements ParameterResolver to provide a few parameters which need to be shut down after the test. After reading the documentation, my first intuition was to write the extension like this:

final class MyExtension implements ParameterResolver {
    private static final Namespace NAMESPACE = Namespace.create(MyExtension.class);

    @Override
    public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
        return parameterContext.getParameter().getType() == Parameter.class;
    }

    @Override
    public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) {
        return extensionContext
                .getStore(NAMESPACE)
                .getOrComputeIfAbsent(ParameterWrapper.class)
                .parameter;
    }

    private static final class ParameterWrapper implements CloseableResource {
        private final Parameter parameter;

        private ParameterWrapper() {
            this.parameter = new Parameter();
        }

        @Override
        public void close() {
            parameter.close();
        }
    }
}

The problem with that solution is that the ExtensionContext is scoped for the whole test class when resolving arguments for the test class constructor, although the test uses TestInstance.Lifecycle.PER_METHOD. As a consequence, my example above will use the same parameter for all tests, instead of creating a new one per test. The documentation does not describe a lot of details about the lifecycle of each ExtensionContext, therefore I cannot declare it as a bug, but this behavior is very unexpected in my opinion. Since the callbacks are on the level of the individual tests, I expected the ExtensionContext to be scoped to the test as well. The same problem applies to TestInstancePreConstructCallback. I haven't tested TestInstanceFactory, but I guess it is also affected.

The obvious workaround is to also add a TestInstancePreDestroyCallback which closes the parameter and removes it from the store. Note that the implementation of that method is not trivial because in contrast to the methods above, this callback receives a child store which is scoped to the test. Unfortunately, this means that calling remove on the given store directly instead of iterating over the parents would not work.

    @Override
    public void preDestroyTestInstance(ExtensionContext context) {
        ParameterWrapper wrapper = null;
        do {
            wrapper = context.getStore(NAMESPACE).remove(ParameterWrapper.class, ParameterWrapper.class);
            context = context.getParent().orElse(null);
        } while (context != null && wrapper == null);
        if (wrapper != null) wrapper.close();
    }

Deliverables

-- or --

  • Documentation of the limitation in the Javadoc of the interfaces or callbacks.

Related issues

@marcphilipp
Copy link
Member

We agree that this is a valid use case that should be simpler to accomplish. We'd like to investigate whether it's feasible to inject the method-level ExtensionContext if Lifecycle.PER_METHOD is used and, if so, for which extension interfaces.

@JojOatXGME
Copy link
Author

Great to hear. If there is anything I can help with, I am happy to hear it.

@victorherraiz
Copy link

victorherraiz commented Nov 19, 2023

TestInstancePreConstructCallback and TestInstancePreDestroyCallback are not symmetric, the context of the latter is the child of the former. Therefore, removecalls from the latter fails, the store is in the parent.
TestInstancePreConstructCallback recives a context that outlives the test using the default config when the extenxion is configured in the test class.
In my opinion, ParameterResolver and TestInstancePreConstructCallback should receive a context just for the current test if the life cycle is configured per method.

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

5 participants