Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare for K2 #349

Merged
merged 3 commits into from
Jun 17, 2024
Merged

Prepare for K2 #349

merged 3 commits into from
Jun 17, 2024

Conversation

eygraber
Copy link
Contributor

@eygraber eygraber commented Feb 7, 2024

Was investigating using KSP 2 and figured I'd start tracking this separate from that.

@eygraber
Copy link
Contributor Author

eygraber commented Feb 7, 2024

The integration tests got kind of screwy.

The issue is that currently KSP is configured to generate code for each platform. It is also configured to generate code for commonMain, however there is no code in commonMain because common-companion is a commonTest source set, and so there is no common code generated.

In K2, common source sets can't see code from platform source sets, so the tests in common and common-companion don't see the generated platform code.

It's expected that common code cannot reference generated code in the compilation of platform code. Generated codes are treated as platform code and K2 explicitly disallow references from common to platform (you'll have to use expect/actual).

To get around that I separated common and common-companion into the common code from them which becomes a commonMain source set, and their tests, which remain a commonTest source set.

Because it's using commonTest I believe tests still run for each platform, it's just that the generated code will be in the commonMain source set.

The only other solution that would work would be to have code and tests duplicated for each platform. You can't make multiple source sets out of the same directory, so there would probably have to be a script that generates them at runtime, or do some kind of trickery with symlinks.

@evant
Copy link
Owner

evant commented Feb 7, 2024

The integration test setup is known to be kludge, it's only set up that way because I couldn't find another way to make it work.

Because it's using commonTest I believe tests still run for each platform, it's just that the generated code will be in the commonMain source set.

👍 That feels right to me and closer to the ideal way I wanted it set up.

@eygraber eygraber marked this pull request as draft February 8, 2024 02:34
@eygraber eygraber mentioned this pull request Feb 8, 2024
@eygraber
Copy link
Contributor Author

@evant are you OK with extracting the integration test changes and merging into main now? It should work fine on current main and would prevent conflicts as new tests are added.

@evant
Copy link
Owner

evant commented Feb 15, 2024

I was just going to ask to do that, these changes are quite large and it would be easier to review if the integration test changes were pulled out

@eygraber
Copy link
Contributor Author

#355

@eygraber
Copy link
Contributor Author

@evant this should be good to go, other than the question of whether the library should impose Kotlin 2.0 on consumers.

@evant evant mentioned this pull request May 22, 2024
@evant
Copy link
Owner

evant commented May 22, 2024

I think I want to get a release out with the current changes before merging this but happy to do right after

@CoreFloDev
Copy link

And so that's blocking the current release?

@evant
Copy link
Owner

evant commented Jun 14, 2024

Released, so this can be moved out of draft

@eygraber eygraber marked this pull request as ready for review June 14, 2024 21:06
@evant evant merged commit 31c086e into evant:main Jun 17, 2024
2 checks passed
@eygraber eygraber deleted the k2 branch June 17, 2024 16:33
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.

3 participants