Cuckoo icon indicating copy to clipboard operation
Cuckoo copied to clipboard

Mock restricts subclasses for method argument

Open flopshot opened this issue 1 year ago • 3 comments

I have a protocol as follows

protocol Networkable {
    func asyncLoad<T: Decodable>(request: Request<T>) async throws -> T
    func asyncLoadResponse<T: Decodable>(request: Request<T>) async throws -> HeaderResponse<T>
}

Here is as an example of a type derived from Request<T>

open class Request<A: Decodable> {
    let httpMethod: HttpMethod
    let path: String
    let queryParams: [String: Any]?
    let headers: [String: String]

    /// Standard request initializer
    ///
    /// - Parameters:
    ///   - httpMethod: HTTP Method for request (GET, POST, PUT, PATCH, DELETE) - Includes body if applicable to the method type.
    ///   - path: Path to append to base URL, which is provided by chosen URLSession
    ///   - queryParams: Query Params to append to the end of the URL
    ///   - headers: Additional headers to set outside of the initial headers configured in chosen URLSession
    /// - Throws: Networking errors related to being unable to create a valid URLRequest
    public required init(httpMethod: HttpMethod, path: String, queryParams: [String: Any]? = nil, headers: [String: String] = [:]) {
        self.httpMethod = httpMethod
        self.path = path
        self.queryParams = queryParams
        self.headers = headers
    }
}

public class GetProductBySlugRequest: Request<Product>   {
    public var productSlug: String?
    public var color: String?
    public var responseId: String?

    public convenience init(productSlug: String, color: String, responseId: String?) {
        
        var params: [String: Any] = [:]
        params["color"] = color

        var headers: [String: String] = [:]
        headers[NuulyHeaders.region] = GlobalSettings.geoLocation?.region
        headers[NuulyHeaders.country] = GlobalSettings.geoLocation?.country
        headers[NuulyHeaders.urbnResponseId] = responseId

        self.init(httpMethod: .get, path: "product/slug/\(productSlug)", queryParams: params, headers: headers)

        self.productSlug = productSlug
        self.color = color
        self.responseId = responseId
    }
}

The mock generated from Cuckoo gives this error when using in tests

        let productRequest = GetProductBySlugRequest(productSlug: slug, color: color, responseId: responseId)
        stub(networkService) { netStub in
           // ❌ Instance method 'asyncLoadResponse(request:)' requires the types 'GetProductBySlugRequest' and 'Request<Product>' be equivalent
            when(netStub.asyncLoadResponse(request: productRequest)).thenReturn(productResponse)
        }

The problem lies with the M1 Matchable type constraints generated by the mock for the asyncLoadResponse method

         func asyncLoadResponse<M1: Cuckoo.Matchable, T: Decodable>(request: M1) -> Cuckoo.ProtocolStubThrowingFunction<(Request<T>), HeaderResponse<T>> where M1.MatchedType == Request<T> /*THE PROBLEM IS THIS TYPE CONSTRAINT*/ {
            let matchers: [Cuckoo.ParameterMatcher<(Request<T>)>] = [wrap(matchable: request) { $0  }]
            return .init(stub: cuckoo_manager.createStub(for: MockNetworkable.self, method:
    """
    asyncLoadResponse(request: Request<T>) async throws -> HeaderResponse<T>
    """, parameterMatchers: matchers))
        }

if I change the method to this below then it works

         func asyncLoadResponse<M1: Cuckoo.Matchable, T: Decodable>(request: M1) -> Cuckoo.ProtocolStubThrowingFunction<(Request<T>), HeaderResponse<T>> where M1.MatchedType : Request<T> /*MAKE THIS TYPE CONSTRAINT DERIVABLE RATHER THAN EQUIVALENT*/ {
            let matchers: [Cuckoo.ParameterMatcher<(Request<T>)>] = [wrap(matchable: request) { $0 as! M1.MatchedType /*FORCE CASTING  HERE IS FINE AS THIS WILL ALWAYS SUCCEED*/ }]
            return .init(stub: cuckoo_manager.createStub(for: MockNetworkable.self, method:
    """
    asyncLoadResponse(request: Request<T>) async throws -> HeaderResponse<T>
    """, parameterMatchers: matchers))
        }

