Matejkob / swift-spyable

Swift macro that simplifies and automates the process of creating spies for testing
MIT License
409 stars 37 forks source link

Support throwable computed properties #74

Open vadimkrutovlv opened 8 months ago

vadimkrutovlv commented 8 months ago

Currently I'm super happy to use this macro, it allows me to drop third party dependency an the fact that mocks are generated in the main target allows me to reduce a boilerplate code for view model mocks for swift ui previews.

However I already run multiple times in a situation when I could use a computed property instead of method but unfortunately I can't specify ThrowableError as for the methods. Consider following code.

@Spyable
protocol FacebookLoginRepositoryProtocol {
    var isRegistered: Bool { get async throws }
}

I would really love to have something like this:

let repositoryMock = FacebookLoginRepositoryProtocolSpy()
repositoryMock.isRegisteredThrowableError = MyError.error

As for now the only code macro generates is this:

class FacebookLoginRepositoryProtocolSpy: FacebookLoginRepositoryProtocol {
    var isRegistered: Bool {
        get {
            underlyingIsRegistered
        }
        set {
            underlyingIsRegistered = newValue
        }
    }

    var underlyingIsRegistered: (Bool)!
}

Would it be possible to implement something like I mentioned above? Sorry I didn't check source code that much and maybe there are some limitations about which I don't know.

Alternatively I still can use a method which is not a big deal, but I like to keep my codebase clean as much as possible.

dafurman commented 8 months ago

For reference, AutoMockable generates syntax that basically looks like this, for the example code provided:

class FacebookLoginRepositoryProtocolMock: FacebookLoginRepositoryProtocol {
    var isRegisteredCallsCount = 0
    var isRegisteredCalled: Bool {
        return isRegisteredCallsCount > 0
    }

    var isRegistered: Bool {
        get async throws {
            isRegisteredCallsCount += 1
            if let error = isRegisteredThrowableError {
                throw error
            }
            if let isRegisteredClosure = isRegisteredClosure {
                return try await isRegisteredClosure()
            } else {
                return underlyingIsRegistered
            }
        }
    }
    var underlyingIsRegistered: Bool!
    var isRegisteredThrowableError: Error?
    var isRegisteredClosure: (() async throws -> Bool)?
}

The simplest approach would probably be to see if we can replicate this output to get the same functionality, though we could probably leave out the <thing>CallsCount and <thing>Called for the purposes of resolving this issue.

This doesn't seem like it'd be too bad, so I'd be happy to take a look at putting up a PR when I get some more time.

vadimkrutovlv commented 8 months ago

@dafurman thanks for you reply, you mean remove propertyCallsCount completely? Honestly I think propertyCallsCount is super useful to verify that exact method or property in this case was called expected amount of times, I'd say it's crucial when it comes to performance. So if possible it would be really great to keep at least Count.

dafurman commented 8 months ago

@vadimkrutovlv Oh I agree in seeing the value in it!

Because I was talking about the Sourcery equivalent, I just mean that since callcount and called don't currently on properties with Spyable, for the sake of fulfillment of this particular issue, I'd suggest deferring addition of that to a separate issue, just to keep changes smaller and focused on supporting throwable computed properties for now.

I think it'd be easier to track adding <thing>callsCount and <thing>called to computed properties in a separate issue.

vadimkrutovlv commented 8 months ago

Oh I see, sorry my bad, I misunderstood you. Yeah it's absolutely fine to track it separately, thank you so much!