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

Nested classes in ProtoBuf produce empty array for native release binary #2608

Closed
whyoleg opened this issue Mar 20, 2024 · 5 comments
Closed

Comments

@whyoleg
Copy link
Contributor

whyoleg commented Mar 20, 2024

Describe the bug

Nested classes in ProtoBuf produce empty array for native release binary.
No such issue with Cbor or with any other target and Native debug binary. (all cases are provided in repository with reproducer)

To Reproduce

@Serializable
data class OuterClass(val data: InnerClass)

@Serializable
data class InnerClass(val data: String)

@OptIn(ExperimentalSerializationApi::class, ExperimentalStdlibApi::class)
class SerializationTest {
    @Test
    fun testProtobufNested() {
        val model = OuterClass(InnerClass("something"))

        val data = ProtoBuf.encodeToByteArray(OuterClass.serializer(), model)
        // `data` is an empty array on Native in release mode
        assertEquals("0a0b0a09736f6d657468696e67", data.toHexString()) 
    }
}

Repository with reproducer: https://github.com/whyoleg/kx-serialization-protobuf-reproducer
To reproduce: ./gradlew macosArm64ReleaseTest or [TARGET_NAME]ReleaseTest for other targets, or just check to run all tests (to compare behaviour)

Expected behavior

Correct serialised value is returned independent to the target or debug/release mode.

Environment

  • Kotlin version: 1.9.23 (same for 2.0.0-Beta5)
  • Library version: 1.6.3
  • Kotlin platforms: Native

Note: I can create the same issue in Kotlin YT, as most likely the issue is there, but thought it would be better to create it here to keep track and may be introduce some kind of tests in repository.

@sandwwraith
Copy link
Member

Thanks for report. It happens for me for both linuxX64ReleaseTest on Linux machine and macosArm64ReleaseTest on Apple Silicon machine. Looks like the Native issue though.

@whyoleg
Copy link
Contributor Author

whyoleg commented Apr 25, 2024

just in case: does it make sense additionally run tests in RELEASE mode (and possibly in other kotlinx.* libraries), to be proactive with such bugs?
If you think it could be useful, I'm ready to provide PR.

@sandwwraith
Copy link
Member

Yes, it makes sense. I'll ask Native team additionally of the best way to do so. We are now refactoring our build scripts (#2654), so you'll have to wait with your PR a little bit :)

@whyoleg
Copy link
Contributor Author

whyoleg commented Apr 26, 2024

okay! Thanks, I will wait for #2654.
At my side I have the following build setup for runnings native tests in release mode. (BTW, this is how I've found this issue)
Created tasks will look like macosArm64ReleaseTest in addition to macosArm64Test (same for other targets) and will be automatically registered to run on check/allTests (I haven't looked on TC setup, so not sure what is needed there).

sandwwraith pushed a commit that referenced this issue Jun 20, 2024
Proposed here: #2608

To run native tests in release mode, separate tasks are created by KGP for additional test runs, f.e macosArm64ReleaseTest or linuxX64ReleaseTest. By default, they will be run also on build, check and allTests tasks, so they should automatically run on TC.

Note: with Kotlin 1.9.22 the original issue (#2608) doesn't reproduce nor via running protobuf tests (which contains tests for inner classes, so I haven't added additional test from original issue), nor via running tests in integration-test module (by adding reproducer test there, not included)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants