You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "rRajivramachandran (via GitHub)" <gi...@apache.org> on 2023/10/20 03:43:32 UTC
[PR] Fix flaky tests by using LinkedHashMap [cloudstack]
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix flaky tests by using LinkedHashMap [cloudstack]
Posted by "rRajivramachandran (via GitHub)" <gi...@apache.org>.
rRajivramachandran closed pull request #8121: Fix flaky tests by using LinkedHashMap
URL: https://github.com/apache/cloudstack/pull/8121
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix flaky tests by using LinkedHashMap [cloudstack]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #8121:
URL: https://github.com/apache/cloudstack/pull/8121#issuecomment-1772030427
Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:
- In case of a new feature add useful documentation (raise doc PR at https://github.com/apache/cloudstack-documentation)
- Be patient and persistent. It might take some time to get a review or get the final approval from the committers.
- Pay attention to the quality of your code, ensure tests are passing and your PR doesn't have conflicts.
- Please follow [ASF Code of Conduct](https://github.com/apache/.github/blob/main/.github/CODE_OF_CONDUCT.md) for all communication including (but not limited to) comments on Pull Requests, Issues, Mailing list and Slack.
- Be sure to read the [CloudStack Coding Conventions](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions).
Apache CloudStack is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@cloudstack.apache.org (https://cloudstack.apache.org/mailing-lists.html)
Slack: https://apachecloudstack.slack.com/
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org