Add test for YAML boolean parsing behavior

[andrross]

This establishes the existing parsing behavior so that we can be sure updates in the 3.x line (i.e. Jackson updates) do not change the user-facing behavior.

Check List

• Functionality includes testing.

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.

[andrross]

@reta What do you think? I'll add this same test to my other PR, but this will establish a baseline for the existing behavior.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit ef9d830)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Assumption Risk The test asserts that parser.text() preserves the original value (e.g., "Yes", "ON", "False") for boolean tokens. This is a strong assumption about the underlying Jackson/SnakeYAML behavior. If the serialization layer normalizes boolean text representations (e.g., always returning "true"/"false"), this assertion will fail. This should be validated against the actual implementation to confirm the behavior is intentional and stable. assertEquals("Expected text() to preserve original value", value, parser.text()); } } for (String value : falsyValues) { String yaml = "---\nfield: " + value + "\n"; try (XContentParser parser = createParser(YamlXContent.yamlXContent, yaml)) { assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); assertEquals("field", parser.currentName()); XContentParser.Token token = parser.nextToken(); assertEquals("Expected VALUE_BOOLEAN token for '" + value + "'", XContentParser.Token.VALUE_BOOLEAN, token); assertTrue("Expected '" + value + "' to be a boolean value", parser.isBooleanValue()); assertFalse("Expected '" + value + "' to parse as false", parser.booleanValue()); assertEquals("Expected text() to preserve original value", value, parser.text()); SnakeYAML Version Coupling The test explicitly references "SnakeYAML's Resolver.BOOL regex" in comments and tests values like "yes", "no", "on", "off" as booleans. This behavior is specific to YAML 1.1 (used by SnakeYAML 1.x). SnakeYAML 2.x changed to YAML 1.2, which only recognizes "true"/"false" as booleans. If the dependency is updated, many of these test cases will break. The PR description mentions this is intended to catch such changes, but the test may need to be updated rather than just failing. String[] truthyValues = { "true", "True", "TRUE", "yes", "Yes", "YES", "on", "On", "ON" }; // YAML boolean false values per SnakeYAML's Resolver.BOOL regex String[] falsyValues = { "false", "False", "FALSE", "no", "No", "NO", "off", "Off", "OFF" };

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to ef9d830

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix incorrect canonical boolean text assertion The assertion that parser.text() preserves the original value (e.g., "yes", "on", "True") after parsing as a boolean may not hold true. Once a YAML value is parsed as a boolean token, text() typically returns the canonical boolean string ("true" or "false"), not the original YAML representation. This assertion is likely incorrect and will cause test failures for non-canonical boolean values like "yes", "YES", "on", "ON", etc. libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java [789] -assertEquals("Expected text() to preserve original value", value, parser.text()); +// For truthy values, text() returns the canonical "true" +assertEquals("Expected text() to return canonical boolean string", "true", parser.text()); Suggestion importance[1-10]: 6 __ Why: This is a valid concern - when YAML parses "yes", "on", "True" etc. as boolean tokens, parser.text() likely returns the canonical "true" string rather than the original representation. If the test is asserting the original value is preserved, it may fail for non-canonical boolean strings like "yes" or "ON".	Low
	Fix canonical boolean text assertion for false values Similarly, for the falsy values loop, parser.text() after parsing a boolean token will return "false" (the canonical form), not the original YAML string like "no", "NO", "off", etc. The assertion should compare against "false" instead of value. libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java [803] -assertEquals("Expected text() to preserve original value", value, parser.text()); +// For falsy values, text() returns the canonical "false" +assertEquals("Expected text() to return canonical boolean string", "false", parser.text()); Suggestion importance[1-10]: 6 __ Why: Same issue as suggestion 1 but for the falsy values loop - parser.text() after parsing a boolean token likely returns "false" rather than the original YAML string like "no", "NO", or "off", making the assertion against value potentially incorrect.	Low

---

Previous suggestions

Suggestions up to commit 71c9b35

