kotest / kotest-extensions-spring

Kotest extension for Spring framework
Apache License 2.0
33 stars 3 forks source link

Lifecycle hooks relying on mutating spring test context are failing #118

Open javolek opened 5 months ago

javolek commented 5 months ago

I discovered this one with the MockMvc. The default behaviour of MockMvc tests ist that it prints the reques/response summary when the test is failing. We observed this behaviour in our older projects where we are using JUnit, but not in the new one with kotest.

I tried to dig deep into source code to find a reason for this and here ist what I found:

The class responsible for this behaviour is actually test executor listener org.springframework.boot.test.autoconfigure.web.servlet.MockMvcPrintOnlyOnFailureTestExecutionListener which checks for an existence of the test exception on the instance of TestContext class. This requires the same instance of the test context to be present for both the test and execution listener. The test context manager (org.springframework.test.context.TestContextManager) stores the test context using ThreadLocal and since kotest executes tests and lifecycle hooks as a separate coroutines, the instance of the TestContext will be mostly different.

There may be further test execution listener relying on this behaviour and therefore failing.

I don't think this can be fixed on the kotest-extension side, but I didn't find anybody mentioning this problem elswhere so I reported this here, since this may really be an issue that is worth at least mentioning in the documentation if not fixed somehow. It is also possible that I am missing something and there is a workarround for this that I just didn't find.

Kantis commented 5 months ago

@javolek it should not be the case that the wrong TestContext is used. Kotest creates a TestContextManager per Spec and stores it as a Coroutine context element, meaning it'll follow the coroutine correcly.

https://github.com/kotest/kotest-extensions-spring/blob/65da860127eef7b7f01de5436539cdf8fb5ac6b9/src/main/kotlin/io/kotest/extensions/spring/SpringTestExtension.kt#L58

Could you provide a small reproducer which demonstrates the issue you're seeing?

javolek commented 5 months ago

Hi Emil, you are right, there is only one instance of TestContextManager per spec, but this instance stores TestContext instance it manages internally with ThreadLocal. This means that there will a single instance of TestContext for each thread regardless there is only a single instance of the TestContextManager across all those threads. This is internal working of TestContextManager class, that's why I think that you can't fix it in kostest. I will try to put a simple project together in a next few days to demonstrate this behaviour.

javolek commented 5 months ago

While writing the tests for a demo , I found out interesting things:

  1. The kotest runs test on a single thread by default. This means that ThreadLocal is not a Problem in this case. The reason why MockMvcPrintOnlyOnFailureTestExecutionListener is not working as expected is simpler than that - the Spring-Extension is ignoring the Exception that happens inside the test instead of passing them to the lifecycle hooks of the TestContextManager, which modifies the test context of the underlying instance of the TestContext class. See the last parameter of the call at: https://github.com/kotest/kotest-extensions-spring/blob/65da860127eef7b7f01de5436539cdf8fb5ac6b9/src/main/kotlin/io/kotest/extensions/spring/SpringTestExtension.kt#L75 The fix should be quite simple as the TestResult is in scope and it should contain the exception thrown on error.

  2. If one try to run the test in more than one thread then it behaves as I suspected - the instance of the TestContext is different. As long as the tests are not modifying the test context and the lifecycle hooks are running all in the same thread this should also be no problem.

Here you can find the demo for the behaviour above: https://github.com/javolek/kotest-demo see the tests.

Another think I found not really correct in the Spring-Extension was the order of the lifecycle hooks after a test case:

      if (testCase.isApplicable()) {
         testContextManager().afterTestMethod(testCase.spec, methodName, null as Throwable?)
         testContextManager().afterTestExecution(testCase.spec, methodName, null as Throwable?)
      }

According to documentation afterTestExecution method should be called before framework specific after-callbacks and the afterTestMethod after them. So the order should be the other way arround. Actually the framework-specific callbacks should be called between these lifecycle hooks, but this does not seem to be possible using kotest-extension-api.