Skip to content

Comments

[fix][test] Fixed Nondeterministic Ordering in Generated Docs Topics#24973

Merged
lhotari merged 1 commit intoapache:masterfrom
LucasEby:fix-CmdGenerateDocsTest
Nov 11, 2025
Merged

[fix][test] Fixed Nondeterministic Ordering in Generated Docs Topics#24973
lhotari merged 1 commit intoapache:masterfrom
LucasEby:fix-CmdGenerateDocsTest

Conversation

@LucasEby
Copy link
Contributor

Fixes #24968

Motivation

The below test compares the generated documentation of CmdGenerateDocsTest#testGenerateDocs. The test assumes a deterministic order of the generated documentation but the order of the listed help and name options was not guaranteed. Due to two sources of nondeterminism hidden in the test and production code, the options could be listed in a different order. Picocli's CommandLine class uses Class#getDeclaredFields internally (via reflection), which has a non-deterministic specification to retrieve the Option annotations and orders these options accordingly. Even though picocli supports an order attribute on Option annotations, CommandLine never sorts by this order - it just maintains whatever order the reflection API gave it. CmdGenerateDocs#generateDocument then iterates over the order that the args method returns, with the assumption that the order is deterministic. Unfortunately, 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.docs.tools.CmdGenerateDocsTest.testGenerateDocs

Modifications

The order of each option in the Argument class is now explicitly stated so getDeclaredFields is no longer utilized to determine the order of the options. Additionally, generateDocument now sorts all argSpecs by their specified order with a new helper method called getSortedArgs. This combination of changes enforces the expected nondeterminism in the ordering of the listed options.

If order is not specified because it does not matter, the order for each option will remain in it's original order as before. This is because according to Collection's documentation, it's sort method is guaranteed to be stable: equal elements will not be reordered as a result of the sort (and the orders are equal because Piccocli's CommandLine class sets order to -1 by default).

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: LucasEby#8

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 11, 2025
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.23%. Comparing base (c4f125c) to head (8b2f7cd).
⚠️ Report is 15 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #24973       +/-   ##
=============================================
+ Coverage     38.77%   74.23%   +35.46%     
- Complexity    13380    33997    +20617     
=============================================
  Files          1856     1920       +64     
  Lines        145342   150146     +4804     
  Branches      16886    17414      +528     
=============================================
+ Hits          56353   111459    +55106     
+ Misses        81459    29783    -51676     
- Partials       7530     8904     +1374     
Flag Coverage Δ
inttests 26.45% <0.00%> (+0.03%) ⬆️
systests 22.87% <0.00%> (+0.02%) ⬆️
unittests 73.76% <100.00%> (+38.76%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../org/apache/pulsar/docs/tools/CmdGenerateDocs.java 80.45% <100.00%> (+80.45%) ⬆️

... and 1416 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lhotari lhotari added this to the 4.2.0 milestone Nov 11, 2025
@lhotari lhotari merged commit 560ed26 into apache:master Nov 11, 2025
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Nondeterministic Ordering in Generated Docs Topics

3 participants