Fixed a flaky test for issue 19847

[liuguoqingfz]

Description

Fixed a flaky test which contains floating-point/rounding-edge flake: it's multiplying an arbitrary randomLong(), potentially very large by an arbitrary randomDouble()*100 high-precision fractional, and the scaled-float encoding path used by StarTree can round at a slightly different intermediate precision than the test’s expectation, producing an off-by-one long.

Related Issues

Resolves #19847

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Summary by CodeRabbit

• Tests
  • Enhanced test stability by introducing bounded random value generation and explicit precision rounding controls to reduce floating-point variability in scaled float mapping tests.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The test testScaledFloatWithStarTree was modified to use bounded random ranges and explicit rounding precision for scaling factors and input values, reducing floating-point edge cases and minimizing test flakiness.

Changes

Cohort / File(s)	Summary
Test Precision & Bounds Adjustments modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldMapperTests.java	Modified random value generation in testScaledFloatWithStarTree: scaled factors now bounded to [0.001, 100.0] with explicit 3-decimal rounding; long input values constrained to [-1T, 1T] range to reduce floating-point rounding inconsistencies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Verify bounded ranges (0.001–100.0 for scaling factors, ±1T for longs) align with test intent and real-world constraints
• Confirm 3-decimal rounding precision is sufficient to eliminate flakiness without masking genuine bugs

Suggested labels

flaky-test, bug

Suggested reviewers

• sachinpkale
• kotwanikunal
• gbbafna
• cwperks
• saratvemulapalli

Poem

🐰 Hop, hop, randomness tamed!
Bounded ranges and rounding framed,
No more wild floating-point sprees,
Tests stable as trees—we're pleased!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing a flaky test for issue 19847. It is concise, clear, and directly related to the changeset.
Description check	✅ Passed	The description includes all required sections: a clear explanation of what the fix achieves (addressing the floating-point rounding edge case), the related issue reference (#19847), and the required checklist with Apache 2.0 license confirmation.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e798353 and df9dd48.

📒 Files selected for processing (1)

• modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldMapperTests.java (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

• GitHub Check: gradle-check
• GitHub Check: assemble (25, windows-latest)
• GitHub Check: assemble (21, ubuntu-24.04-arm)
• GitHub Check: assemble (25, ubuntu-latest)
• GitHub Check: assemble (21, windows-latest)
• GitHub Check: assemble (25, ubuntu-24.04-arm)
• GitHub Check: assemble (21, ubuntu-latest)
• GitHub Check: precommit (21, macos-15)
• GitHub Check: precommit (21, windows-2025, true)
• GitHub Check: precommit (25, ubuntu-24.04-arm)
• GitHub Check: precommit (25, macos-15)
• GitHub Check: precommit (21, ubuntu-24.04-arm)
• GitHub Check: precommit (25, windows-latest)
• GitHub Check: precommit (21, macos-15-intel)
• GitHub Check: precommit (25, ubuntu-latest)
• GitHub Check: precommit (21, ubuntu-latest)
• GitHub Check: precommit (21, windows-latest)
• GitHub Check: precommit (25, macos-15-intel)
• GitHub Check: Analyze (java)
• GitHub Check: detect-breaking-change

🔇 Additional comments (3)

modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldMapperTests.java (3)

110-114: LGTM! Bounded scaling factors reduce floating-point edge cases.

Replacing randomDouble() * 100 with randomDoubleBetween(0.001, 100.0, true) appropriately constrains the scaling factors to a reasonable range, preventing the extreme precision values that contributed to test flakiness.

---

116-119: LGTM! Explicit rounding eliminates precision edge cases.

The explicit rounding to 3 decimal places (Math.round(value * 1_000d) / 1_000d) is mathematically correct and effectively eliminates high-precision fractional components that could round differently between the test expectation and the StarTree encoding path. This directly addresses the root cause of the flaky test.

---

125-128: LGTM! Bounded input values ensure deterministic floating-point behavior.

Constraining random longs to ±1_000_000_000_000L (±1 trillion) is a pragmatic fix that keeps the products of value * scalingFactor well within the range where double-precision floating-point arithmetic is deterministic and accurate. This prevents the precision loss that occurred when unbounded randomLong() generated values near Long.MIN_VALUE or Long.MAX_VALUE, which could cause off-by-one errors when cast to long at lines 152 and 155.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

❌ Gradle check result for df9dd48: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[andrross]

There's a test failure in the class that was changed here:

REPRODUCE WITH: ./gradlew ':modules:mapper-extras:test' --tests 'org.opensearch.index.mapper.ScaledFloatFieldMapperTests.testScaledFloatWithStarTree' -Dtests.seed=3CFCC0B013FFC340 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=xnr -Dtests.timezone=America/Argentina/Catamarca -Druntime.java=25

ScaledFloatFieldMapperTests > testScaledFloatWithStarTree FAILED
    java.lang.AssertionError: expected:<1181493098135> but was:<1181493098136>
        at __randomizedtesting.SeedInfo.seed([3CFCC0B013FFC340:300F51795172BC6F]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.junit.Assert.assertEquals(Assert.java:633)
        at org.opensearch.index.mapper.ScaledFloatFieldMapperTests.validateScaledFloatFields(ScaledFloatFieldMapperTests.java:152)
        at org.opensearch.index.mapper.ScaledFloatFieldMapperTests.testScaledFloatWithStarTree(ScaledFloatFieldMapperTests.java:132)