Skip to content

Fix stringPayload ordering to match test expectations #2362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anjanmandal
Copy link

@anjanmandal anjanmandal commented May 8, 2025

What is the purpose of this PR?

This pull request aims to fix a flaky test in the HttpEventPublisher.java by modifying the getStringPayload() method in HttpEventPublisher.java. The previous implementation caused inconsistencies in the serialized JSON string due to the output ordering the HttpEventPublisherTest.java was looking for.

Why the test fails?

The flaky test failed because getStringPayload() did not return the JSON in the format HttpEventPublisherTest.java inside src/test/java/com/google/cloud/teleport/splunk/. This inconsistency triggered test failures when run under NonDex.

Expected results

The test should consistently pass when getStringPayload() produces the same output regardless of runtime order.

Actual results

The test occasionally fails because the payload string produced by getStringPayload() was not ordered properly.

Description of fix

Rewrote the getStringPayload() method to manually and deterministically construct the expected JSON string. This eliminates any GSON-related ordering issues by using StringBuilder to control field layout and ordering explicitly for each SplunkEvent.

Approach Taken

To resolve the flaky behavior in the stringPayloadTest, I first closely examined the test outputs, particularly the discrepancies between the expected and but was values. After multiple test runs and several hours of observation, I identified that the test failure stemmed from non-deterministic ordering in the serialized JSON payload. The output varied across runs, indicating that the issue was related to how the payload string was being generated.

I then navigated to the test implementation at:

src/test/java/com/google/cloud/teleport/splunk/HttpEventPublisherTest

Inside the stringPayloadTest method, I reviewed the expected structure of the JSON string. Using that insight, I investigated the production code in:

src/main/java/com/google/cloud/teleport/splunk/HttpEventPublisher.java

The method in question, getStringPayload(), originally relied on GSON.toJson() to serialize each SplunkEvent. However, GSON does not guarantee field ordering, which explained the flaky test failures under NonDex.

To fix this, I manually reconstructed the JSON string using a StringBuilder to ensure consistent field ordering. I preserved the exact order expected by the test: time, host, source, sourcetype, index, and event. This deterministic construction guarantees the same output across all runs, eliminating the root cause of the flakiness.

Here’s a simplified view of the updated logic:

String getStringPayload(List<SplunkEvent> events) {
  StringBuilder sb = new StringBuilder();
  for (SplunkEvent event : events) {
    sb.append('{')
      .append("\"time\":").append(event.time()).append(',')
      .append("\"host\":").append(GSON.toJson(event.host())).append(',')
      .append("\"source\":").append(GSON.toJson(event.source())).append(',')
      .append("\"sourcetype\":").append(GSON.toJson(event.sourceType())).append(',')
      .append("\"index\":").append(GSON.toJson(event.index())).append(',')
      .append("\"event\":").append(GSON.toJson(event.event()))
      .append('}');
  }
  return sb.toString();
}

After implementing this fix, the test passed consistently across multiple NonDex runs, confirming the resolution of the flakiness.

Copy link

google-cla bot commented May 8, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant