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

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

Open
javolek opened this issue May 22, 2024 · 3 comments
Open

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

javolek opened this issue May 22, 2024 · 3 comments

Comments

@javolek
Copy link

javolek commented May 22, 2024

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

Kantis commented May 23, 2024

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

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

@javolek
Copy link
Author

javolek commented May 24, 2024

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

javolek commented May 24, 2024

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:

    testContextManager().afterTestMethod(testCase.spec, methodName, null as Throwable?)

    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.

Kantis added a commit that referenced this issue May 27, 2024
Partial fix for issue #118.

Need to figure out some way to handle that Spring uses a
ThreadLocal for keeping track of the TestContext to fix last problem.
Kantis added a commit that referenced this issue May 27, 2024
* Correcing callback order and propagate error

Partial fix for issue #118.

Need to figure out some way to handle that Spring uses a
ThreadLocal for keeping track of the TestContext to fix last problem.

* Adding test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants