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

Migrate performance tests to occur on tests project #227

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Migrate performance tests to occur on tests project #227

merged 1 commit into from
Sep 5, 2023

Conversation

JakeWharton
Copy link
Collaborator

This will open up the ability to assert on JS and native which simply isn't possible with the Kotlin Compiler Testing infrastructure.

This will open up the ability to assert on JS and native which simply isn't possible with the Kotlin Compiler Testing infrastructure.
@JakeWharton
Copy link
Collaborator Author

Here's the followup with a JS improvement that this now enables testing: https://github.com/JakeWharton/Poko/compare/jw.migrate-performance.2023-09-05...JakeWharton:Poko:jw.js-equals.2023-09-05

@@ -20,4 +20,5 @@ include(
":poko-annotations",
":poko-gradle-plugin",
":poko-tests",
":poko-tests:performance",
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a technical reason a new module is needed, or just preference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The tests only run once and they run on the JVM. I suppose we could keep them in the compiler module since it's only a test dependency.

Copy link
Owner

Choose a reason for hiding this comment

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

Oh right. I'm good with them here then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another option is a disconnected project which consumes the artifacts through a repo-local Maven install. This was what I set up first, but the signing configuration always wants to sign things so it doesn't work and I didn't want to mess with it. This is what I usually do on other projects since you have infinite flexibility due to the disconnected project. As a downside, it's a bit more setup.

Copy link
Owner

Choose a reason for hiding this comment

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

This is all new to me and it's not obvious to me what we're missing out on due to inflexibility. But if we are missing out on something I'm open to refactor the signing logic to not get in the way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One example would be that you can vary the Kotlin version. If you want to test / support a range of versions at once, this would be an easy way to test those bounds.

Another thing that you can do is vary compilation properties. So instead of having to add a duplicate target for supporting K2 like you did recently, the project could be compiled twice, once with K2 and once without.

I'm not sure we're losing anything yet, but it's something to consider long term depending on how many more things come up.

@drewhamilton drewhamilton merged commit 5dc1902 into drewhamilton:main Sep 5, 2023
5 checks passed
@JakeWharton JakeWharton deleted the jw.migrate-performance.2023-09-05 branch September 6, 2023 01:37
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.

2 participants