spock icon indicating copy to clipboard operation
spock copied to clipboard

Mocking in given block is ignored when mocked in setup method

Open huehnerlady opened this issue 5 years ago • 14 comments

Issue description

Currently you can declare a certain mocked behaviour in the setup method, which will then be used for every test in the class. If you want to override that behaviour you HAVE to put it in the then block. This is very confusing to people as it is not the natural flow they would describe the test

How to reproduce

groovy example of a test

interface MyInterface {
    int myMethod()

}

class MyInterfaceTest extends Specification {
    def myInterface = Mock(MyInterface)

    def setup() {
        myInterface.myMethod() >> 1
    }

    def "testing regular behaviour"() {
        when:
         def result = myInterface.myMethod()
        then:
         result == 1
     }

    def "testing exceptional behaviour"() {
        given:
         myInterface.myMethod() >> 2
        when:
         def result = myInterface.myMethod()
        then:
         result == 2
      } 
}

Tests in words:

myMethod should always return 1

testing regular behaviour
When myMethod is called
Then the result is 1

testing exceptional behaviour
Given myMethod returns 2
When I call myMethod
Then the result is 2

In this case the second test will fail You would have to write something like

    def "testing exceptional behaviour"() {
        when:
         def result = myInterface.myMethod()
        then:
         _ * myInterface.myMethod() >> 2
         result == 2
      } 

In words:

When I call myMethod
Then any call of myMethod returns 2
and the result is 2

To me the first wording of the test is a natural flow in comparison to the second version. So my proposal is to add the given block to override the declarations of the setup-Method.

It is Important though, that the then-Block should still always win as you need to verify how often methods are called as well.

huehnerlady avatar Jan 15 '19 08:01 huehnerlady

Thanks @huehnerlady +1

EduardoSolanas avatar Jan 15 '19 10:01 EduardoSolanas

@robfletcher @leonard84 could you maybe have a look at this as you were looking at the similar issue this one was created from?

huehnerlady avatar Jan 17 '19 16:01 huehnerlady

Is there the possibility to get some answer here? It is open for 2 months now...

huehnerlady avatar Mar 15 '19 12:03 huehnerlady

@leonard84 @Vampire @MartyIX can please someone take a statement to my issue? It is open since middle of January and nobody answered which is really disappointing!

huehnerlady avatar Mar 29 '19 09:03 huehnerlady

@huehnerlady I'm not affiliated with Spock in any way. I have just contributed a few patches. You can try https://gitter.im/spockframework/spock though :)

MartyIX avatar Mar 29 '19 09:03 MartyIX

@MartyIX sorry I just took the people that contributed in the last few months, but thanks I will try that :)

huehnerlady avatar Mar 29 '19 09:03 huehnerlady

Same for me, it is pretty rude to randomly ping contributors. If at all, ping team members. But actually, this is an open source project where people contribute their sparse spare time, so you cannot always expect timely answers. You could fix the behaviour and send a patch though. ;-)

Vampire avatar Mar 29 '19 17:03 Vampire

@Vampire I am really sorry, I did not think of that. I did try to address the issue as recommended by MartyIX.

but the patch is a good idea as well, thanks

huehnerlady avatar Apr 01 '19 07:04 huehnerlady

@huehnerlady sorry for the late answer. We are working on spock-2.0 at the moment, so please wait for contributions until that branch is merged to master, otherwise it will create unnecessary effort.

Regarding your issue, as I said in #321 there is a current working way to override the setup behavior, so I see the priority not so high.

Furthermore, the semantics are unclear. From a user endpoint I could read two things happening for given: myInterface.myMethod() >> 2:

  • invocations of myMethod return 1 for the first, and 2 for the second invocation
  • it always returns 2

leonard84 avatar Apr 03 '19 20:04 leonard84

the problem is as described in the other issue that it is confusing the way it is at the moment, as you have to override things in the then block instead of the given block.

To me the behaviour is clear as you do not want to add functionality but override existing functionality, so it would be the second approach.

Keep in mind that this is how junit works as well.

When will the 2.0 branch be merged into master?

Many thanks

huehnerlady avatar Apr 04 '19 11:04 huehnerlady

@huehnerlady

Keep in mind that this is how junit works as well.

JUnit does not provide any mocking functionality that I know of. Did you mean Mockito?

When will the 2.0 branch be merged into master?

Let me quote Blizzard here, when it's done ;) as @Vampire already mentioned above we work on this in our spare time.

leonard84 avatar Apr 04 '19 17:04 leonard84

Btw. the "when it's done" happened already. :-)

Vampire avatar Oct 03 '20 16:10 Vampire

Does someone know in which version this was fixed? I searched the three issues for a fix-version and the 2.0, 2.1, 2.2 release notes for any mention of #1308, #1311 or #962 but could not find anything unfortunately.

mfriess2 avatar Jul 26 '22 15:07 mfriess2

I have opened the PR #1762 to try to fix this issue, with my expectation of the API. So the override of interactions will only work, if both do not specify a cardinalitiy and match exactly the same InvocationConstraints. IMHO this is mostly the intention of such a code.

If I write

3 * myInterface.myMethod() >> 1

I want these three invocations. But if I write:

myInterface.myMethod() >> 1

I do not care, give me that last declaration in this order setup => given=>when => then. Note: then-Interations will always have precedence. For more details and tests, see PR documentation and the JavaMockInteractionOverridesSpec.

This would fix the issue here, but preserve most of the original behavior of Spock.

This can possibly be a breaking change, when someone relies on the existing strange behavior, but I think this can be neglected. None of the existing spock tests in this repo broke. If we really want, we could add a configuration flag to preserve the old behavior, if someone is relying on it.

AndreasTu avatar Aug 14 '23 16:08 AndreasTu