Skip to content

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
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