Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support throwable computed properties #74

Open
vadimkrutovlv opened this issue Dec 23, 2023 · 4 comments
Open

Support throwable computed properties #74

vadimkrutovlv opened this issue Dec 23, 2023 · 4 comments

Comments

@vadimkrutovlv
Copy link

vadimkrutovlv commented Dec 23, 2023

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
Copy link
Contributor

dafurman commented Jan 7, 2024

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
Copy link
Author

@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
Copy link
Contributor

dafurman commented Jan 8, 2024

@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
Copy link
Author

vadimkrutovlv commented Jan 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants