[WFCORE-6214] Fixed Flaky Tests in HelpSupportTestCase.testStandalone and BootScriptInvokerTestCase #5366
Conversation
|
Hello, RishabhPrabhu5. I'm waiting for one of the admins to verify this patch with /ok-to-test in a comment. |
|
Hi @RishabhPrabhu5 , thank you for contributing to WildFly! Could you add the Jira number to the commit message? Like what you did with the title, having the Jira number in the commit message is a requirement for us. It helps us to track the history with the Jira issue. |
| Set<SynopsisOption> conflicts = new HashSet<>(currentOption.conflictWith); | ||
| Set<SynopsisOption> conflicts2 = new HashSet<>(currentOption.conflictWith); | ||
| Set<SynopsisOption> conflicts = new LinkedHashSet<>(currentOption.conflictWith); | ||
| Set<SynopsisOption> conflicts2 = new LinkedHashSet<>(currentOption.conflictWith); |
There was a problem hiding this comment.
I think that the correct fix would be to use
Set<SynopsisOption> conflicts2 = new TreeSet<>(Comparator.comparing(SynopsisOption::getName));
conflicts2.addAll(currentOption.conflictWith);
In order to iterate the conflicts using the natural ordering of Options (name).
| @@ -180,8 +181,8 @@ private String addSynopsisOption(SynopsisOption currentOption) { | |||
| } | |||
| synopsisBuilder.append(" |"); | |||
| // Keep the set of all conflicts, they will be removed from all conflicts, being handled at this level. | |||
| Set<SynopsisOption> conflicts = new HashSet<>(currentOption.conflictWith); | |||
| Set<SynopsisOption> conflicts2 = new HashSet<>(currentOption.conflictWith); | |||
| Set<SynopsisOption> conflicts = new LinkedHashSet<>(currentOption.conflictWith); | |||
There was a problem hiding this comment.
This change is not needed.
|
@RishabhPrabhu5 thank-you for your fix. I added a comment. Running your plugin I found another one in : https://github.com/wildfly/wildfly-core/blob/main/cli/src/test/java/org/jboss/as/cli/impl/BootScriptInvokerTestCase.java#L217 Thank-you! |
RishabhPrabhu5
force-pushed
the
fix-flaky-test
branch
from
f14c461
to
ee2aace
Compare
RishabhPrabhu5
changed the title
|
Thank you for the feedback, I have made the changes requested and have updated the JIRA description. Please let me know if there is anything more I can do. |
github-actions
bot
added
the
deps-ok
|
/ok-to-test |
|
Windows - Bootable JAR - JDK 11 is unrelated to this PR. |
yersan
added
the
ready-for-merge
|
@RishabhPrabhu5 , merged!, thanks for contributing to WildFly, and especially for the great PR description, that helps in doing the review. |
|
Thank you for your email and accepting my PR. I hope my fix was useful, and
I look forward to contribute more in the future. Thanks!
…On Wed, Feb 15, 2023 at 5:06 AM Yeray Borges ***@***.***> wrote:
@RishabhPrabhu5 <https://github.com/RishabhPrabhu5> , merged!, thanks for
contributing to WildFly, and especially for the great PR description, that
helps in doing the review.
—
Reply to this email directly, view it on GitHub
<#5366 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AYYIO436OBSDNIMQQNH5QDTWXSTCVANCNFSM6AAAAAAUKVD7EY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
What is the purpose of this PR
org.jboss.as.cli.impl.aesh.HelpSupportTestCaseandorg.jboss.as.cli.impl.BootScriptInvokerTestCaseHashSet's non-deterministic iteration order.Why the tests fail
HelpSupportTestCasecallsSynopsisGenerator, which uses aHashSetas its data structure for the conflicts object. As per Java 11 documentation, the ordering ofHashSetis not constant. So, the test may fail as the given order can be different from the expected order.BootScriptInvokerTestCase#testuses aHashSetas its data structure for the echos object. As per Java 11 documentation, the ordering ofHashSetis not constant. So, the test may fail as the given order can be different from the expected order.Reproduce the test failure
Run the tests with
NonDexmaven plugin. The commands to recreate the flaky test failures are:mvn -pl cli edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.jboss.as.cli.impl.aesh.HelpSupportTestCase#testStandalonemvn -pl cli edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.jboss.as.cli.impl.BootScriptInvoker#testFixing the flaky test now may prevent flaky test failures in future Java versions.
Expected results
NonDex.Actual Result
We get the following failure for test
org.jboss.as.cli.impl.aesh.HelpSupportTestCase#testStandalone:[ERROR]
testStandalone(org.jboss.as.cli.impl.aesh.HelpSupportTestCase)Time elapsed: 0.254 s <<< FAILURE!org.junit.ComparisonFailure:org.jboss.as.cli.impl.aesh.Commands$Standalone$Command10. EXPECTED[command1 [<argument>] ( [--all-server-groups] | [--replace] | [--server-groups] )]. FOUND[command1 [<argument>] ( [--all-server-groups] | [--server-groups] | [--replace] )]expected:<...server-groups] | [--[replace] | [--server-groups]] )>but was:<...server-groups] | [--[server-groups] | [--replace]] )>We get the following failure for test
org.jboss.as.cli.impl.BootScriptInvokerTestCase#test:<<< FAILURE!- inorg.jboss.as.cli.impl.BootScriptInvokerTestCase[ERROR]test(org.jboss.as.cli.impl.BootScriptInvokerTestCase)Time elapsed: 0.066 s<<< FAILURE!java.lang.AssertionErrorFix
conflicts2fromHashSettoTreeSetin methodorg.jboss.as.cli.impl.aesh.SynopsisGenerator#addSynopsisOption(SynopsisOption).HashSettoLinkedHashSetinorg.jboss.as.cli.impl.BootScriptInvokerTestCase#testJira Issue:
https://issues.redhat.com/browse/WFCORE-6214
This PR is similar to some merged PRs in other Wildfly projects:
wildfly/wildfly#13728
wildfly/wildfly#11833
wildfly/jboss-ejb-client#534