GoogleCloudPlatform / appengine-plugins

A client Java library to manage App Engine Java applications for any project that performs App Engine Java application management. For example, the Maven, Gradle and Eclipse App Engine plugins, custom user tools, etc.
Apache License 2.0
40 stars 26 forks source link

Fix Flaky Test caused by HashSet Ordering permutations #903

Closed bmk15897 closed 1 year ago

bmk15897 commented 1 year ago

Related to the Issue #902

Description

Flaky Test found using NonDex by running the command - mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -Dtest=com.google.cloud.tools.managedcloudsdk.install.InstallerTest#testCall_withOverrideComponents

The logged failure for the test com.google.cloud.tools.managedcloudsdk.install.InstallerTest.testCall_withOverrideComponents -

[ERROR] Failures: 
[ERROR]   InstallerTest.testCall_withOverrideComponents:115 
Argument(s) are different! Wanted:
mockCommandRunner.run(
    [scriptexec, test-install.script, --path-update=false, --command-completion=false, --quiet, --usage-reporting=true, --override-components, myothercomponent, mycomponent],
    C:\Users\Bharati\AppData\Local\Temp\junit6297198120928949252,
    {"PROPERTY" = "value"},
    mockConsoleListener
);
-> at com.google.cloud.tools.managedcloudsdk.command.CommandRunner.run(CommandRunner.java:49)
Actual invocations have different arguments:
mockCommandRunner.run(
    [scriptexec, test-install.script, --path-update=false, --command-completion=false, --quiet, --usage-reporting=true, --override-components, mycomponent, myothercomponent],
    C:\Users\Bharati\AppData\Local\Temp\junit6297198120928949252,
    {"PROPERTY" = "value"},
    mockConsoleListener
);
-> at com.google.cloud.tools.managedcloudsdk.install.Installer.install(Installer.java:99)

Investigation

The test fails at InstallerTest.testCall_withOverrideComponents:115 with the error regarding a mismatch of arguments while using the overrides variable declared on line 102. The variable is declared using the HashSet constructor. According to the documentation, the HashSet class makes no guarantees as to the iteration order of the set; in particular, it does not guarantee that the order will remain constant over time. This makes the test outcome non-deterministic. To fix this, HashSet needs to be replace with a data structure that can guarantee the order of its elements.

Fix

LinkedHashSet preserves the iteration ordering, which is the order in which elements were inserted into the set. As this data structure is deterministic, replacing the HashSet with the LinkedHashSet removes the non-determinism without needing any further changes and ensures that the flakiness from the test is removed. The PR does not introduce a breaking change.

google-cla[bot] commented 1 year ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.