Skip to content

Comments

[fix][test] Fixed Nondeterministic Ordering in SchemaInfoTest#9

Open
LucasEby wants to merge 1 commit intomasterfrom
fix-SchemaInfoTest
Open

[fix][test] Fixed Nondeterministic Ordering in SchemaInfoTest#9
LucasEby wants to merge 1 commit intomasterfrom
fix-SchemaInfoTest

Conversation

@LucasEby
Copy link
Owner

@LucasEby LucasEby commented Nov 10, 2025

Fixes apache#24967

Motivation

This fix is the same as this fix which was previously accepted and merged: apache#24821

Each of the below tests incorrectly assumes that the json key-value pairs being asserted would have a deterministic order. The order of key-value pairs is not guaranteed in JSON, however. As a result, the ordering can change due to different environments producing the contents in different orders despite the logical contents being the same. Harmless re-ordering could flip the tests from pass to fail despite the data being semantically the same:

  • org.apache.pulsar.client.impl.schema.SchemaInfoTest$SchemaInfoBuilderTest.testNullPropertyValue
  • org.apache.pulsar.client.impl.schema.SchemaInfoTest.testSchemaInfoToString

Modifications

We no longer compare raw strings "as-is" with Assert.assertEquals. Instead, we compare the json structures with JsonAssert.assertEquals(expectedJson, actualJson, JSONCompareMode) which parses both inputs into JSON trees. It treats these JSON trees as unordered collections of key-value pairs when determining if they are equal so differences in property order no longer matter. This ensures the tests pass consistently, even when the serializer outputs fields in a different order.

In essence, these changes keep the spirit of the original tests while eliminating failures caused solely by allowed (but previously unexpected) reordering.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as

  • org.apache.pulsar.client.impl.schema.SchemaInfoTest$SchemaInfoBuilderTest.testNullPropertyValue
  • org.apache.pulsar.client.impl.schema.SchemaInfoTest.testSchemaInfoToString

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

[Bug] Nondeterministic Ordering in SchemaInfoTest

1 participant