Brightify / Cuckoo

Boilerplate-free mocking framework for Swift!
MIT License
1.67k stars 175 forks source link

async/await is not fully supported in protocol with associated types #441

Closed mecoFarid closed 1 year ago

mecoFarid commented 1 year ago

Although there is a merged PR here related to async/await #410, the feature is not fully supported, at least, in protocols with associated types.

Example:

protocol Datasource<T>{
    associatedtype T
    func get() async -> Result<T, Error>
}

Snippet from generated class:

Screen Shot 2022-12-30 at 21 03 18

IDE complains saying:

Invalid conversion from 'async' function of type '() async -> Result<T, any Error>' to synchronous function type '() -> Result<T, any Error>'

Solution: I have added required keywords to generated code, it is fixed:

Screen Shot 2022-12-30 at 21 03 40
MatyasKriz commented 1 year ago

That's a good find, @mecoFarid, thank you. 🙂 I'm currently working on integrating Tuist with the project in order to make it much easier to test changes and start contributing to Cuckoo. I think I'll have it out by next week. If you want to try, however, you may fix this as the project stands.

mecoFarid commented 1 year ago

@MatyasKriz The fix is very easy. I have made changes and followed the Contribution part. Every time I build the project it just skips my changes in CuckooGenerator.

Out of curiosity I totally removed the CuckooGenerator project from workspace then tried building the project with Cmd + U. And it still generates GeneratedMocks even though I have removed CuckooGenerator from workspace.

I checked the run script and apparently there is download option too. I disabled the network even though I was NOT providing --downloadoption and yet it still generates the GeneratedMocks.

Is there anything I am missing out?

NOTE: This is the command I am executing./.run generate --testable Cuckoo --exclude ExcludedTestClass,ExcludedProtocol --output Tests/Swift/Generated/GeneratedMocks.swift Tests/Swift/Source/*.swift

MatyasKriz commented 1 year ago

Hey there, @mecoFarid. Yeah, that's one of the pitfalls of the current project structure. You need to pass the argument --clean right after ./run so that the generator is rebuilt. So ./run --clean generate ... should work. 🙂

I have revised the project structure to cut out the run script altogether and to rebuild the generator every time tests are ran for good measure. This means that the tests will take a little longer to run, but there will be no pitfalls where a contributor changes some generator code and doesn't see the desired results, as has been your case.

I'll have to do some additional testing to make sure I haven't broken project distribution, but I'd like to merge this ASAP as I hope it will make it easier for contributors to navigate in the project.

mecoFarid commented 1 year ago

Great, thanks @MatyasKriz. I created MR. One thing I noticed is there is no CI on Github to run all tests when MR is created.

It would be really great if we had CI to run either when creating MR or before merging. This would assure there is no broken API. If we do not have, I can work on it.

MR: https://github.com/Brightify/Cuckoo/pull/442

MatyasKriz commented 1 year ago

Absolutely agree, @mecoFarid. I've been thinking about utilizing the GitHub Actions, but I haven't used it yet and so I don't know if it's possible.

hlung commented 1 year ago

What release will this fix be in?

MatyasKriz commented 1 year ago

Hey, @hlung, I was waiting on https://github.com/Brightify/Cuckoo/pull/454 to be merged, but I guess I can release a version now. Remember that you can always target the master branch if you need some changes that are merged but not released yet. 🙂

You can expect this fix in version 1.10.0 shortly.