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

Add documentation for usage with Mockk and other mock libraries #583

Closed
LeoColman opened this issue Jan 15, 2019 · 19 comments · Fixed by #1423
Closed

Add documentation for usage with Mockk and other mock libraries #583

LeoColman opened this issue Jan 15, 2019 · 19 comments · Fixed by #1423
Labels
enhancement ✨ Suggestions for adding new features or improving existing ones. good-first-issue 👶 Suitable for newcomers looking to contribute to the project.
Milestone

Comments

@LeoColman
Copy link
Member

LeoColman commented Jan 15, 2019

Recently, in Mockk's slack and KotlinTest's slack, some users have reported troubles when using Mockk with KotlinTest, due to the mockk context not being cleared after tests.

This is due to the isolation mode being incorrect for the use case, as the user normally wants the class to be instantiated once per test (InstancePerTest isolation).

As Mockk is a very used mocking library for Kotlin, among other libraries, maybe we should create a specific part in the documentation for explaining this, or a paragraph in isolation mode.

Users don't associate directly that the "Bug" in their mocks using KotlinTest is due to isolation mode. Usually because the default mode for JUnit is single instance per test, and we don't do that, so writing the same test "bugs" their code because of it.

What do you think, @sksamuel ?

Also summoning @oleksiyp

@LeoColman LeoColman added the enhancement ✨ Suggestions for adding new features or improving existing ones. label Jan 15, 2019
@dave08
Copy link
Contributor

dave08 commented Jan 15, 2019

Side point to possibly consider: Resetting mocks meaning re-initializing them which isn't always so efficient.

One way or the other it might be worth it to consider a standard way to "hook" into the test executions to reset existing (in Mockk there's the clearMocks and clearXXXX methods for this) or re-initialize mocks somehow... maybe with some kind of TestListener for this purpose (it could be initialized with a list of each type of mock to clear...)?

Or maybe supporting some kind of @Mockk annotation to initialize/clear mocks declared on the class (maybe implemented by a TestListener for this).

@sksamuel
Copy link
Member

So taking what @dave08 says into account, perhaps we could have an extension MockKExtension which runs re-init code rather than creating new mocks (but also update docs to show both ways?)

@LeoColman
Copy link
Member Author

LeoColman commented Jan 15, 2019

The problem is when should mocks be cleared. After Test? After Spec?

@sksamuel
Copy link
Member

Yes that's a good point.

@LeoColman
Copy link
Member Author

I think that if we want to keep the default expected behavior for most of mockk users, it would be to clear after test. I think it's important to document both ways too, as a default behavior won't be good enough for some users

@sksamuel
Copy link
Member

By setting the isolation mode, your mocks will be created correctly and will fit in with the natural state of the rest of your spec variables. There's nothing more required than that to make it work properly.

But we can look at an extension which could reset mocks (after every leaf test or after every test - two extensions just extending from a parent), in case you don't want to use a different isolation mode.

@dave08
Copy link
Contributor

dave08 commented Jan 15, 2019

Apart from resetting mocks, for actual creation there's this from Mockk documentation (for JUnit), there's relaxed settings to consider and spies, I wonder if this MockKAnnotations.init can be reused in KotlinTest, then the TestListener could receive it in the contructor along with a enum value of when it should run (before test or spec) or something similar:

...
  @MockK
  lateinit var car1: Car

  @RelaxedMockK
  lateinit var car2: Car

  @MockK(relaxUnitFun = true)
  lateinit var car3: Car

  @SpyK
  var car4 = Car()
  
  @InjectMockKs
  var trafficSystem = TrafficSystem()
  
  @Before
  fun setUp() = MockKAnnotations.init(this, relaxUnitFun = true) // turn relaxUnitFun on for all mocks
...

Now that I think of it, this is simpler than resetting.. since there's lots of types to handle (constructorMocks, staticMocks, and regular mocks) with differing clear methods. Clearing might be better served with real before/afterXXX DSL methods, but it still needs to be decided if you want to include those...

@LeoColman
Copy link
Member Author

LeoColman commented Jan 15, 2019

Initing mocks via annotations probably work just fine with MockKAnnotations, as it will scan the class in runtime.

with differing clear methods

I think one can use unmockkAll or clearMocks for clearing everything without an issue.

However, I don't believe that the usual way of using Mockk is by using annotations. I personally have never used them, and I only see them in somewhat legacy systems that don't have a good dependency injection dynamic.

The usual way I use mockks is with

val mocked: Database = mockk(relaxed = true)

init {
    "Foo" {
        every { mocked.getData() } returns null
    } 
}

@LeoColman
Copy link
Member Author

I think that the issue we want to solve is with these variables that are mocked and when they should be reset. I think we should be somewhat agnostic to Mockk's implementation

@dave08
Copy link
Contributor

dave08 commented Jan 15, 2019

I also don't use the annotation form, but by initializing the mocks the regular way, it's either resetting the mocks or using one class per test...

I like to stay away from DI's in regular unit tests (not integration tests where there's no choice...), so the annotations are maybe an option.

