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

Replace SynchronizedObject reference with kotlin/Any in class Metadata #404

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

mvicsokolova
Copy link
Collaborator

@mvicsokolova
Copy link
Collaborator Author

mvicsokolova commented Feb 27, 2024

In this PR I've also added a new test: a multi-module project where kotlinx-atomicfu is only applied to one module instead of the root project application. The bug with from the issue was only reproduced with cross-module usage of the class that inherited SynchronizedObject.

This test revealed another problem that was fixed here #405, based on the changes from the current PR.

val atomicfuDir = buildDir.resolve("classes/atomicfu/")
(if (atomicfuDir.exists() && atomicfuDir.isDirectory) atomicfuDir else buildDir).let {
val atomicfuDirExists = atomicfuDir.exists() && atomicfuDir.isDirectory
Copy link
Contributor

Choose a reason for hiding this comment

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

In tests, we should know for sure how classes were transformed and what directories are expected to be there.
I recommend reducing the amount of conditional logic here and instead explicitly pass a directory and a flag to filter out the metadata from the test.
Otherwise, we may end up in a situation when build directories layout will change, this test won't be update accidentally, and it'll no longer catch the problem with afu-references as it will always filter the metadata.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, this got very complicated. I've separated these cases

@mvicsokolova
Copy link
Collaborator Author

I've fixed the tests: in case of bytecode transformation we should only check classes in classes/atomicfu directory, otherwise classes/atomicfu-orig directory with original classes will be checked as well.

@mvicsokolova mvicsokolova merged commit 37b4bdf into develop Feb 29, 2024
1 check passed
@mvicsokolova mvicsokolova deleted the metadata-lost branch February 29, 2024 12:55
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.

None yet

2 participants