Spying on Spys does not work properly

[Vampire]

To kill some PIT mutants, I added a Spy that verifies some condition and then uses callRealMethod() in a setup() method.

In some tests I also added a Spy on the same object that also calls callRealMethod().

so in the end it was like

testee.field = Spy(testee.field)
testee.field.someMethod() >> {
    // verify some condition here
    callRealMethod()
}
testee.field = Spy(testee.field)
testee.field.someMethod() >> {
    // verify some other condition here
    callRealMethod()
}

Now if testee.field.someMethod() is called I would have expected that the second spy tests "some other condition", then delegates to the first spy which tests "some condition" and then delegates to the real method.

Unfortunately, the first spy was skipped and after testing "some other condition" the second spy delegated to the real method directly.

This causes the mutant to not be killed properly (or rather a timeout due to a deadlock which costs more time than would have been necessary).

I worked around it by instead using a helper method that gets a closure, but it would be nice if it would actually work like expected. The workaround looks something like:

def prepareLocks(Closure someMethodInterceptor = { callRealMethod() }) {
    testee.field = Spy(testee.field)
    testee.field.someMethod() >> {
        // verify some condition here
        someMethodInterceptor.tap { it.delegate = owner.delegate }.call()
    }
}
def test() {
    given:
        prepareLocks {
            // verify some other condition here
            callRealMethod()
        }
    // other test content
}

[leonard84]

I don't quite get the use-case for spying on spies to be honest. I don't think that we will "fix" something here, as it is quite the edgecase and the verification could be handled in the first interaction. Spies should be avoided if possible, as is noted in the docs.

Think twice before using this feature. It might be better to change the design of the code under specification.

[Vampire]

This is a very simplified example with which you can reproduce the issue and not the real-world example.
In the real-world example I can not verify in the first interaction like in this simplified example and I do avoid spies where possible (unless I missed something).
The first half of the example was in the setup() method, the second half in the two tests where it was necessary.

In the meantime I pushed my project, so you can have a look at the real-world example, currently of course with the work-around I described:

https://github.com/Vampire/command-framework/blob/eb87c56da8f46b6886d8411a74de11a71ba8f174/src/test/groovy/net/kautler/command/api/CommandHandlerTest.groovy#L542-L566
and
https://github.com/Vampire/command-framework/blob/eb87c56da8f46b6886d8411a74de11a71ba8f174/src/test/groovy/net/kautler/command/api/CommandHandlerTest.groovy#L1043-L1060

The production class uses double-checked locking for lazy and optional initialization of a field.
If I run PIT, it mutates the code, which includes removing calls to the lock() and unlock() methods which then causes deadlocks and the PIT run has to wait for timeout to kill the mutant which is a big waste of time.

Due to that, I wanted to replace the write lock and read lock by spies, that decorate the real ones and fail-fast if the write lock gets requested without first releasing the read lock to still kill the mutant but not having to wait for the timeout.

I wanted to simply do this for all tests in a setup() method to make sure no test has to wait for a deadlock.

But I wanted to reach 100% branch coverage and to reach that with the double-checked locking, I needed to have another Spy around the write lock spy, added in the test, so that I can set the initialized field between the outer and inner ifs and before the write lock got acquired.

I can of course do it like I do it now, but I think it is hackier than necessary if the spying on spies would work properly.
Now I have to call the prepareLocks method manually everywhere instead of setting this up in a setup method and only adjusting in the two tests where it is necessary.

---

I would really appreciate if this would be fixed.
But if it does not get fixed, then maybe at least an error should be thrown if you try to spy on a spy, as currently it works but just behaves unexpectedly.

[leonard84]

Spies (in 1.3) work by creating an instrumented version of the target objects class and then creating a new instance and copying all the state over, so you don't have the wrapping of objects you think you are getting. Mock/Spy interaction matching works by target identity, since you are basically creating two separate instances they don't have any relationship hierarchy.

Adding a check, that the target of a spy is not a mock object would make sense.

[Vampire]

I see, sad, but yes, then at least an error should come up. :-)