Ninja-Squad / springmockk

SpringMockK: MockBean and SpyBean, but for MockK instead of Mockito
Apache License 2.0
485 stars 31 forks source link

@SpykBean not reset / cleared between tests #85

Open moreginger opened 2 years ago

moreginger commented 2 years ago

In ClearMocksTestExecutionListener.clearMocks there is iteration through a set of beans registered with MockkCreatedBeans. However, only @MockKBeans are registered with this (during MockkPostProcessor.registerMock), @SpykBeans are not. This means that @SpykBeans are not reset / cleared between tests, which can cause some very hard to debug behavior.

I haven't fully understood the architecture but perhaps they could be registered in MockkPostProcessor.createSpyIfNecessary?

Note, as a workaround the injected @SpykBean can be cleared manually with:

@SpykBean lateinit var spy: Something
...
clearMocks((spy as Advised).targetSource.target!!)
jnizet commented 2 years ago

Hi @moreginger

I'm not sure why you think spy beans are not reset between tests. They are.

Here is a complete minimal demo showing that they are:

import com.ninjasquad.springmockk.SpykBean
import io.mockk.every
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.MethodOrderer.OrderAnnotation
import org.junit.jupiter.api.Order
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestMethodOrder
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.stereotype.Component

@SpringBootTest(classes = [Foo::class, Bar::class])
@TestMethodOrder(OrderAnnotation::class)
class SpyTest {
    @SpykBean
    lateinit var foo: Foo

    @Autowired
    lateinit var bar: Bar

    @Test
    @Order(1)
    fun `should greet`() {
        every { foo.greet(any()) } returns "Hi JB"

        assertThat(bar.sayHello()).isEqualTo("Hi JB")
    }

    @Test
    @Order(2)
    fun `should greet 2`() {
        assertThat(bar.sayHello()).isEqualTo("Hello Tim!")
    }
}

@Component
class Foo {
    fun greet(s: String) = "Hello $s!"
}

@Component
class Bar(val foo: Foo) {
    fun sayHello() = foo.greet("Tim")
}

The tests should pass, showing that the spy has been reset after the first test. Replace @SpykBean by @SpykBean(clear = MockkClear.NONE), and the second test won't pass anymore because then the spy won't be reset.

If you think there is a bug somewhere, please provide a complete minimal repro (inline code as above or as a zip file), and also test first that the same test using the native Mockito-based mock support coming with Spring Boot doesn't also fail.

moreginger commented 2 years ago

Well that's very interesting. I'm confident in what I've seen as the clearMocks trick fixed it, and I debugged through ClearMocksTestExecutionListener.clearMocks to see that they weren't cleared. Given that you aren't adding SpykBeans to MockkCreatedBeans, I guess it must be the second loop in the clearMocks method that is supposed to clear them?

I'll try to put together a test project for this.

moreginger commented 2 years ago

OK, I think I see what the difference is. The bean I'm spying has @Repository stereotype. This seems to mean that the bean has an extra $$EnhancerBySpringCGLIB proxy around it. When we get to this block in ClearMocksTestExecutionListener.clearMocks:

if (clear == MockkClear.get(bean)) {
  io.mockk.clearMocks(bean)
}

...then bean is the proxied bean, whereas the MockkClear was stored for the unproxied bean. Therefore we get MockkClear.NONE and the spy is never cleared.

If I change the stereotype to @Service then the bean isn't proxied and it gets cleared as you'd expect.

So I think the issue is that clearing doesn't work when beans are proxied for any reason, which includes @Repository stereotype.

(Let me know if you still need a test case for this... I think it should be easy to adapt one of your existing cases now?)

jnizet commented 2 years ago

Hi @moreginger

Yes, a test case would be nice.

bakomchik commented 1 year ago

I managed to make it work using next workaround @AfterEach fun afterEach() { val unProxiedMock = AopTestUtils.getTargetObject<MyRepository>(myRepository) clearMocks(unProxiedMock) }

LouisXhaferi commented 1 year ago

Hey, I had the same problem when working with FeignClient, which I suppose is similar to the Repository stereotype.

Here's an example. Spring Boot Integration Test

As to my understanding, the spy should have been reset between methods, but isn't. Same behavior also occurs, when I declare the FeignClient bean inside of the SpringBootTest, like

@SpringBootTest(webEnvironment = NONE)
class PingPongServiceTest(
    @Autowired private val service: PingPongService
) {
    @SpyBean
    private lateinit var client: PingPongFeignClient

    // ...rest
}

Same also occurs when work with Mocks.

I feel like this is not how it is supposed to work, but correct me if I'm wrong. Is sharing Spies and Mocks illegal?

borjab commented 9 months ago

Had the same problem here with version 3.1.2 and @SpykBean. In my case, the spied bean was proxied by having the @Transactional annotation.

The workaround also worked:

    @AfterEach
    override fun tearDown() {
        // Workaround for issue https://github.com/Ninja-Squad/springmockk/issues/85
        clearMocks(AopTestUtils.getTargetObject<FindRoomsRepository>(findRoomsSpringDataRepository))
    }
jantobola commented 1 month ago

I can confirm that the problem, in general, is with Spring CGLIB proxies.

fun get(mock: Any): MockkClear {
    return entries.firstOrNull { it.mockRef.refersTo(mock) }?.clearMode ?: NONE
}

This is always evaluated to NONE.

image