rRajivramachandran opened a new pull request, #8121: URL: https://github.com/apache/cloudstack/pull/8121
This PR is to fix a few tests whose results are non deterministic/flaky in the module plugins/network-elements/nicira-nvp. ### Setup: Java version: 11.0.20.1 Maven version: 3.8.8 ### Test failure reproduction: The test `com.cloud.network.nicira.NiciraRestClientTest#testExecuteLiveLockWhenControllerAllowsLoginAndFollowsWithUnauthorizedButDoesNotRediect` assumes the order of keys to be maintained by a `HashMap`. This issue was verified using the [NonDex plugin](https://github.com/TestingResearchIllinois/NonDex). #### Steps: ``` git clone https://github.com/apache/cloudstack.git cd cloudstack mvn install -pl plugins/network-elements/nicira-nvp -am -DskipTests mvn -pl plugins/network-elements/nicira-nvp edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=com.cloud.network.nicira.NiciraRestClientTest#testExecuteLiveLockWhenControllerAllowsLoginAndFollowsWithUnauthorizedButDoesNotRediect ``` ### Root cause and fix: Test failure in NonDex mode: ``` [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.767 s <<< FAILURE! - in com.cloud.network.nicira.NiciraRestClientTest [ERROR] testExecuteLiveLockWhenControllerAllowsLoginAndFollowsWithUnauthorizedButDoesNotRediect(com.cloud.network.nicira.NiciraRestClientTest) Time elapsed: 1.761 s <<< ERROR! java.lang.NullPointerException ``` The NullPointerException occurs at the following location https://github.com/apache/cloudstack/blob/fcbf540369d5d51145573dc948e21930531a3fad/plugins/network-elements/nicira-nvp/src/main/java/com/cloud/network/nicira/NiciraRestClient.java#L87 because the `mockResponse` created in the test, does not get returned for the `execute` method. To verify that the issue was due to the HttpRequestMatcher used by mockito, the following line: https://github.com/apache/cloudstack/blob/fcbf540369d5d51145573dc948e21930531a3fad/plugins/network-elements/nicira-nvp/src/test/java/com/cloud/network/nicira/NiciraRestClientTest.java#L190 was modfied to read `when(httpClient.execute(eq(HTTP_HOST), any(HttpUriRequest.class), eq(httpClientContext)))`. This allows matching any request class object. The test passed with this change when run on the NonDex tool confirming that the issue was here. On adding logs to the `HttpRequestMatcher` class, it was deduced that the test fails when the order of keys returned by the login parameters is not the same as that configured in the test class. Failing case(ordered shuffled): ``` 2023-10-19 18:11:15,312 ERROR [root] (main:) Wanted payload:password=adminpassword&username=admin 2023-10-19 18:11:15,312 ERROR [root] (main:) Actual payload:username=admin&password=adminpassword [ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.423 s <<< FAILURE! - in com.cloud.network.nicira.NiciraRestClientTest [ERROR] testExecuteLiveLockWhenControllerAllowsLoginAndFollowsWithUnauthorizedButDoesNotRediect(com.cloud.network.nicira.NiciraRestClientTest) Time elapsed: 1.413 s <<< ERROR! java.lang.NullPointerException ``` Passing case(order not shuffled): ``` 2023-10-19 18:11:09,513 ERROR [root] (main:) Wanted payload:username=admin&password=adminpassword 2023-10-19 18:11:09,517 ERROR [root] (main:) Actual payload:username=admin&password=adminpassword [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.968 s - in com.cloud.network.nicira.NiciraRestClientTest ``` The test class configures login parameters here: https://github.com/apache/cloudstack/blob/fcbf540369d5d51145573dc948e21930531a3fad/plugins/network-elements/nicira-nvp/src/test/java/com/cloud/network/nicira/NiciraRestClientTest.java#L82-L93 and the source code builds them here: https://github.com/apache/cloudstack/blob/fcbf540369d5d51145573dc948e21930531a3fad/plugins/network-elements/nicira-nvp/src/main/java/com/cloud/network/nicira/NiciraRestClient.java#L122-L131 ### Fix To make the map ordered for ensuring deterministic results during serialization/deserialization, changes were done at the source code and test by converting the `HashMap` to a `LinkedHashMap`. ### Verification of fix - The test passes when run with the NonDex tool. - The entire test suite of the module passes. ### Additional fixes The change also fixed following tests in the same class which were flaky due to the same reason: - com.cloud.network.nicira.NiciraRestClientTest#testExecuteTwoConsecutiveUnauthorizedExecutions - com.cloud.network.nicira.NiciraRestClientTest#testExecuteUnauthorizedThenSuccess -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