My question is, Is there is a way to change this in the mock generator for this library? If not, would you be willing to accept a pull request to somehow allow this? Would a pull request to change this be feasible? I'd like some direction before I start working on a fix for this, if even possible.

Thanks

flopshot avatar Dec 09 '23 21:12 flopshot

Hey, @flopshot. The derivable constraint syntax is not compatible with concrete types, so you can't simply write M1.MatchedType: Int, it complains that Int is neither a protocol nor a class. So then we'd have to compare the type to our type database to check whether the type is a class or protocol.

It's certainly possible to implement this behavior to add even stronger generics support. If you're up for it, please start on the feature/swiftsyntax branch. It's 2.0 release of the library & generator, even though it's not ready for release just yet, but getting close.

Asking about any questions or problems getting the project running is appreciated, as that branch contains a lot of major changes throughout the project, hopefully for the better. 😄 The contributing section in README should still apply, so you can start off there.

MatyasKriz avatar Dec 12 '23 22:12 MatyasKriz

Thanks for the response. In the end, we couldn't simply use $0 as! M1.MatchedType because we may want the stub accept different subclasses in the same test, which wasn't possible with that force cast to the Monomorphic type MatchedType

Also, the compiler error was misleading, because what actually was needed was just to make Request conform to Matchable

We ended up using type erasure to solve our problem and leave the generated code as is

extension Request : Matchable {
    public var matcher: Cuckoo.ParameterMatcher<Request> {
        ParameterMatcher<Request>(matchesFunction: { (request: Request) -> Bool in
            if let productByChoiceIdsRequestRHS = request as? GetProductsByChoiceIdsRequest,
               let productByChoiceIdsRequestLHS = self.self as? GetProductsByChoiceIdsRequest {
                return productByChoiceIdsRequestRHS.choiceIds == productByChoiceIdsRequestLHS.choiceIds
            }

            if let getRecommendationTrayRequestRHS = request as? GetRecommendationTrayRequest,
               let getRecommendationTrayRequestLHS = self.self as? GetRecommendationTrayRequest {
                return getRecommendationTrayRequestLHS.path == getRecommendationTrayRequestRHS.path &&
                getRecommendationTrayRequestLHS.itemsPerPage == getRecommendationTrayRequestRHS.itemsPerPage &&
                getRecommendationTrayRequestLHS.categorySlug == getRecommendationTrayRequestRHS.categorySlug &&
                getRecommendationTrayRequestLHS.contextIds == getRecommendationTrayRequestRHS.contextIds &&
                getRecommendationTrayRequestLHS.excludeChoiceIds == getRecommendationTrayRequestRHS.excludeChoiceIds &&
                getRecommendationTrayRequestLHS.clientFilters.keys.sorted() == getRecommendationTrayRequestRHS.clientFilters.keys.sorted()
            }
 //...
}

Doing this fixed the issue, for using this in our large code base. Which brings me to my next point, it would be better if the compiler error was more helpful by explicitly saying Request<T> does not conform to Matchable instead of Instance method 'asyncLoadResponse(request:)' requires the types 'GetProductBySlugRequest' and 'Request<Product>' be equivalent Which I would be happy to make a pull request for, as this was very annoying and led me down a bad path.

If anyone has this problem, the better solutions, though long term, is to use a new class called EquatableRequest<T>, conform it to Equatable and simply inherit any requests you'd like to match in tests to EquatableRequest<T> then you could simple have this single Matchable conformance

extension EquatableRequest : Matchable {
    public var matcher: Cuckoo.ParameterMatcher<Request> {
        ParameterMatcher<Request>(matchesFunction: { (request: Request) -> Bool in
            request == self.self
}

flopshot avatar Dec 15 '23 21:12 flopshot

Asking about any questions or problems getting the project running is appreciated, as that branch contains a lot of major changes throughout the project, hopefully for the better. 😄 The contributing section in README should still apply, so you can start off there.

Thanks I will give it a shot next week

flopshot avatar Dec 15 '23 21:12 flopshot

2.0 has been released and this hopefully fixed? If not, please reopen. 🙂

MatyasKriz avatar Apr 23 '24 15:04 MatyasKriz