Category	Suggestion	Impact
General	Verify token type before accessing text value When isBooleanValue() returns false, calling parser.text() may throw an exception or return unexpected results depending on the token type. The parser.nextToken() call before this block sets the current token, and if the YAML parser treats the value as a string token, parser.text() should work, but you should verify the token type first to avoid potential issues with non-string tokens. libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java [812-813] assertFalse("Expected '" + value + "' to NOT be a boolean value", parser.isBooleanValue()); +assertEquals(XContentParser.Token.VALUE_STRING, parser.currentToken()); assertEquals(value, parser.text()); Suggestion importance[1-10]: 5 __ Why: Adding a token type assertion before calling parser.text() improves test robustness by explicitly verifying the parser treats non-boolean values as VALUE_STRING tokens, making the test more precise and self-documenting.	Low
	Validate token type before reading boolean value parser.booleanValue() may advance or consume the parser state or throw an exception if called after isBooleanValue() depending on the implementation. Additionally, calling booleanValue() twice (once implicitly via isBooleanValue() and once explicitly) could be problematic. Consider checking the token type to ensure the current token is VALUE_BOOLEAN before calling booleanValue(). libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java [787] +assertEquals(XContentParser.Token.VALUE_BOOLEAN, parser.currentToken()); assertTrue("Expected '" + value + "' to parse as true", parser.booleanValue()); Suggestion importance[1-10]: 4 __ Why: Adding a VALUE_BOOLEAN token assertion before booleanValue() adds clarity, but the concern about isBooleanValue() consuming parser state is likely unfounded as it's typically a non-consuming check, making this a minor improvement rather than a critical fix.	Low

Suggestions up to commit 3ad8611

Category	Suggestion	Impact
Possible issue	Fix incorrect non-boolean test values The values "y", "Y", "n", and "N" are valid YAML 1.1 boolean values per SnakeYAML's Resolver.BOOL regex pattern, which includes single-character y/Y/n/N. Including them in the nonBooleans array will cause the test to fail incorrectly. They should either be moved to the truthy/falsy arrays or explicitly documented if the intent is to verify that the implementation deviates from YAML 1.1 spec. libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java [804] -String[] nonBooleans = { "truthy", "nope", "yep", "often", "None", "NOTICE", "trUe", "fAlse", "TruE", "y", "Y", "n", "N" }; +// y, Y, n, N are YAML 1.1 booleans - move to truthy/falsy or document intentional deviation +String[] nonBooleans = { "truthy", "nope", "yep", "often", "None", "NOTICE", "trUe", "fAlse", "TruE" }; Suggestion importance[1-10]: 7 __ Why: The values "y", "Y", "n", and "N" are indeed recognized as boolean values in YAML 1.1 by SnakeYAML's Resolver.BOOL regex, so including them in nonBooleans could cause test failures. This is a potentially significant correctness issue in the test.	Medium
General	Verify token type before asserting text value Calling parser.text() after parser.isBooleanValue() may not reliably return the original string if the parser has already consumed or transformed the token. The token type should be verified first (e.g., asserting it is VALUE_STRING) before calling parser.text() to ensure the assertion is meaningful and consistent. libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java [812-813] assertFalse("Expected '" + value + "' to NOT be a boolean value", parser.isBooleanValue()); +assertEquals(XContentParser.Token.VALUE_STRING, parser.currentToken()); assertEquals(value, parser.text()); Suggestion importance[1-10]: 4 __ Why: Adding a currentToken() assertion before calling parser.text() improves test robustness by explicitly verifying the token type, but this is a minor enhancement since isBooleanValue() returning false already implies the token is not a boolean.	Low

[github-actions]

❌ Gradle check result for 3ad8611: 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?

[github-actions]

✅ Gradle check result for 3ad8611: SUCCESS

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.01%. Comparing base (aff3489) to head (ef9d830).
⚠️ Report is 2 commits behind head on 2.19.

Additional details and impacted files

@@             Coverage Diff              @@
##               2.19   #21296      +/-   ##
============================================
+ Coverage     71.92%   72.01%   +0.08%     
+ Complexity    66009    64485    -1524     
============================================
  Files          5342     5122     -220     
  Lines        307392   300234    -7158     
  Branches      44862    44090     -772     
============================================
- Hits         221105   216225    -4880     
+ Misses        67823    65902    -1921     
+ Partials      18464    18107     -357     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Persistent review updated to latest commit 71c9b35

[github-actions]

Persistent review updated to latest commit ef9d830

[github-actions]

❌ Gradle check result for ef9d830: null

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?

[github-actions]

❌ Gradle check result for ef9d830: 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?

[github-actions]

❌ Gradle check result for ef9d830: null

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?

[github-actions]

✅ Gradle check result for ef9d830: SUCCESS