Another option could be Delegate properties to do the same (somewhat like Spek's by memoized) and decide the scope test or spec there by mockk<Database>(Scope.AfterTest).

Btw you mean:
clearAllMocks | clears regular, object, static and constructor mocks

@oleksiyp
Copy link

MockK supports a lot of ways to clear/unmockk and this is very confusing.

Extension as well exists, but it does not clear any mocks, just initialize to new instances:

class MockKExtension : TestInstancePostProcessor, ParameterResolver {
...
    override fun postProcessTestInstance(testInstance: Any, context: ExtensionContext) {
        MockKAnnotations.init(testInstance)
    }
...
}

@oleksiyp
Copy link

clearAllMocks doesn't work for parallel tests

@oleksiyp
Copy link

One possible thing to do is create an extension function for some scope that kotlintest is using and attach it somehow.

@sksamuel
Copy link
Member

@Kerooker lets create documentation now, showing an example with an IsolationMode, because 3.2 is about to be released. We can do something cleverer for 3.3 if required.

@JustinTullgren
Copy link
Contributor

Documentation would be appreciated. It would be nice to see a constructor injection example without relaxed = true as that is a "sensible default" provided by mockk.

override fun isInstancePerTest() = true
val mocked: Database = mockk() {
  every { getData() } returns "foo"
}
var victim: Service = AService(mocked)

init {
    "Foo" {
        victim.foo() shouldBe "foo"
    } 
}

@sksamuel sksamuel added the good-first-issue 👶 Suitable for newcomers looking to contribute to the project. label Sep 4, 2019
@sksamuel sksamuel added this to the 4.0 milestone Sep 4, 2019
@stale
Copy link

stale bot commented Mar 10, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 10, 2020
@stale stale bot closed this as completed Mar 17, 2020
@OliverCulleyDeLange
Copy link

OliverCulleyDeLange commented Apr 12, 2020

Reviving this issue from the dead, as some documentation around kotest + Mockk would still be nice. I've just started trying out a load of test & mock frameworks for Android and kotest & mockk seem nice given their kotlin based nature. I struggled initially setting them up together. I'll share with you my failed naive attempts so you know what a newbie might do after reading the getting started readme...

These two fail with kotlin.UninitializedPropertyAccessException: lateinit property thing has not been initialized

class FirstAttemptSpec : FreeSpec({
    @MockK
    lateinit var thing: Thing

    beforeTest {
        MockKAnnotations.init(this)
    }
    "Test something"{
        every { thing.name } returns "Blah"
        thing.name shouldBe "Blah"
    }
})

@ExtendWith(MockKExtension::class)
class SecondAttemptSpec : FreeSpec({
    @MockK
    lateinit var thing: Thing
    "Test something"{
        every { thing.name } returns "Blah"
        thing.name shouldBe "Blah"
    }
})

On reading up on kotest styles, it was mentioned that the two style are functionally equivalent. However I tried the other style (using the init{} block) and suddenly everything worked as expected:

class WorkingSpec : FreeSpec() {
    @MockK
    lateinit var thing: Thing
    init {
        beforeTest {
            MockKAnnotations.init(this)
        }
        "Test something"{
            every { thing.name } returns "Blah"
            thing.name shouldBe "Blah"
        }
    }
}

Not quite though, as the @ExtendWith(MockKExtension::class) annotation still doesn't work...

This is either a bug or just somethin that needs documenting for newbies like me :)

@sksamuel sksamuel removed the wontfix label Apr 12, 2020
@sksamuel
Copy link
Member

sksamuel commented Apr 12, 2020 via email

@OliverCulleyDeLange
Copy link

I made a gist about getting up and running with kotest and mockk if anyone stubles upon this like i did and wants some copy paste code

@LeoColman LeoColman reopened this Apr 29, 2020
LeoColman added a commit that referenced this issue Apr 29, 2020
LeoColman added a commit that referenced this issue Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ Suggestions for adding new features or improving existing ones. good-first-issue 👶 Suitable for newcomers looking to contribute to the project.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants