Fix test flakiness due to getDeclaredFields() without adding dependencies

[kaiyaok2]

Description
45 flaky tests are found using Nondex when running commands, including all tests presented in the earlier PR #654. This PR now fixes all of them without introducing new plugins / dependencies.
The flaky tests can be found when running the following command:
mvn edu.illinois:nondex-maven-plugin:1.1.2:nondex -Dtest={test}
For example, replacing {test} by CheckoutTest checks for all flaky tests in CheckoutTest.java.
All 45 flaky tests are related to JSON-format strings. 44 of them are representing JSONObject-likes and 1 of them is representing JSONArray-like. Direct comparisons between JSON-formatted strings may occasionally fail upon changes of environment (e.g. using future versions of Java), as element orders in JSONObjects/JSONArrays are not preserved. This issue is due to possible order permutation in the routine getDeclaredFields().
Proposed Fixes
Changes to specific test files fall into the following 3 categories:

• For assertions that include raw comparison of JSON-format strings: call my implemented helper function assertJsonStringEquals() instead. The function makes direct comparisons after parsing input strings with Gson's JsonParser() method, allowing permutation of element orders in JSONs.
• For tests that include jsonRequest.contains(some_JSON_string), add all possible permutation patterns of some_JSON_string
• For the test that casts JSONObject-like strings to base64 before comparison: We'll decode the base64-formatted JSON strings to SaleToAcquirerData objects and then do a direct comparison.

[lgtm-com]

This pull request introduces 1 alert when merging a5f2faa into 8aa7dbd - view on LGTM.com

new alerts:

• 1 for Spurious Javadoc @param tags

[kaiyaok2]

Hi there, I've resolved the alert in the comments.

[michaelpaul]

Hi @kaiyaok2, thanks for submitting your contribution! I've just noticed an issue with the new method com.adyen.util.Util#jsonObjectStringToTreeMap.

The format produced by it can only be used in the tests.

[michaelpaul]

Hi @kaiyaok2, the SaleToAcquirerData is an important class on the library that we don't want to change to avoid breaking existing merchant applications. The new changes hide possible exceptions from the application and exposing new exceptions would lead to a breaking change in the API.

Feel free to change the test code of the library.

[kaiyaok2]

Hi @kaiyaok2, the SaleToAcquirerData is an important class on the library that we don't want to change to avoid breaking existing merchant applications. The new changes hide possible exceptions from the application and exposing new exceptions would lead to a breaking change in the API.

Feel free to change the test code of the library.

Hi there, thanks for your message! I updated a commit to remove possible exceptions by using Gson instead, and hopefully this will not break the API.

Other 44 of the 45 flaky tests can be done solely with modifications to test files only. However, for base64 comparison, I believe that the test flakiness cannot be resolved(without hardcoding) unless we force toBase64() to work with order-deterministic inputs.

[michaelpaul]

Checkout this another idea to deal with the base64 encoded JSON strings. In test code we can tradeoff some performance in favor of determinism.

[kaiyaok2]

Checkout this another idea to deal with the base64 encoded JSON strings. In test code we can tradeoff some performance in favor of determinism.

I just updated with a commit with all proposed changes done!

Also, the other flaky tests in the repo are similar to ones in testGsonAndJacksonSerializeNotificationRequest() and testPaymentsRequestWithXidAndCavv(), and can be done smoothly with some inline changes in test files. Shall I do another commit fixing all those?

[michaelpaul]

I just updated with a commit with all proposed changes done!

Also, the other flaky tests in the repo are similar to ones in testGsonAndJacksonSerializeNotificationRequest() and testPaymentsRequestWithXidAndCavv(), and can be done smoothly with some inline changes in test files. Shall I do another commit fixing all those?

Nice! You can update those as well if you like. :)

[kaiyaok2]

I just updated with a commit with all proposed changes done!
Also, the other flaky tests in the repo are similar to ones in testGsonAndJacksonSerializeNotificationRequest() and testPaymentsRequestWithXidAndCavv(), and can be done smoothly with some inline changes in test files. Shall I do another commit fixing all those?

Nice! You can update those as well if you like. :)

The latest commit updates all the other tests 👍

[kaiyaok2]

Hi @michaelpaul could you please check the newest version?

[kaiyaok2]

Hi @AlexandrosMor @michaelpaul, What are the checks for coverage/coveralls? How can I ensure that these checks pass?

[michaelpaul]

Hi @AlexandrosMor @michaelpaul, What are the checks for coverage/coveralls? How can I ensure that these checks pass?

Hi @kaiyaok2, the coveralls process runs unit tests with code coverage. I believe we need to make some adjustments for this check. If the tests are passing on your machine, then you don't have anything else to do.

[kaiyaok2]

Hi @AlexandrosMor @michaelpaul, What are the checks for coverage/coveralls? How can I ensure that these checks pass?

Hi @kaiyaok2, the coveralls process runs unit tests with code coverage. I believe we need to make some adjustments for this check. If the tests are passing on your machine, then you don't have anything else to do.

Got it, I had all tests passing on my machine. Thanks for clarifying!