Skip to content

Add test to check that sealed unions are serialized the same way as legacy unions#2752

Merged
bulldozer-bot[bot] merged 3 commits intodevelopfrom
mpritham/sealed-unions-toString
Dec 18, 2025
Merged

Add test to check that sealed unions are serialized the same way as legacy unions#2752
bulldozer-bot[bot] merged 3 commits intodevelopfrom
mpritham/sealed-unions-toString

Conversation

@mpritham
Copy link
Contributor

Before this PR

Add a small test to validate that the same Conjure union object is serialized the same way whether using the sealed classes implementation or the legacy implementation.

After this PR

==COMMIT_MSG==
Add test to check that sealed unions are serialized the same way as legacy unions
==COMMIT_MSG==

Possible downsides?

@changelog-app
Copy link

changelog-app bot commented Dec 15, 2025

Generate changelog in changelog/@unreleased

Type (Select exactly one)

  • Feature (Adding new functionality)
  • Improvement (Improving existing functionality)
  • Fix (Fixing an issue with existing functionality)
  • Break (Creating a new major version by breaking public APIs)
  • Deprecation (Removing functionality in a non-breaking way)
  • Migration (Automatically moving data/functionality to a new system)

Description

Add test to check that sealed unions are serialized the same way as legacy unions

Check the box to generate changelog(s)

  • Generate changelog entry

}

@Test
void sealedUnionsAreSerializedTheSameAsLegacyUnions() throws IOException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the test. The remainder of the code is modifying SimpleUnion in ete-service.yml to match the one in example-unions.yml

@changelog-app
Copy link

changelog-app bot commented Dec 15, 2025

Successfully generated changelog entry!

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.


📋Changelog Preview

💡 Improvements

  • Add test to check that sealed unions are serialized the same way as legacy unions (#2752)

@kunalrkak
Copy link
Contributor

@mpritham We have a bunch of these serde tests here, with the expected output hardcoded (since it should not change) - but if you want to add a object <> object test, that file feels like a better place for it!

@mpritham mpritham force-pushed the mpritham/sealed-unions-toString branch from 8f69baf to 79f7c1d Compare December 15, 2025 19:56
@mpritham
Copy link
Contributor Author

Sounds good. Moved.

Comment on lines 942 to 943
assertThat(mapper.writeValueAsString(SimpleUnion.foo("foo")))
.isEqualTo(mapper.writeValueAsString(undertow.com.palantir.product.SimpleUnion.foo("foo")));
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not immediately clear which one is the sealed interface and which one isn't. Should we add some verification that one is sealed and one isn't? (to avoid test refactors breaking our assumptions by making both sealed or both unsealed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm importing the SimpleUnion from the sealedunions package now

@mpritham mpritham force-pushed the mpritham/sealed-unions-toString branch from be21559 to f9ef6ae Compare December 17, 2025 22:50
@bulldozer-bot bulldozer-bot bot merged commit 8c622bd into develop Dec 18, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants