Fix stubbing by chained calls on collections (#565)

[AlexKrupa]

Related links

Issue: #565
Changes that affected expected behavior: #387, #536

Introduction

Any feedback is appreciated. I'm completely new to the internals of MockK, so please excuse any shortcomings of my solutions.

I recommend checking commits separately, because there are alternative solutions. I'll clean those up in case a solution is chosen.

Tests pass for both solutions.

Explanation

This is an attempt at fixing stubbing by chained calls on collection, such as:

val mock = mockk<SomeClass>()
every { mock.map.get("foo") } returns 0

Expected behavior broke when (Jvm)AnyValueGenerator started returning concrete instances for collection types. Before those branches existed, the generator would fall back to orInstantiateVia lambda, which – when passed from RecordingState.call – created a temporary mock on invocation.

Solutions

Solution 1: Return temporary mocks for collection types in RecordingState.call

Encapsulating the change here helps avoid concerning other consumers of AnyValueGenerator, making the scope of the change much smaller than solution 2.

A possible disadvantage is that these collection (or other) types have to be kept in sync between RecordingState and AnyValueGenerator. Perhaps we could obtain expose those types from AnyValueGenerator instead?

Solution 2: Add flag to configure AnyValueGenerator's behavior

If interfering in AnyValueGenerator isn't a concern, we can pass a flag to indicate whether generated objects should be mocks. This adds more complexity than solution 1.

[Raibaz]

Thanks a lot for taking the time to investigate this and to provide two different solutions!

In fact, when I wrote #387 and then #536 I only took into account the AnyValueGenerator usage made by the defaultAnswer function in MockKStub, as the case detailed by #565 was not covered by any tests and I didn't notice I was breaking it.

I personally like solution 1 better, because it looks more self-contained and pertaining only the case you are fixing, without impacting other clients of the AnyValueGenerator.

I don't think it's a big deal having to keep the collection types in sync between the RecordingState and the AnyValueGenerator, as we already cover most of the existing collection types and I don't think many more will be added in the future.

If you can please revert aeb8baa, or forcepush 5146a16 so that solution 1 is the one being merged, I'll merge this straight away.

Thanks again!

[AlexKrupa]

Thanks for replying! I reverted the commit and squashed the other changes into one.