Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR elastic#138668

Type: Clean (correct implementation)

Original PR Title: Test utility for BytesReference equality
Original PR Description: Today if you assert two BytesReference objects are equal, and they
aren't, then the test output is unhelpful because BytesReference
instances do not include their contents in their default string
representations. This commit introduces a new matcher specifically for
testing BytesReference instances for equality which generates more
useful output.
Original PR URL: elastic#138668


PR Type

Enhancement, Tests


Description

  • Introduce equalBytes() matcher for improved BytesReference equality testing

  • Provides detailed hex dump output on assertion failures for better debugging

  • Replace equalTo() with equalBytes() across 60+ test files

  • Enhance test readability with explicit BytesReference comparison semantics


Diagram Walkthrough

flowchart LR
  A["New Matcher<br/>BytesReferenceTestUtils"] -->|provides| B["equalBytes<br/>Matcher"]
  B -->|used in| C["60+ Test Files"]
  C -->|replaces| D["equalTo<br/>Comparisons"]
  B -->|generates| E["Hex Dump<br/>Output"]
Loading

File Walkthrough

Relevant files
Enhancement
1 files
BytesReferenceTestUtils.java
New test utility for BytesReference equality assertions   
+77/-0   
Tests
59 files
BytesReferenceTestUtilsTests.java
Unit tests for BytesReferenceTestUtils matcher                     
+49/-0   
PointInTimeIT.java
Replace equalTo with equalBytes for BytesReference             
+15/-14 
ApiKeyServiceTests.java
Replace equalTo with equalBytes for BytesReference             
+25/-18 
FsBlobContainerTests.java
Replace equalTo with equalBytes for BytesReference             
+16/-14 
ClientTransformIndexerTests.java
Replace equalTo with equalBytes for BytesReference             
+11/-10 
RestRequestTests.java
Replace equalTo with equalBytes for BytesReference             
+12/-11 
AbstractBytesReferenceTestCase.java
Replace equalTo with equalBytes for BytesReference             
+8/-7     
DefaultRestChannelTests.java
Replace equalTo with equalBytes for BytesReference             
+8/-7     
AbstractThirdPartyRepositoryTestCase.java
Replace equalTo with equalBytes for BytesReference             
+5/-4     
BulkRequestTests.java
Replace equalTo with equalBytes for BytesReference             
+4/-3     
S3RepositoryThirdPartyTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-2     
BytesStreamsTests.java
Replace equalTo with equalBytes for BytesReference             
+4/-3     
GoogleCloudStorageBlobContainerRetriesTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-2     
GoogleCloudStorageBlobContainerStatsTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-2     
RecyclerBytesStreamOutputTests.java
Replace equalTo with equalBytes for BytesReference             
+4/-3     
RetrySearchIntegTests.java
Replace equalTo with equalBytes for BytesReference             
+5/-4     
ChunkedRestResponseBodyPartTests.java
Replace equalTo with equalBytes for BytesReference             
+4/-2     
ClientScrollableHitSourceTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
RetryingInputStreamTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-2     
CompositeBytesReferenceTests.java
Replace equalTo with equalBytes for BytesReference             
+4/-3     
GetActionIT.java
Replace equalTo with equalBytes for BytesReference             
+4/-3     
ShardBulkInferenceActionFilterTests.java
Replace equalTo with equalBytes for BytesReference             
+4/-3     
RoundTripTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
PagedBytesReferenceTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-1     
BlobContainerUtilsTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
ChunkedLoggingStreamTests.java
Replace equalTo with equalBytes for BytesReference             
+7/-8     
RestGetSourceActionTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
GetAutoscalingCapacityActionResponseTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-2     
Netty4HttpPipeliningHandlerTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
SearchFieldsIT.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
EngineTestCase.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
AuthenticationTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
HttpTracerTests.java
Replace equalTo with equalBytes for BytesReference             
+5/-3     
ShardSearchRequestTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
GoogleCloudStorageBlobStoreRepositoryTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
GetFieldMappingsResponseTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-1     
ChunkedTrainedModelPersisterIT.java
Replace equalTo with equalBytes for BytesReference             
+3/-2     
TermVectorsUnitTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
TranslogTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
InnerHitBuilderTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
TranslogHeaderWriterTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
PITAwareQueryClientTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-2     
DeflateCompressedXContentTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
BytesStreamOutputTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-1     
CancellationTests.java
Replace equalTo with equalBytes for BytesReference             
+3/-2     
RestContentAggregatorTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
IndexRequestTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
MultipartUploadTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
RestMainActionTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
PutPipelineRequestTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
LongBoundsTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
XContentDataHelperTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
DoubleBoundsTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
SubjectTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
OutboundHandlerTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
NativePrivilegeStoreTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
DeflateCompressTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
RepositoryAnalysisSuccessIT.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
DatafeedJobTests.java
Replace equalTo with equalBytes for BytesReference             
+2/-1     
Additional files
4 files
SamlAuthenticationStateTests.java +2/-1     
MonitoringBulkDocTests.java +2/-1     
MonitoringBulkRequestTests.java +2/-1     
BytesReferenceMonitoringDocTests.java +2/-1     

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Not Applicable: The PR only updates test assertions and introduces a test-only matcher without adding or
modifying production code paths that perform critical actions or logging; audit trail
requirements cannot be assessed from these changes alone.

