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

Fixed flaky test-case: ConsumerJsonRecordTest.should_serialize_a_record_with_headers #684

Merged
merged 6 commits into from
Oct 8, 2024

Conversation

hermya
Copy link
Contributor

@hermya hermya commented Oct 6, 2024

Fixed flaky test-case

Fixes Issue

PR Branch
fork

Motivation and Context

ConsumerJsonRecordTest.should_serialize_a_record_with_headers is a flaky test, fixed in this PR.

POINT OF FAILURE -> random attribute ordering in hashmap
Fixed using NonDex pugin
Suggested command to check all flaky tests :

mvn nondex:nondex

For particular test:
Add the following lines to your pom.xml

  <plugin> <!-- for flaky testing -->
      <groupId>edu.illinois</groupId>
      <artifactId>nondex-maven-plugin</artifactId>
      <version>2.1.7</version>
  </plugin>

and run

mvn -pl ./core nondex:nondex -Dtest=org.jsmart.zerocode.core.kafka.receive.message.ConsumerJsonRecordTest#should_serialize_a_record_with_headers -DnondexRuns=10

OR

mvn -pl ./core edu.illinois:nondex-maven-plugin:2.1.7:nondex -Dtest=org.jsmart.zerocode.core.kafka.receive.message.ConsumerJsonRecordTest#should_serialize_a_record_with_headers -DnondexRuns=10

For more information : https://github.com/TestingResearchIllinois/NonDex

Checklist:

  • New Unit tests were added

    • Covered in existing Unit tests
  • Integration tests were added

    • Covered in existing Integration tests
  • Test names are meaningful

  • Feature manually tested and outcome is successful

  • PR doesn't break any of the earlier features for end users

    • WARNING! This might break one or more earlier earlier features, hence left a comment tagging all reviewrs
  • Branch build passed in CI

  • No 'package.*' in the imports

  • Relevant DOcumentation page added or updated with clear instructions and examples for the end user

    • Not applicable. This was only a code refactor change, no functional or behaviourial changes were introduced
  • Http test added to http-testing module(if applicable) ?

    • Not applicable. The changes did not affect HTTP automation flow
  • Kafka test added to kafka-testing module(if applicable) ?

    • Not applicable. The changes did not affect Kafka automation flow

hermya and others added 6 commits August 31, 2024 16:10
ConsumerJsonRecordTest.should_serialize_a_record_with_headers
POINT OF FAILURE -> random attribute ordering in hashmap
Fixed using NonDex, plugin added in parent pom.
Suggested command to check flaky tests :
> mvn nondex:nondex

For particular test:
>mvn -pl ./core nondex:nondex -Dtest=org.jsmart.zerocode.core.kafka.receive.message.ConsumerJsonRecordTest#should_serialize_a_record_with_headers -DnondexRuns=10

For more information : https://github.com/TestingResearchIllinois/NonDex
…o modify pull request message about the pom.xml change
Fixed flaky test-case: ConsumerJsonRecordTest.should_serialize_a_record_with_headers
@nirmalchandra
Copy link
Collaborator

@hermya , appreciate the fix. Can you create a Issue(short, one or two lines in fine) and link to this ticket please?

We'll have a look soon

@hermya
Copy link
Contributor Author

hermya commented Oct 6, 2024

@nirmalchandra, thank you! Created an issue and updated it in the above comment as well.
For your ref: /issues/685

@nirmalchandra
Copy link
Collaborator

nirmalchandra commented Oct 6, 2024

@nirmalchandra, thank you! Created an issue and updated it in the above comment as well. For your ref: /issues/685

Thank you @hermya , can you please put couple of line about the fix (how the test was fixed) ?
Also, please explain how it was never failed during the CI build earlier?
Is it failing in your local laptop (sometimes?) ?

In the mean time I will get the CI triggered(as soon as we see the details I mean).

Also, just checking, you have updated to "LinkedhashMap" in the test, which looks fine. But was the maven dependency which you added&removed from the POM file, was pushed by mistake ?

@hermya
Copy link
Contributor Author

hermya commented Oct 6, 2024

You're welcome @nirmalchandra !

1. Can you please put couple of line about the fix (how the test was fixed)?

In HashMap implementation of Map, there's no guarantee of key ordering. LinkedHashMap keeps the order of insertion and it's toString() or keySet() will also maintain that order, ensuring deterministic representation.

2. Please explain how it was never failed during the CI build earlier:

It might have never failed because the hash-key ordering is dependent on the current state of underlying jvm. It is possible that it fails on a different environment

3. Is it failing in your local laptop (sometimes?) ?

It fails with the nondex. Nondex is a useful tool for such situations, where outputs of non-deterministic states are compared.

4. But was the maven dependency which you added&removed from the POM file, was pushed by mistake?

I added it earlier, thinking that it'd be a good idea for your experimentation. However, you can run nondex without it using slightly modified command mentioned in main comment, so I removed it for cleanliness.

@nirmalchandra nirmalchandra merged commit a47a627 into authorjapps:master Oct 8, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants