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

[Fix] Fix flaky test OptionUtilTest.test #5894

Merged
merged 4 commits into from
Nov 24, 2023
Merged

Conversation

ThugJudy
Copy link
Contributor

Purpose of this pull request

The test case org.apache.seatunnel.api.configuration.util.OptionUtilTest.test fails due to the non-deterministic behavior of the function

clazz.getDeclaredFields 

https://github.com/ThugJudy/seatunnel/blob/b8c313045ea3301f3fa08bc46a9014a4ab9ac66f/seatunnel-api/src/main/java/org/apache/seatunnel/api/configuration/util/OptionUtil.java#L72 which does not guarantee the ordering of elements in the class object.

Due to this, the arrayList of options created does not have a definite order in every run.

I found and confirmed the flaky behavior using an open-source research tool NonDex, which shuffles implementations of nondeterminism operations.

Reproduce

The following command can be used to reproduce assertion failures and verify the fix:

mvn edu.illinois:nondex-maven-plugin:2.1.1:nondex -pl Seatunnel-api -Dtest='org.apache.seatunnel.api.configuration.util.OptionUtilTest#test'

Fix

Sorting the options list based on key ensures that the order of the elements is constant during the test run

https://github.com/ThugJudy/seatunnel/blob/b8c313045ea3301f3fa08bc46a9014a4ab9ac66f/seatunnel-api/src/test/java/org/apache/seatunnel/api/configuration/util/OptionUtilTest.java#L34

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

@Hisoka-X
Copy link
Member

Thanks for create this PR @ThugJudy . Could you open CI on your fork repository? Please follow the guide https://github.com/apache/seatunnel/pull/5894/checks?check_run_id=18912326036. Thanks.

@Hisoka-X Hisoka-X changed the title [Fix] flaky test org.apache.seatunnel.api.configuration.util.OptionUtilTest.test [Fix] Fix flaky test OptionUtilTest.test Nov 23, 2023
@Hisoka-X Hisoka-X added the test label Nov 23, 2023
@ThugJudy
Copy link
Contributor Author

Thanks for create this PR @ThugJudy . Could you open CI on your fork repository? Please follow the guide https://github.com/apache/seatunnel/pull/5894/checks?check_run_id=18912326036. Thanks.

I opened CI on my forked repo.

@Hisoka-X
Copy link
Member

The UT not passed. Could you retry failed UT?

@ThugJudy
Copy link
Contributor Author

The UT not passed. Could you retry failed UT?

I re-ran it and it passed now. Please check.

@hailin0 hailin0 merged commit a6bd041 into apache:dev Nov 24, 2023
3 of 4 checks passed
alextinng pushed a commit to alextinng/seatunnel that referenced this pull request Dec 19, 2023
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.

3 participants