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

Generalize encoding/decoding tests #168

Merged
merged 6 commits into from
Jan 28, 2024
Merged

Conversation

thake
Copy link
Member

@thake thake commented Sep 15, 2023

The current unit tests for encoding have been refactored as part of #160. Duplicate tests have been removed, and some bugs have been fixed on the way.

@thake thake requested a review from Chuckame September 15, 2023 06:41
Copy link
Contributor

@Chuckame Chuckame left a comment

Choose a reason for hiding this comment

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

Hello back, happy new year. Happy to come back on the project, I didn't have that much time until now! Feel free to tell me if you want me pushing a commit on this branch if you don't have time or di not understand :)

@thake thake requested a review from Chuckame January 21, 2024 20:13
Comment on lines +80 to +88
inline fun <reified T> EnDecoder.testEncodeDecode(
value: T,
shouldMatch: RecordBuilderForTest,
serializer: KSerializer<T> = avro.serializersModule.serializer<T>(),
schema: Schema = avro.schema(serializer)
) {
val encoded = testEncodeIsEqual(value, shouldMatch, serializer, schema)
testDecodeIsEqual(encoded, value,serializer, schema)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is what I'm talking about: taking the expected avro representation, encoding it with avro lib, and then use it to test our decoding part

Suggested change
inline fun <reified T> EnDecoder.testEncodeDecode(
value: T,
shouldMatch: RecordBuilderForTest,
serializer: KSerializer<T> = avro.serializersModule.serializer<T>(),
schema: Schema = avro.schema(serializer)
) {
val encoded = testEncodeIsEqual(value, shouldMatch, serializer, schema)
testDecodeIsEqual(encoded, value,serializer, schema)
}
inline fun <reified T> EnDecoder.testEncodeDecode(
value: T,
shouldMatch: RecordBuilderForTest,
serializer: KSerializer<T> = avro.serializersModule.serializer<T>(),
schema: Schema = avro.schema(serializer)
) {
testEncodeIsEqual(value, shouldMatch, serializer, schema)
testDecodeIsEqual(avroLibBinaryEncoding(shouldMatch.toAvro(schema)), value, serializer, schema)
}

@Chuckame
Copy link
Contributor

Let's merge it as it is, as I'm currently adding ktlint, spotless and reformat everything, and also moving forward on the v2, so having simpler tests will be awesome.

I'm not a fan of the open door of multiple EnDecode implementation as it complexifies for a need that we don't have now, and mostly avro4k should not have multiple way of doing avro or users won't really know what to chose.

If we want to test decoding and encoding in multiple container types (single object, confluent schema registry, ...), let's tackle some "wrapping" api to be able of decorate a binary avro message with whatever we need.

I'll do a little other PR to fix this point after all the code reformat

@Chuckame Chuckame merged commit 1723ebc into main Jan 28, 2024
1 check passed
Chuckame pushed a commit to Chuckame/avro4k that referenced this pull request Jan 28, 2024
* removed duplicate decoding tests and generalized tests to be used for other decoders
* fixed decoding of short values
* fixed decoding of fixed values
Chuckame added a commit that referenced this pull request Jan 28, 2024
Generalize encoding/decoding tests (#168)
@Chuckame Chuckame deleted the generalize-encoding-tests branch June 24, 2024 12:51
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