Migrate from java.io to kotlinx-io

takahirom / roborazzi

Make JVM Android integration test visible 🤖📸

https://takahirom.github.io/roborazzi/

Apache License 2.0

704 stars 31 forks source link

Migrate from java.io to kotlinx-io #368

Closed eyedol closed 4 months ago

eyedol commented 4 months ago

Fixes #302

Changes:

[x] 1. Migrate these two classes CaptureResult and CaptureResults to use classes from kotlinx.io as initial implementation to evaluate if it can work.
[x] 2. Migrate the use of CaptureResult and CaptureResults across the project
[x] 3. Migrate iOS implementations to make use of the migrated CaptureResult and CaptureResults classes.
[x] 4. Migrate RoboCanvas to use classes from kotlinx.io.
[ ] 5. Migrate RoboCanvas use of kotlinx.io.
[x] 6. Make sure all existing unit tests are passing as expected.
[x] 7. Possible regression to make sure nothing is broken or introduced.

To make RoboConvas resuable interface across the different supported targets, the issue with differ needs to be fixed so we can move RoboCanvas to commonMain for reuse.

eyedol commented 4 months ago

@takahirom opened a PR to get something going. Currently using kotlinx.io lib. Seems straightforward though I'm hitting an issue with dropbox's differ library. It's not able to resolve iOS packages for some reasons. I saw the PR you made to address this but it seems it didn't resolve it entirely as even using a snapshot release of the differ, I still faced the same issue.

Issue:

:roborazzi-compose-ios:iosArm64Main: Could not find com.dropbox.differ:differ-iosarm64:0.0.2.
Required by:
    project :roborazzi-compose-ios > com.dropbox.differ:differ:0.0.2

Might you know why?

takahirom commented 4 months ago

@eyedol I'm not sure either. So, I asked the maintainer. https://github.com/dropbox/differ/pull/16#issuecomment-2119081492

eyedol commented 4 months ago

@eyedol I'm not sure either. So, I asked the maintainer. dropbox/differ#16 (comment)

@takahirom Opened a PR on the differ project. Hopefully it fixes it

eyedol commented 4 months ago

@takahirom should be ready for a merge. Will wait for the differ issue to be fixed then we can move RoboCanvas to common code.

takahirom commented 4 months ago

@eyedol Thanks! Sorry for the delay—I was focused on stabilizing Roborazzi. I'll merge this change. Thanks for your contribution! It seems you're aware of what we need to do to migrate iOS to the existing implementation. Could you create some issues to address this?

eyedol commented 4 months ago

Thanks for the merge. Sure. I made the ticket: https://github.com/takahirom/roborazzi/issues/383

takahirom commented 4 months ago

@eyedol Thanks!