Kolos65 / Mockable

A Swift macro driven auto-mocking library.
MIT License
199 stars 14 forks source link

Optional Properties should be return nil #15

Closed emadhegab closed 3 months ago

emadhegab commented 5 months ago

I have a protocol

protocol MyServicing { var name: String? var age: Int }

when i use Mockable on it.

func testAge() { given(mockMyServicing).age.willReturn(20) }

if the implementation uses "name" , i will get the error Fatal error: No return value found for member name.

I think it should be defaulted to nil if i don't provide it . in case of huge protocols. it would be tedious to provide all optional if they don't really relate to the testing objective.. WDYT?

xmanu commented 4 months ago

I would also love to have strict behavior. This could be made configurable. For example mockK has a relaxed mode (strict, meaning all calls have to be mocked, even when returning null) is their default.

See https://mockk.io/#relaxed-mock

I would love if the following would be possible:

@Mockable
protocol MyServicing {
var name: String?
var age: Int
func voidFunc()
}

func testName() {
let mockMyServicing = MockMyServicing(strict: true) // Would have a default value of false so the current constructor and behavior stays valid and consistent!
given(mockMyServicing).name.willReturn(nil)
// This would also should make method calls with void return strict:
given(mockMyServicing).voidFunc().willReturn(()) // or as a shorthand .justRuns()
}

The relaxed mode could automatically return nil and allow calls to void functions.

This could hopefully be implemented by a relatively small change to the macro generation and Mocker.

I might take a look at this when I have time and try to do a PR, if this is something that @Kolos65 finds acceptable...

xmanu commented 3 months ago

I have created a PR, that implements both the strict mode and the lenient behavior for optional properties.

Kolos65 commented 3 months ago

@xmanu Fully strict behaviour is now available in 0.0.4

xmanu commented 3 months ago

This ticket should probably not be completed though, as relaxed mode is still open...

emadhegab commented 3 months ago

ok. sorry i mis read your comment above :) good job man :)

Kolos65 commented 3 months ago

@emadhegab I already have some ideas regarding your issue, it will be addressed shortly 😊

hainayanda commented 3 months ago

I think it will be better if the user can choose between strict and lenient modes. I personally feel that the non-throwing void method shouldn't need explicit mocking.

I guess if the strict will be a default behavior, better to keep the lenient modes configurable, so many users like me can skip mocking the non-throwing void method that did nothing:

// for strict mode
let strictService = MockMyService(mode: .strict)

// for full relaxed mode
let relaxedService = MockMyService(mode: .relaxed)

// for custom lenient mode
let lenientService = MockMyService(mode: . lenient(for: [.nonThrowingVoid]))

or maybe we can add it as attribute parameters:

@Mockable(defaultMode: .relaxed)
protocol MyService { ... }
Kolos65 commented 3 months ago

Relaxed mode (WIP): #34

xmanu commented 3 months ago

That looks really flexible and well thought through.

One could argue about some of the defaults, I for example would expect false, 0, and 0.0 to be defaults respectively, but I couldn't argue for or against that...

So LGTM!

Kolos65 commented 3 months ago

I was leaning towards 1 to prevent zero division errors, but it might be worth making those configurable too.

Also I will add a global configuration option so the default policy can be changed for every mock.

xmanu commented 3 months ago

That's a good argument! Not sure if this has to be configurable. One can always explicitly provide the mocked return value...

emadhegab commented 3 months ago

would be great to add to the PR a read me to that new behaviours as well. what is default and how to change it .

Kolos65 commented 3 months ago

@emadhegab @xmanu @hainayanda

Relaxed Mode is now available in 0.0.5