MikeSchulze / gdUnit4

A Godot Unit Test Framework. Support for GDScript and C# unit testing
MIT License
565 stars 30 forks source link

GD-544: Spies (and mocks?) should support Callables #544

Closed mathrick closed 2 months ago

mathrick commented 2 months ago

Is your feature request related to a problem? Please describe. It is currently impossible to create a spy on a Callable (#542). However, it'd be useful to do so when writing tests for an API making use of callables. In that case, spying makes sense to ensure the protocol is maintained. In this test I'm not testing any logic inside the callable, I'm just testing that the protocol the callable is a part of is maintained. This is because the callable is not a part of the code being tested, it's always supplied by the user:

func test_resolve_promise():
    var callback = spy(func(x): pass)
    var promise = Promise.new().then(callback)
    promise.resolve(42)
    verify(callback, 1).call(42)

I don't know whether there's any real need for mocking callables, since it's trivial to create a lambda that does or returns whatever the test needs, and object mocks can override methods already. So it's probably safe to skip adding support for callables to Mock if it requires any extra work.

Describe the solution you'd like It should be possible to call spy() with a Callable, just as it's possible to do so with an Object. In this case, the following methods should probably be supported by verify():

And the following methods should be implemented, but not necessarily be targets for verify():

Describe alternatives you've considered When the callable is a lambda, it's normally possible do all the recording of calls manually, (e.g. by changing the pass part inside the callback above), but using spies would be a more natural choice since they already have the functionality to do this kind of thing.

MikeSchulze commented 2 months ago

@mathrick I started to adding support for Callable on spy.

Callable is a native class and do not inherit from Object. This means that it cannot be doubled and requires an additional wrapper class to achieve this. But all classes are based on Godot object where has already the function signature image

And do collide with the Callable class function signatures. For the call I was able to hack the signature to avoid typed values, but for callv it is impossible to override by a signature to map to Callable

The implementation is actual on draft, hopefully this is the only conflict.

mathrick commented 2 months ago

Thank you! I didn't have the time to look over your code before, sorry about that.

I was thinking initially that the spies themselves could be implemented as Callables, but that wouldn't really help with the conflicts, because they wouldn't be able to intercept .call() and .call_deferred() if that was the case either. I suppose there's not much that can be done here.

One issue I see with the spies being Objects is that using them will break any code that does an is Callable check. Which is going to be a real problem, since x is Callable is the only API GDScript exposes for checking if things are callables that I know of; there's no equivalent of Python's callable(x) and magic methods. I don't understand the way the implementation works entirely, so it's possible I'm misreading it, but have you accounted for that possibility?

MikeSchulze commented 2 months ago

That's a good point, yes with object as spy instance it will be a problem to recognize it as callable. But that shouldn't be a problem, I don't know why you should do an instance of here if the arguments are typed To avoid compiler conflicts, the function that takes the callable argument must be untyped, or the value must be typed as a Variant beforehand. Including the callable in an object is the only way to create the spy as we cannot inherit from a callable as it is not supported by Godot.