Skip to content
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

Flaky Test in org.apache.druid.java.util.emitter.core.EmitterTest.testBasicAuthenticationAndNewlineSeparating #15137

Closed
wants to merge 2 commits into from

Conversation

zhouyinn
Copy link

What is the purpose of this PR

  • This PR fixes the error resulting from the flaky test org.apache.druid.java.util.emitter.core.EmitterTest
  • The mentioned test may fail or pass without changes made to the source code when it is run in different JVMs due to Json's non-deterministic iteration order.

Why the test fails

The test testBasicAuthenticationAndNewlineSeparating fails due to the order of byte buffer data returned by the org.asynchttpclient.Request object is non-deterministic. This issue arises because asynchronous methods can process data in a different order from the order of reception

Reproduce the test failure

mvn -pl org.apache.druid:druid-processing edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest="org.apache.druid.java.util.emitter.core.EmitterTest#testBasicAuthenticationAndNewlineSeparating"

  • Fixing the flaky test now may prevent unreliable test results in future code changes.

Expected results

The test should run successfully when run with NonDex.

Test failing before changes:

[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 4.573 s <<< FAILURE! -- in org.apache.druid.java.util.emitter.core.EmitterTest
[ERROR] org.apache.druid.java.util.emitter.core.EmitterTest.testBasicAuthenticationAndNewlineSeparating -- Time elapsed: 4.561 s <<< FAILURE! java.lang.AssertionError at org.junit.Assert.fail(Assert.java:87)

EXPECTED:
{"metrics":{"value":1},"feed":"test"}
{"metrics":{"value":2},"feed":"test"}

BUT FOUND:
{"feed":"test","metrics":{"value":1}}
{"metrics":{"value":2},"feed":"test"}

Fix

  • Instead of directly comparing strings that represent JSON objects, compare the equality of complete JSON trees by evaluating the equality of their corresponding root nodes.
  • Utilize Jackson to read the input JSON as a JsonNode and compare them. Even when the order of attributes in the input JSON variables differs, the equals() method will disregard the order and consider them as equal.

Copy link

github-actions bot commented Mar 7, 2024

This pull request has been marked as stale due to 60 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If you think
that's incorrect or this pull request should instead be reviewed, please simply
write any comment. Even if closed, you can still revive the PR at any time or
discuss it on the [email protected] list.
Thank you for your contributions.

@github-actions github-actions bot added the stale label Mar 7, 2024
Copy link

github-actions bot commented Apr 5, 2024

This pull request/issue has been closed due to lack of activity. If you think that
is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Apr 5, 2024
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