Skip to content
New issue

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

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

Already on GitHub? # to your account

TestInstancePreDestroyCallback does not follow the wrapping behavior of callbacks in lifecycle PER_CLASS #2209

Closed
joerg1985 opened this issue Mar 9, 2020 · 1 comment

Comments

@joerg1985
Copy link

joerg1985 commented Mar 9, 2020

The extension TestInstancePreDestroyCallback does not follow the wrapping behavior of Extensions if the lifecycle is PER_CLASS, as described in https://junit.org/junit5/docs/snapshot/user-guide/#extensions-execution-order-wrapping-behavior

The expected order with lifecycle PER_CLASS

  • Extension1.postProcessTestInstance
  • Extension2.postProcessTestInstance
  • Extension1.beforeAll
  • Extension2.beforeAll
  • Extension2.afterAll
  • Extension1.afterAll
  • Extension2.preDestroyTestInstance
  • Extension1.preDestroyTestInstance

The order using JUnit 5.6.0. with lifecycle PER_CLASS

  • Extension1.postProcessTestInstance
  • Extension2.postProcessTestInstance
  • Extension1.beforeAll
  • Extension2.beforeAll
  • Extension2.afterAll
  • Extension1.afterAll
  • Extension1.preDestroyTestInstance
  • Extension2.preDestroyTestInstance

The order using JUnit 5.6.0. with lifecycle PER_METHOD is as expected

  • Extension1.beforeAll
  • Extension2.beforeAll
  • Extension1.postProcessTestInstance
  • Extension2.postProcessTestInstance
  • Extension2.preDestroyTestInstance
  • Extension1.preDestroyTestInstance
  • Extension2.afterAll
  • Extension1.afterAll

Steps to reproduce

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@ExtendWith({ Dummy2Test.Extension1.class, Dummy2Test.Extension2.class })
public class Dummy2Test {
    private static final Logger LOG = Logger.getLogger(Dummy2Test.class.getName());
    
    @Test
    public void justATest() {
        LOG.fine("@Test");
    }
    
    public static class Extension1 implements BeforeAllCallback, AfterAllCallback , TestInstancePostProcessor,TestInstancePreDestroyCallback {

        @Override
        public void beforeAll(ExtensionContext ec) throws Exception {
            LOG.warning("Extension1.beforeAll");
        }

        @Override
        public void afterAll(ExtensionContext ec) throws Exception {
            LOG.warning("Extension1.afterAll");
        }
        
        @Override
        public void postProcessTestInstance(Object o, ExtensionContext ec) throws Exception {
            LOG.warning("Extension1.postProcessTestInstance");
        }

        @Override
        public void preDestroyTestInstance(ExtensionContext ec) throws Exception {
            LOG.warning("Extension1.preDestroyTestInstance");
        }
        
    }
    
    public static class Extension2 implements BeforeAllCallback, AfterAllCallback , TestInstancePostProcessor, TestInstancePreDestroyCallback {

        @Override
        public void beforeAll(ExtensionContext ec) throws Exception {
            LOG.warning("Extension2.beforeAll");
        }

        @Override
        public void afterAll(ExtensionContext ec) throws Exception {
            LOG.warning("Extension2.afterAll");
        }
        
        @Override
        public void postProcessTestInstance(Object o, ExtensionContext ec) throws Exception {
            LOG.warning("Extension2.postProcessTestInstance");
        }

        @Override
        public void preDestroyTestInstance(ExtensionContext ec) throws Exception {
            LOG.warning("Extension2.preDestroyTestInstance");
        }
    }
}

Context

  • Used versions (Jupiter/Vintage/Platform): 5.6.0
  • Build Tool/IDE: maven
@sbrannen
Copy link
Member

sbrannen commented Mar 9, 2020

Thanks for raising the issue and providing such detail.

This definitely appears to be a bug in the implementation of ClassBasedTestDescriptor.invokeTestInstancePreDestroyCallback(JupiterEngineExecutionContext), since it should in fact be using ExtensionRegistry.getReversedExtensions(Class<E>).

@sbrannen sbrannen self-assigned this Mar 9, 2020
@sbrannen sbrannen added this to the 5.7 M1 milestone Mar 9, 2020
@marcphilipp marcphilipp modified the milestones: 5.7 M1, 5.6.1 Mar 12, 2020
sbrannen added a commit that referenced this issue Mar 12, 2020
Prior to this commit, when using PER_CLASS test instance lifecycle
semantics, TestInstancePreDestroyCallback extensions were invoked in
the order in which they were registered, which violates the general
"wrapping" principle for extensions invoked after a container or test.

This commit fixes this by invoking TestInstancePreDestroyCallback
extensions in reverse registration order when PER_CLASS test instance
lifecycle semantics have been configured.

Fixes #2209
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants