Replace multi-line strings with java text blocks#20269
Replace multi-line strings with java text blocks#20269andrross merged 3 commits intoopensearch-project:mainfrom
Conversation
Signed-off-by: Craig Perkins <cwperx@amazon.com>
cwperks
requested review from
a team,
Bukhtawar,
CEHENKLE,
Rishikesh1159,
anasalkouz,
andrross,
ashking94,
dbwiddis,
gbbafna,
jed326,
kotwanikunal,
mch2,
msfroh,
owaiskazi19,
reta,
sachinpkale,
saratvemulapalli,
shwetathareja and
sohami
as code owners
WalkthroughThis PR replaces many manually concatenated or escaped JSON/YAML string literals in tests and one build script with Java/Groovy multiline text blocks (triple-quoted strings). No public APIs or control flow were changed; edits are formatting-only for test/build inputs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)modules/ingest-common/src/internalClusterTest/java/org/opensearch/ingest/common/IngestRestartIT.java (2)
⏰ 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)
🔇 Additional comments (21)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugins/repository-gcs/build.gradle (1)
340-348: Potential issue: Leading whitespace in the generated JSON file.The Groovy multi-line string includes leading whitespace and a leading newline that will be written to the file. The opening
"""followed by a newline on line 340, combined with the indentation on lines 341-347, will produce JSON with leading whitespace and indented content.While JSON parsers are typically lenient about whitespace, this changes the file content from the previous implementation. Consider using Groovy's
stripIndent()orstripMargin()to remove the leading whitespace:serviceAccountFile.parentFile.mkdirs() - serviceAccountFile.setText(""" + serviceAccountFile.setText("""\ { "type": "service_account", "project_id": "integration_test", "private_key_id": "${UUID.randomUUID().toString()}", "private_key": "-----BEGIN PRIVATE KEY-----\n${encodedKey}\n-----END PRIVATE KEY-----\n", "client_email": "integration_test@appspot.gserviceaccount.com", "client_id": "123456789101112130594" - }""", 'UTF-8') + }""".stripIndent(), 'UTF-8')Alternatively, align the content without extra indentation if
stripIndent()isn't available in your Groovy version.server/src/test/java/org/opensearch/index/query/BoolQueryBuilderTests.java (1)
165-192: Consider completing the text block migration.The
getAlternateVersions()method uses a mixed approach: starting with a text block, concatenating strings with+=, then appending another text block. While functional, this doesn't fully embrace text blocks.Consider using
StringBuilderor a fully dynamic approach:-String contentString = """ - { - "bool" : { - """; +StringBuilder contentString = new StringBuilder(""" + { + "bool" : { + """); if (tempQueryBuilder.must().size() > 0) { QueryBuilder must = tempQueryBuilder.must().get(0); - contentString += "\"must\": " + must.toString() + ","; + contentString.append("\"must\": ").append(must.toString()).append(","); expectedQuery.must(must); } // ... similar changes for mustNot, should, filter -contentString = contentString.substring(0, contentString.length() - 1); -contentString += """ - } - }"""; +contentString.setLength(contentString.length() - 1); +contentString.append(""" + } + }"""); -alternateVersions.put(contentString, expectedQuery); +alternateVersions.put(contentString.toString(), expectedQuery);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
client/rest-high-level/src/test/java/org/opensearch/client/documentation/IndicesClientDocumentationIT.java(8 hunks)libs/x-content/src/test/java/org/opensearch/common/xcontent/ConstructingObjectParserTests.java(2 hunks)modules/ingest-common/src/internalClusterTest/java/org/opensearch/ingest/common/IngestRestartIT.java(4 hunks)modules/mapper-extras/src/test/java/org/opensearch/index/query/RankFeatureQueryBuilderTests.java(2 hunks)plugins/repository-gcs/build.gradle(1 hunks)plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetWorkloadGroupResponseTests.java(3 hunks)qa/full-cluster-restart/src/test/java/org/opensearch/upgrades/FullClusterRestartIT.java(4 hunks)qa/smoke-test-http/src/test/java/org/opensearch/http/HttpCompressionIT.java(1 hunks)qa/smoke-test-http/src/test/java/org/opensearch/http/SystemIndexRestIT.java(3 hunks)server/src/internalClusterTest/java/org/opensearch/search/geo/GeoShapeIntegrationIT.java(4 hunks)server/src/test/java/org/opensearch/common/xcontent/builder/XContentBuilderTests.java(1 hunks)server/src/test/java/org/opensearch/index/query/BoolQueryBuilderTests.java(7 hunks)server/src/test/java/org/opensearch/index/query/DisMaxQueryBuilderTests.java(3 hunks)server/src/test/java/org/opensearch/index/query/ExistsQueryBuilderTests.java(1 hunks)server/src/test/java/org/opensearch/index/query/IdsQueryBuilderTests.java(1 hunks)server/src/test/java/org/opensearch/index/query/MatchAllQueryBuilderTests.java(1 hunks)server/src/test/java/org/opensearch/index/query/MatchNoneQueryBuilderTests.java(1 hunks)server/src/test/java/org/opensearch/index/query/MatchPhrasePrefixQueryBuilderTests.java(2 hunks)server/src/test/java/org/opensearch/index/query/MatchPhraseQueryBuilderTests.java(3 hunks)server/src/test/java/org/opensearch/index/query/MatchQueryBuilderTests.java(3 hunks)server/src/test/java/org/opensearch/index/query/PrefixQueryBuilderTests.java(2 hunks)server/src/test/java/org/opensearch/index/query/RegexpQueryBuilderTests.java(3 hunks)server/src/test/java/org/opensearch/index/query/SimpleQueryStringBuilderTests.java(2 hunks)server/src/test/java/org/opensearch/index/query/TermQueryBuilderTests.java(2 hunks)server/src/test/java/org/opensearch/index/query/WrapperQueryBuilderTests.java(1 hunks)server/src/test/java/org/opensearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java(1 hunks)server/src/test/java/org/opensearch/rest/action/document/RestUpdateActionTests.java(1 hunks)server/src/test/java/org/opensearch/search/NestedIdentityTests.java(1 hunks)server/src/test/java/org/opensearch/search/builder/SearchSourceBuilderTests.java(15 hunks)server/src/test/java/org/opensearch/search/fetch/subphase/highlight/HighlightBuilderTests.java(5 hunks)server/src/test/java/org/opensearch/search/fetch/subphase/highlight/HighlightFieldTests.java(1 hunks)server/src/test/java/org/opensearch/search/rescore/QueryRescorerBuilderTests.java(2 hunks)server/src/test/java/org/opensearch/tasks/ListTasksResponseTests.java(2 hunks)test/framework/src/test/java/org/opensearch/test/rest/yaml/section/ClientYamlTestSuiteTests.java(4 hunks)test/framework/src/test/java/org/opensearch/test/rest/yaml/section/DoSectionTests.java(18 hunks)test/framework/src/test/java/org/opensearch/test/rest/yaml/section/SkipSectionTests.java(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-12T13:31:51.234Z
Learnt from: andriyredko
Repo: opensearch-project/OpenSearch PR: 20017
File: plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java:642-646
Timestamp: 2025-12-12T13:31:51.234Z
Learning: In the OpenSearch ReactorNetty4 secure HTTP transport tests (plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java), the createBuilderWithPort() helper intentionally uses randomBoolean() for the HTTP/3 enabled setting to ensure all tests validate correct behavior with both HTTP/3 enabled and disabled configurations.
Applied to files:
test/framework/src/test/java/org/opensearch/test/rest/yaml/section/ClientYamlTestSuiteTests.java
🧬 Code graph analysis (1)
client/rest-high-level/src/test/java/org/opensearch/client/documentation/IndicesClientDocumentationIT.java (1)
libs/core/src/main/java/org/opensearch/core/xcontent/MediaTypeRegistry.java (1)
MediaTypeRegistry(57-418)
⏰ 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). (21)
- GitHub Check: gradle-check
- GitHub Check: Mend Security Check
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: detect-breaking-change
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: Analyze (java)
🔇 Additional comments (54)
server/src/test/java/org/opensearch/index/query/MatchQueryBuilderTests.java (6)
164-169: LGTM! Clean modernization to text blocks.The conversion to text blocks with
String.formatted()improves readability while maintaining functional equivalence. The indentation and placeholder usage are correct.
291-306: LGTM! Improved readability.The text block format makes the JSON structure clearer without changing the test behavior. The indentation is correct and the JSON content is preserved.
368-378: LGTM! Error test data correctly migrated.The text block conversion preserves the multi-field JSON structure needed to trigger the parsing error test case.
382-388: LGTM! Shorthand format preserved.The text block correctly represents the shorthand JSON notation for the multi-field error test case.
394-401: LGTM! Array test case correctly converted.The text block preserves the JSON array structure needed to test the terms array parsing error.
404-409: LGTM! Shorthand array format preserved.The text block correctly represents the shorthand array notation for the terms array error test case.
server/src/test/java/org/opensearch/index/query/IdsQueryBuilderTests.java (1)
82-104: LGTM! Text blocks improve test readability.The migration from concatenated strings to Java text blocks makes the JSON test data significantly more readable while preserving the exact semantic content.
qa/smoke-test-http/src/test/java/org/opensearch/http/HttpCompressionIT.java (1)
53-60: LGTM! Cleaner JSON representation.The text block format makes the sample document structure much clearer and easier to maintain.
plugins/workload-management/src/test/java/org/opensearch/plugin/wlm/action/GetWorkloadGroupResponseTests.java (3)
84-105: LGTM! Much clearer expected JSON structure.The text block makes the expected output structure immediately visible, improving test maintainability.
110-141: LGTM! Excellent readability improvement.The multiline expected JSON is now easy to read and verify against actual output.
146-155: LGTM! Clean empty array representation.Even simple expected outputs benefit from the text block format consistency.
server/src/test/java/org/opensearch/index/query/MatchAllQueryBuilderTests.java (1)
60-70: LGTM! Cleaner test input formatting.The text block makes the JSON query structure more readable.
test/framework/src/test/java/org/opensearch/test/rest/yaml/section/SkipSectionTests.java (5)
88-100: LGTM! Text block with formatted string works well.Nice use of text blocks combined with
String.formatted()for dynamic version insertion.
102-113: LGTM! Cleaner YAML test input.The text block format makes the YAML structure immediately clear.
139-150: LGTM! Improved readability for multi-field YAML.The text block makes it easy to see all the skip section fields at a glance.
152-159: LGTM! Simple and clear.Even minimal YAML benefits from consistent text block usage.
161-168: LGTM! Consistent formatting.Text block maintains consistency across all test cases.
server/src/test/java/org/opensearch/index/query/TermQueryBuilderTests.java (4)
149-158: LGTM! Clear error test case.The text block makes the invalid array structure obvious for this error test.
160-174: LGTM! Improved JSON readability.The nested structure of the term query is now much easier to read and verify.
186-210: LGTM! Error cases are clearer.Both error test JSONs are now easy to read and understand why they should fail parsing.
212-223: LGTM! BigInteger test case is clear.The text block makes it easy to see the large integer value being tested.
server/src/test/java/org/opensearch/index/query/ExistsQueryBuilderTests.java (1)
174-188: LGTM! Clean exists query representation.The text block format makes the query structure clear and easy to maintain.
server/src/test/java/org/opensearch/search/rescore/QueryRescorerBuilderTests.java (1)
309-379: LGTM! Significantly improved error test readability.Converting all the rescoreElement strings to text blocks makes it much easier to see what's wrong with each malformed configuration and what the correct structure should be. This is a great improvement for test maintainability.
server/src/test/java/org/opensearch/search/fetch/subphase/highlight/HighlightFieldTests.java (1)
111-117: LGTM! Text blocks improve readability.The conversion to Java text blocks makes the expected JSON output much cleaner and easier to read, while preserving the exact formatting needed for the assertions.
Also applies to: 125-128
server/src/test/java/org/opensearch/search/fetch/subphase/highlight/HighlightBuilderTests.java (1)
176-180: LGTM! Excellent readability improvement for test inputs.The migration to text blocks significantly improves the readability of JSON test inputs across all error-handling test cases. The multiline format makes it much easier to understand the test scenarios at a glance.
Also applies to: 185-193, 211-215, 220-228, 240-245, 250-258, 266-268, 280-285, 297-304
server/src/test/java/org/opensearch/index/query/WrapperQueryBuilderTests.java (1)
124-129: LGTM! Clean text block conversion.The text block format makes the JSON structure more readable while maintaining semantic equivalence for the test assertion.
server/src/test/java/org/opensearch/index/query/MatchNoneQueryBuilderTests.java (1)
56-61: LGTM! Text block enhances readability.The conversion to a text block makes the JSON structure clearer and more maintainable.
server/src/test/java/org/opensearch/search/NestedIdentityTests.java (1)
87-93: LGTM! Text blocks significantly improve readability.The conversion to text blocks makes the expected JSON outputs much easier to read, especially for the nested structure. The formatting is preserved correctly for the assertions.
Also applies to: 101-111
server/src/test/java/org/opensearch/rest/action/document/RestUpdateActionTests.java (1)
71-77: LGTM! Clean conversion to text block.The text block format makes the JSON test payload more readable and maintainable.
server/src/test/java/org/opensearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java (1)
783-788: LGTM! Text blocks improve test clarity.The migration to text blocks makes the malformed JSON test cases much more readable and easier to understand, which is especially valuable for error-handling tests where clarity is important for maintainability.
Also applies to: 794-799, 805-812, 819-831
libs/x-content/src/test/java/org/opensearch/common/xcontent/ConstructingObjectParserTests.java (1)
192-197: LGTM! Text blocks correctly preserve line numbers for exception tests.The conversion to text blocks maintains the newline structure necessary for accurate line number assertions in the exception tests. The formatting is preserved correctly and the text blocks improve readability.
Also applies to: 216-221
modules/mapper-extras/src/test/java/org/opensearch/index/query/RankFeatureQueryBuilderTests.java (1)
125-164: LGTM! Text blocks improve test readability.The migration from concatenated strings to Java text blocks enhances code readability while preserving all test semantics. The
.formatted()method correctly injects dynamic field values.server/src/internalClusterTest/java/org/opensearch/search/geo/GeoShapeIntegrationIT.java (1)
205-295: LGTM! Consistent text block migration for test inputs.The conversion of mapping and source JSON literals to text blocks is correct and improves maintainability. All semantic content is preserved.
server/src/test/java/org/opensearch/tasks/ListTasksResponseTests.java (1)
65-125: LGTM! Expected JSON outputs now more readable.The text block format makes the expected JSON structures much easier to read and maintain. The conversion is accurate and preserves all whitespace and structure.
qa/full-cluster-restart/src/test/java/org/opensearch/upgrades/FullClusterRestartIT.java (1)
329-336: LGTM! REST request payloads now clearer.The conversion of JSON request bodies to text blocks makes the test REST operations much easier to understand. All JSON structures are preserved correctly.
Also applies to: 410-417, 1374-1380, 1433-1441
server/src/test/java/org/opensearch/search/builder/SearchSourceBuilderTests.java (1)
124-752: LGTM! Comprehensive text block adoption across test methods.The systematic conversion of JSON test inputs to text blocks significantly improves readability across this test class. All conversions are accurate, with proper use of
.formatted()for dynamic value injection.server/src/test/java/org/opensearch/index/query/RegexpQueryBuilderTests.java (1)
86-177: LGTM! Query test inputs now more maintainable.The text block conversion improves readability of both valid query inputs and error case scenarios. The
.formatted()usage correctly injects dynamic test values.server/src/test/java/org/opensearch/index/query/SimpleQueryStringBuilderTests.java (1)
279-283: LGTM! Query string test inputs improved.The text block format makes the simple query string JSON structures clearer. The
.formatted()injection intestDefaultFieldParsingis correctly implemented.Also applies to: 400-417
server/src/test/java/org/opensearch/index/query/PrefixQueryBuilderTests.java (1)
67-159: LGTM! Prefix query tests now clearer.The conversion to text blocks improves readability for both standard query formats and error case validation. The
.formatted()method correctly injects field names and values.qa/smoke-test-http/src/test/java/org/opensearch/http/SystemIndexRestIT.java (1)
130-192: LGTM - Text block conversions are correct.The JSON payloads for component template, index template, and system index mappings have been properly converted to text blocks. The formatting is consistent with proper indentation, and the JSON structure is preserved.
test/framework/src/test/java/org/opensearch/test/rest/yaml/section/DoSectionTests.java (1)
176-722: LGTM - YAML and JSON test payloads correctly converted to text blocks.The numerous test payloads throughout this file have been properly migrated to text blocks. Both the input YAML strings and expected JSON bodies are consistently formatted. The indentation is preserved correctly for YAML parsing.
server/src/test/java/org/opensearch/common/xcontent/builder/XContentBuilderTests.java (1)
325-354: LGTM - Pretty-print assertion text blocks are correctly formatted.The expected output strings for JSON and YAML pretty-printing have been properly converted to text blocks. The use of
.trim()on line 354 correctly handles the potential trailing whitespace difference between the builder output and the text block.client/rest-high-level/src/test/java/org/opensearch/client/documentation/IndicesClientDocumentationIT.java (4)
325-412: LGTM - Documentation example JSON payloads correctly converted.The JSON payloads in the create-index examples have been properly converted to text blocks while preserving the documentation tags. The formatting improves readability of the documentation examples.
477-709: LGTM - Mapping and field mapping JSON payloads are well-formatted.The text block conversions for put-mapping and get-field-mappings maintain proper JSON structure and documentation tag placement.
2016-2179: LGTM - Settings and template JSON payloads correctly migrated.The index settings source and template whole-source payloads have been properly converted to text blocks with correct indentation and preserved documentation tags.
2379-2386: LGTM - V2 index template mappings converted correctly.The composable index template mapping JSON is properly formatted as a text block.
modules/ingest-common/src/internalClusterTest/java/org/opensearch/ingest/common/IngestRestartIT.java (3)
91-97: LGTM - Pipeline JSON with string interpolation correctly converted.The pipeline definition properly uses
.formatted()for interpolatingMockScriptEngine.NAMEinto the text block. The JSON structure is preserved.
129-140: LGTM - Script and non-script pipeline definitions correctly formatted.Both pipeline definitions are properly converted to text blocks, with
pipelineWithScriptusing.formatted()for the script engine name interpolation.
206-258: LGTM - Stored script and dedicated ingest node pipeline definitions correctly migrated.The remaining pipeline definitions maintain proper JSON structure as text blocks, with appropriate use of
.formatted()where variable interpolation is needed.server/src/test/java/org/opensearch/index/query/MatchPhrasePrefixQueryBuilderTests.java (1)
96-101: LGTM! Clean migration to text blocks.The conversion from concatenated strings to Java text blocks improves readability while preserving the exact JSON structure. The use of
.formatted()for string interpolation is appropriate.Also applies to: 158-176, 180-200, 206-216, 220-226
server/src/test/java/org/opensearch/index/query/BoolQueryBuilderTests.java (1)
257-305: LGTM! Clean text block conversions.The JSON test cases are now much more readable with text blocks. The structure is preserved and the syntax is correct.
Also applies to: 316-317, 323-324, 330-331, 337-338, 344-345, 351-352, 363-364, 379-380, 390-391
server/src/test/java/org/opensearch/index/query/DisMaxQueryBuilderTests.java (1)
80-85: LGTM! Clean migration to text blocks.All JSON test strings have been successfully converted to text blocks with proper formatting and interpolation. The changes improve readability without altering functionality.
Also applies to: 96-111, 121-143
server/src/test/java/org/opensearch/index/query/MatchPhraseQueryBuilderTests.java (1)
105-110: LGTM! Clean text block adoption.The conversion to text blocks with
.formatted()interpolation improves test readability while maintaining the exact JSON structure and test semantics.Also applies to: 151-168, 174-184, 195-205, 209-215
test/framework/src/test/java/org/opensearch/test/rest/yaml/section/ClientYamlTestSuiteTests.java (1)
59-66: LGTM! Excellent YAML readability improvement.The conversion of YAML test definitions to text blocks significantly improves readability and maintainability. The structure is preserved correctly, and the multi-line format makes it much easier to understand the test scenarios.
Also applies to: 69-76, 78-88, 152-175, 236-275, 322-346
|
❌ Gradle check result for ad43583: 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? |
Signed-off-by: Craig Perkins <cwperx@amazon.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/src/test/java/org/opensearch/index/query/SimpleQueryStringBuilderTests.java (1)
279-283: Verify the extra whitespace in the JSON string.The text block conversion looks correct, but line 282 has extra spaces before the closing brace:
"query" : "%s" }. This may be intentional to preserve the original format, but it appears unusual.If the extra whitespace is unintentional, consider this cleanup:
String contentString = String.format(Locale.ROOT, """ { "simple_query_string" : { - "query" : "%s" } + "query" : "%s"} }""", query);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
modules/ingest-common/src/internalClusterTest/java/org/opensearch/ingest/common/IngestRestartIT.java(5 hunks)modules/mapper-extras/src/test/java/org/opensearch/index/query/RankFeatureQueryBuilderTests.java(3 hunks)server/src/test/java/org/opensearch/index/query/DisMaxQueryBuilderTests.java(4 hunks)server/src/test/java/org/opensearch/index/query/MatchPhrasePrefixQueryBuilderTests.java(3 hunks)server/src/test/java/org/opensearch/index/query/MatchPhraseQueryBuilderTests.java(4 hunks)server/src/test/java/org/opensearch/index/query/MatchQueryBuilderTests.java(3 hunks)server/src/test/java/org/opensearch/index/query/PrefixQueryBuilderTests.java(3 hunks)server/src/test/java/org/opensearch/index/query/RegexpQueryBuilderTests.java(4 hunks)server/src/test/java/org/opensearch/index/query/SimpleQueryStringBuilderTests.java(2 hunks)test/framework/src/test/java/org/opensearch/test/rest/yaml/section/SkipSectionTests.java(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- test/framework/src/test/java/org/opensearch/test/rest/yaml/section/SkipSectionTests.java
- server/src/test/java/org/opensearch/index/query/MatchPhrasePrefixQueryBuilderTests.java
- server/src/test/java/org/opensearch/index/query/MatchQueryBuilderTests.java
- server/src/test/java/org/opensearch/index/query/RegexpQueryBuilderTests.java
- modules/mapper-extras/src/test/java/org/opensearch/index/query/RankFeatureQueryBuilderTests.java
- modules/ingest-common/src/internalClusterTest/java/org/opensearch/ingest/common/IngestRestartIT.java
⏰ 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: Analyze (java)
- GitHub Check: precommit (21, macos-15)
- GitHub Check: precommit (25, ubuntu-24.04-arm)
- GitHub Check: precommit (21, ubuntu-24.04-arm)
- GitHub Check: precommit (21, macos-15-intel)
- GitHub Check: precommit (25, macos-15-intel)
- GitHub Check: precommit (21, windows-2025, true)
- GitHub Check: precommit (25, macos-15)
- GitHub Check: precommit (25, ubuntu-latest)
- GitHub Check: precommit (21, windows-latest)
- GitHub Check: precommit (21, ubuntu-latest)
- GitHub Check: precommit (25, windows-latest)
- GitHub Check: assemble (21, windows-latest)
- GitHub Check: assemble (25, ubuntu-latest)
- GitHub Check: detect-breaking-change
- GitHub Check: assemble (21, ubuntu-24.04-arm)
- GitHub Check: assemble (25, windows-latest)
- GitHub Check: assemble (21, ubuntu-latest)
- GitHub Check: assemble (25, ubuntu-24.04-arm)
🔇 Additional comments (13)
server/src/test/java/org/opensearch/index/query/PrefixQueryBuilderTests.java (4)
46-46: LGTM!The
Localeimport is correctly added to support locale-independent formatting in the text block conversion below.
68-73: LGTM!The conversion to a text block with
String.format(Locale.ROOT, ...)improves readability while maintaining locale-independent formatting. The JSON structure is properly preserved.
137-147: LGTM!The text block conversion correctly preserves the JSON structure for the multi-field test case, improving readability without altering test behavior.
151-157: LGTM!The text block conversion correctly preserves the short-format JSON structure, maintaining test behavior while improving code readability.
server/src/test/java/org/opensearch/index/query/SimpleQueryStringBuilderTests.java (1)
400-417: LGTM! Clean text block conversion.The text block syntax is correct, indentation is consistent, and the JSON structure is properly preserved. This improves readability compared to concatenated or escaped strings.
server/src/test/java/org/opensearch/index/query/MatchPhraseQueryBuilderTests.java (5)
50-50: LGTM! Necessary import for locale-safe formatting.The
Localeimport supports theLocale.ROOTusage inString.formaton line 106, ensuring consistent formatting regardless of system locale.
106-111: Excellent use of text blocks and locale-safe formatting.The conversion to a text block with
String.format(Locale.ROOT, ...)improves readability while ensuring consistent formatting across different system locales. The format specifiers work correctly with the alphanumeric test data generated in lines 103-104.
152-169: LGTM! Text blocks greatly improve JSON readability.The conversion to text blocks makes the test JSON much more readable while preserving the original test logic (shorthand input parsed to expanded output).
175-185: LGTM! Clean text block conversion.The text block format enhances readability of the JSON test input without changing test behavior.
196-216: LGTM! Text blocks improve readability of error test cases.The conversion to text blocks makes the multi-field JSON structures clearer while preserving the error-testing behavior.
server/src/test/java/org/opensearch/index/query/DisMaxQueryBuilderTests.java (3)
48-48: LGTM: Necessary import for locale-aware formatting.The
Localeimport supports theString.format(Locale.ROOT, ...)calls introduced with the text block conversions, ensuring consistent formatting across locales.
97-112: LGTM: Clean text block conversion with proper formatting.The conversion to a text block with
String.format(Locale.ROOT, ...)improves readability while maintaining consistent JSON structure and locale-independent formatting.
122-144: LGTM: Excellent readability improvement.The static JSON conversion to a text block significantly improves readability without requiring
String.format, making the test data much easier to understand and maintain.
server/src/test/java/org/opensearch/index/query/DisMaxQueryBuilderTests.java
Show resolved
Hide resolved
|
❌ Gradle check result for a580bcf: 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? |
|
❌ Gradle check result for 8fa91a0: 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? |
|
❕ Gradle check result for 8fa91a0: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #20269 +/- ##
============================================
- Coverage 73.20% 73.17% -0.03%
+ Complexity 71745 71728 -17
============================================
Files 5795 5795
Lines 328304 328298 -6
Branches 47283 47279 -4
============================================
- Hits 240334 240238 -96
- Misses 68663 68790 +127
+ Partials 19307 19270 -37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Description
Now that the minimum java version is 21 we can use Java Text Blocks throughout the repo.
This PR contains many adaptations to switch to using Text Blocks which greatly enhances the readability of json and yaml in tests.
Related Issues
No issue filed. Modernization using java >= 15 java patterns.
Check List
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
✏️ Tip: You can customize this high-level summary in your review settings.