Skip to content

Conversation

JanTie
Copy link
Collaborator

@JanTie JanTie commented Jun 13, 2024

Tests on ios may still fail, due to potential invalid path. We may have to take a look into that as soon as we decide whether we want to run the tests on all platforms.

The problem why the targets could not be added before was laying in the file-reading library that was used.
See goncalossilva/kotlinx-resources#101. As the issue has not been adressed yet, i swapped the library with okio. As okios wasm impl is not stable yet i left that one with a todo for now

Closes #4

Comment on lines 3 to 5
import okio.FileSystem

actual val FILE_SYSTEM: FileSystem = FileSystem.SYSTEM
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding to the non-working ios/apple targets, I think we have to do it like here and change the path.

I can invastigate in the next days.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently, by now it somehow seems like the ios tests would run 🤔
Can you confirm on your system? I also used the fakeFileSystem, which allows execution of wasm targets now.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually not, when running the task allTests
also wasm tests don't work

Screenshot 2024-07-11 at 20 13 00

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further I look into finding a setup to read resources from every test target, I think it's more work when it helps 🙄

What about moving every json resource into a multiline string literal within the corresponding test. Or global values within Kotlin test directory?

I'll try to make a commit as a proposal

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yikes... there are big files... I may think this can lead to OOM errors sooner or later

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to close this PR in favor of #29. Since adding json data to kotlin files is not possible.

@JanTie JanTie requested a review from elcolto July 7, 2024 18:57
@elcolto elcolto closed this Jul 22, 2024
@elcolto
Copy link
Owner

elcolto commented Jul 22, 2024

Closed PR, because JS target was added with #29 again. WASM target still to come

@elcolto elcolto deleted the re-add-targets branch July 22, 2024 17:06
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

Successfully merging this pull request may close these issues.

Re-add JS and WASM targets
2 participants