Referred Code
    .withContent(responseBody, null)
    .build();

assertThat(
    ChunkedLoggingStreamTestUtils.getDecodedLoggedBody(
        LogManager.getLogger(HTTP_BODY_TRACER_LOGGER),
        Level.TRACE,
        "[" + request.getRequestId() + "] request body",
        ReferenceDocs.HTTP_TRACER,
        () -> assertNotNull(tracer.maybeLogRequest(request, null))
    ),
    equalBytes(responseBody)
);

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Test Utility Scope: The new matcher enhances test failure messages but does not affect runtime error handling;
robustness of production error handling cannot be determined from test-only changes.

Referred Code
public enum BytesReferenceTestUtils {
    ;

    /**
     * Like {@link Matchers#equalTo} except it reports the contents of the respective {@link BytesReference} instances on mismatch.
     */
    public static Matcher<BytesReference> equalBytes(BytesReference expected) {
        return new BaseMatcher<>() {
            @Override
            public boolean matches(Object actual) {
                return Objects.equals(expected, actual);
            }

            @Override
            public void describeTo(Description description) {
                if (expected == null) {
                    description.appendValue(null);
                } else {
                    appendBytesReferenceDescription(expected, description);
                }
            }


 ... (clipped 35 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Test Detail Output: The matcher prints hex dumps of BytesReference for assertion mismatches in tests; while
appropriate for tests, impact on user-facing errors or secure logs in production is not
assessed here.

Referred Code
public enum BytesReferenceTestUtils {
    ;

    /**
     * Like {@link Matchers#equalTo} except it reports the contents of the respective {@link BytesReference} instances on mismatch.
     */
    public static Matcher<BytesReference> equalBytes(BytesReference expected) {
        return new BaseMatcher<>() {
            @Override
            public boolean matches(Object actual) {
                return Objects.equals(expected, actual);
            }

            @Override
            public void describeTo(Description description) {
                if (expected == null) {
                    description.appendValue(null);
                } else {
                    appendBytesReferenceDescription(expected, description);
                }
            }


 ... (clipped 35 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Test Logging Only: Several tests validate logged HTTP bodies and bytes equality; no production logging
changes are introduced, so secure logging compliance cannot be fully assessed from this
diff.

Referred Code
);

var responseBody = new BytesArray(randomUnicodeOfLengthBetween(1, 100).getBytes(StandardCharsets.UTF_8));
assertThat(
    ChunkedLoggingStreamTestUtils.getDecodedLoggedBody(
        LogManager.getLogger(HttpTracerTests.HTTP_BODY_TRACER_LOGGER),
        Level.TRACE,
        "[" + request.getRequestId() + "] response body",
        ReferenceDocs.HTTP_TRACER,
        () -> channel.sendResponse(new RestResponse(RestStatus.OK, RestResponse.TEXT_CONTENT_TYPE, responseBody))
    ),
    equalBytes(responseBody)
);

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Tests Only: Changes replace equality assertions with a bytes matcher in tests and do not modify input
validation logic; security of input handling in production cannot be evaluated from these
changes.

Referred Code
public void testContentOrSourceParam() {
    Exception e = expectThrows(ElasticsearchParseException.class, () -> contentRestRequest("", emptyMap()).contentOrSourceParam());
    assertEquals("request body or source parameter is required", e.getMessage());
    assertThat(contentRestRequest("stuff", emptyMap()).contentOrSourceParam().v2(), equalBytes(new BytesArray("stuff")));
    assertThat(
        contentRestRequest("stuff", Map.of("source", "stuff2", "source_content_type", "application/json")).contentOrSourceParam().v2(),
        equalBytes(new BytesArray("stuff"))
    );
    assertThat(
        contentRestRequest("", Map.of("source", "{\"foo\": \"stuff\"}", "source_content_type", "application/json"))
            .contentOrSourceParam()
            .v2(),
        equalBytes(new BytesArray("{\"foo\": \"stuff\"}"))
    );

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Assert against the correct decoded value

Correct the test assertion by initializing decodedBytes from the decoded builder
instead of the original builder to ensure the decoded content is actually being
verified.

server/src/test/java/org/elasticsearch/index/mapper/XContentDataHelperTests.java [130-148]

 public void testEmbeddedObject() throws IOException {
     // XContentType.JSON never produces VALUE_EMBEDDED_OBJECT
     XContentBuilder builder = XContentBuilder.builder(XContentType.CBOR.xContent());
     builder.startObject();
     CompressedXContent embedded = new CompressedXContent("{\"field\":\"value\"}");
     builder.field("bytes", embedded.compressed());
     builder.endObject();
     var originalBytes = BytesReference.bytes(builder);
 
     try (XContentParser parser = createParser(builder)) {
         assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
         assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken());
         assertEquals("bytes", parser.currentName());
         assertEquals(XContentParser.Token.VALUE_EMBEDDED_OBJECT, parser.nextToken());
         assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken());
     }
 
     var encoded = XContentDataHelper.encode(createParser(builder));
     assertTrue(XContentDataHelper.isEncodedObject(encoded));
 
     var decoded = XContentFactory.jsonBuilder();
     XContentDataHelper.decodeAndWrite(decoded, encoded);
-    var decodedBytes = BytesReference.bytes(builder);
+    var decodedBytes = BytesReference.bytes(decoded);
 
     assertThat(decodedBytes, equalBytes(originalBytes));
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a bug in the test where decodedBytes is initialized from the wrong XContentBuilder, making the assertion a self-comparison and thus ineffective.

Medium
General
Correct the order of assertion arguments

Reverse the arguments in the assertThat call to match the standard
assertThat(actual, expected) convention for better failure messages.

modules/transport-netty4/src/test/java/org/elasticsearch/http/netty4/Netty4HttpPipeliningHandlerTests.java [551-553]

 private static void assertContentAtIndexEquals(List<Object> messagesSeen, int index, BytesReference single) {
-    assertThat(single, equalBytes(Netty4Utils.toBytesReference(((ByteBufHolder) messagesSeen.get(index)).content())));
+    assertThat(Netty4Utils.toBytesReference(((ByteBufHolder) messagesSeen.get(index)).content()), equalBytes(single));
 }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that the arguments for assertThat are swapped compared to the original assertEquals, which would lead to confusing test failure messages.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants