Migrate from java.io to kotlinx-io #368

eyedol · 2024-05-18T20:36:45Z

Fixes #302

Changes:

1. Migrate these two classes CaptureResult and CaptureResults to use classes from kotlinx.io as initial implementation to evaluate if it can work.
2. Migrate the use of CaptureResult and CaptureResults across the project
3. Migrate iOS implementations to make use of the migrated CaptureResult and CaptureResults classes.
4. Migrate RoboCanvas to use classes from kotlinx.io.
5. Migrate RoboCanvas use of kotlinx.io.
6. Make sure all existing unit tests are passing as expected.
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 · 2024-05-18T21:01:07Z

@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 · 2024-05-19T03:12:43Z

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

takahirom · 2024-05-19T03:16:50Z

include-build/roborazzi-core/src/commonMain/kotlin/com/github/takahirom/roborazzi/KotlinxIo.kt

+val Path.nameWithoutExtension: String
+  get() = name.substringBeforeLast(".")
+
+fun Path.relativeTo(base: Path): Path {


Looks great! When I tried this, I wasn't able to find a way to implement it. Could we add a small unit test for this?

Good call. I'll add unit test for it.

Added with 51e989a

eyedol · 2024-05-21T13:08:12Z

@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 · 2024-05-21T20:58:01Z

...e-build/roborazzi-core/src/commonMain/kotlin/com/github/takahirom/roborazzi/CaptureResult.kt

+  val compareFile: Path?
+  val actualFile: Path?
+  val goldenFile: Path?


@takahirom I was thinking of using String here to abstract away the Path class. That way we can decouple the use of the kotlinio package even further. Wdyt?

Since the JSON element is just a string, I think it's appropriate to use a string here. I don't have a strong opinion on this matter.

Thanks for your input. I ran away with your comment and went ahead and change it to String to abstract kotlinx-io class even away from the core code base.

See: e0d21c2

takahirom

Looks great!

CaptureResult and CaptureResutls in commonMain

… KotlinxIo Path base

At the moment it's not needed. Will be needed in commonMain when migration RoboCanvas

class

A way to abstract away from kotlinx-io classes from the core code base

eyedol · 2024-05-24T21:21:19Z

@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 · 2024-05-25T03:03:12Z

@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 · 2024-05-25T19:50:59Z

Thanks for the merge. Sure. I made the ticket: #383

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [io.github.takahirom.roborazzi](https://togithub.com/takahirom/roborazzi) | plugin | minor | `1.17.0` -> `1.18.0` | | [io.github.takahirom.roborazzi:roborazzi-junit-rule](https://togithub.com/takahirom/roborazzi) | dependencies | minor | `1.17.0` -> `1.18.0` | | [io.github.takahirom.roborazzi:roborazzi-compose](https://togithub.com/takahirom/roborazzi) | dependencies | minor | `1.17.0` -> `1.18.0` | | [io.github.takahirom.roborazzi:roborazzi](https://togithub.com/takahirom/roborazzi) | dependencies | minor | `1.17.0` -> `1.18.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>takahirom/roborazzi (io.github.takahirom.roborazzi)</summary> ### [`v1.18.0`](https://togithub.com/takahirom/roborazzi/releases/tag/1.18.0) [Compare Source](https://togithub.com/takahirom/roborazzi/compare/1.17.0...1.18.0) ##### Migrating from `java.io.File` to `kotlinx-io` **Note: We are not changing the API for users, so the captureRoboImage methods are still using the java.io.File class.** Roborazzi previously used `java.io.File` for saving and loading files. Although Roborazzi supports iOS, sharing much of the logic is challenging when using `java.io.File`. Therefore, we have migrated to `kotlinx-io`. We are attentive to the Kotlin versions our users employ. We upgraded from version 1.9.21 to 1.9.22 due to a dependency on kotlinx-io. Since Kotlin 1.9.22 was released last year, it should be compatible with most projects. Thank you, [@eyedol](https://togithub.com/eyedol), for your outstanding contribution! Your efforts are greatly appreciated. | \--- org.jetbrains.kotlinx:kotlinx-io-core:0.3.3 | +--- org.jetbrains.kotlinx:kotlinx-io-bytestring:0.3.3 | | \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 (*) | \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 (*) ##### What's Changed - Migrate from java.io to kotlinx-io by [@eyedol](https://togithub.com/eyedol) in [https://github.com/takahirom/roborazzi/pull/368](https://togithub.com/takahirom/roborazzi/pull/368) - Use kotlinx-io 0.3.3 to avoid updating Kotlin by [@takahirom](https://togithub.com/takahirom) in [https://github.com/takahirom/roborazzi/pull/381](https://togithub.com/takahirom/roborazzi/pull/381) ##### New Contributors - [@eyedol](https://togithub.com/eyedol) made their first contribution in [https://github.com/takahirom/roborazzi/pull/368](https://togithub.com/takahirom/roborazzi/pull/368) **Full Changelog**: takahirom/roborazzi@1.17.0...1.18.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate).

takahirom · 2024-05-27T00:51:04Z

@eyedol
Thanks!

eyedol marked this pull request as draft May 18, 2024 20:37

takahirom reviewed May 19, 2024

View reviewed changes

eyedol force-pushed the migration/migrateFromJavaIoToKotlinXIo branch from 0d1d723 to 51e989a Compare May 19, 2024 17:49

eyedol changed the title ~~[WIP] Migration/migrate from java io to kotlin x io~~ [WIP] Migration/migrate from java.io to kotlinx-io May 19, 2024

eyedol force-pushed the migration/migrateFromJavaIoToKotlinXIo branch from a3533c8 to fa48fd7 Compare May 20, 2024 19:53

eyedol commented May 21, 2024

View reviewed changes

eyedol requested a review from takahirom May 21, 2024 20:59

takahirom approved these changes May 23, 2024

View reviewed changes

eyedol marked this pull request as ready for review May 23, 2024 19:55

eyedol changed the title ~~[WIP] Migration/migrate from java.io to kotlinx-io~~ Migration/migrate from java.io to kotlinx-io May 23, 2024

eyedol force-pushed the migration/migrateFromJavaIoToKotlinXIo branch from e0d21c2 to 14dce92 Compare May 23, 2024 21:00

eyedol changed the title ~~Migration/migrate from java.io to kotlinx-io~~ Migrate from java.io to kotlinx-io May 24, 2024

eyedol added 15 commits May 24, 2024 11:39

Initial implementation of a crossplatform implementation of

217b53d

CaptureResult and CaptureResutls in commonMain

Implement an extension function that resolves a relative path

23f4e01

Migrate the use of File based CaptureResult and CaptureResults to use…

6ffab35

… KotlinxIo Path base

Rename kotlin io's based capature results to the original class name

3669240

Fix code clean ups

820f392

Move differ lib from commonMain from commonMainJvm for now

ed0b3a3

At the moment it's not needed. Will be needed in commonMain when migration RoboCanvas

Remove unused file

c4847b2

Use compileOnly to add kotlinx serialization json as originally done

538d8b1

Remove unused file

1528543

Fix Path's relativeTo implementation

cb85844

Add missing kotlinx json serialization lib to roborazzi-painter module

16b75a8

Add unit tests for KotlinxIo functions and extensions

5bbe7dd

Migrate iOS implementation of CaptureResults to the one from commonMain

c140600

Migrating RoboCanvas to use kotlin io Path class instead of the File

bb74e82

class

Replace kotlinx io Path class with String

0fd9f27

A way to abstract away from kotlinx-io classes from the core code base

eyedol force-pushed the migration/migrateFromJavaIoToKotlinXIo branch from 66c026d to 0fd9f27 Compare May 24, 2024 15:42

takahirom merged commit 847e398 into takahirom:main May 25, 2024
5 checks passed

eyedol deleted the migration/migrateFromJavaIoToKotlinXIo branch May 25, 2024 19:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate from java.io to kotlinx-io #368

Migrate from java.io to kotlinx-io #368

eyedol commented May 18, 2024 •

edited

Loading

eyedol commented May 18, 2024

takahirom commented May 19, 2024

takahirom May 19, 2024

eyedol May 19, 2024

eyedol May 19, 2024

eyedol commented May 21, 2024

eyedol May 21, 2024

takahirom May 23, 2024 •

edited

Loading

eyedol May 23, 2024 •

edited

Loading

takahirom left a comment

eyedol commented May 24, 2024

takahirom commented May 25, 2024

eyedol commented May 25, 2024

takahirom commented May 27, 2024

Migrate from java.io to kotlinx-io #368

Migrate from java.io to kotlinx-io #368

Conversation

eyedol commented May 18, 2024 • edited Loading

eyedol commented May 18, 2024

takahirom commented May 19, 2024

takahirom May 19, 2024

Choose a reason for hiding this comment

eyedol May 19, 2024

Choose a reason for hiding this comment

eyedol May 19, 2024

Choose a reason for hiding this comment

eyedol commented May 21, 2024

eyedol May 21, 2024

Choose a reason for hiding this comment

takahirom May 23, 2024 • edited Loading

Choose a reason for hiding this comment

eyedol May 23, 2024 • edited Loading

Choose a reason for hiding this comment

takahirom left a comment

Choose a reason for hiding this comment

eyedol commented May 24, 2024

takahirom commented May 25, 2024

eyedol commented May 25, 2024

takahirom commented May 27, 2024

eyedol commented May 18, 2024 •

edited

Loading

takahirom May 23, 2024 •

edited

Loading

eyedol May 23, 2024 •

edited

Loading