knowm / XChange

XChange is a Java library providing a streamlined API for interacting with 60+ Bitcoin and Altcoin exchanges providing a consistent interface for trading and accessing market data.
http://knowm.org/open-source/xchange/
MIT License
3.82k stars 1.93k forks source link

Fix flaky test in kraken module #4788

Closed rRajivramachandran closed 7 months ago

rRajivramachandran commented 8 months ago

This PR fixes the non deterministic behaviour of a test org.knowm.xchange.kraken.KrakenUtilsTest#testAdaptTradeHistoryByCurrencyPair. This happens as the test assumes the order of keys in a hashmap, which is not guaranteed. It has been detected using NonDex tool.

Reproduction of issue

  1. mvn install -pl xchange-kraken
  2. mvn -pl xchange-kraken edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.knowm.xchange.kraken.KrakenUtilsTest#testAdaptTradeHistoryByCurrencyPair
[INFO] Running org.knowm.xchange.kraken.KrakenUtilsTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.452 s <<< FAILURE! -- in org.knowm.xchange.kraken.KrakenUtilsTest
[ERROR] org.knowm.xchange.kraken.KrakenUtilsTest.testAdaptTradeHistoryByCurrencyPair -- Time elapsed: 0.431 s <<< FAILURE!
org.opentest4j.AssertionFailedError: 

expected: "TY5BYV-WJUQF-XPYEYD-2"
 but was: "TY5BYV-WJUQF-XPYEYD-3"
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at org.knowm.xchange.kraken.KrakenUtilsTest.testAdaptTradeHistoryByCurrencyPair(KrakenUtilsTest.java:74)

Root cause

The test class org.knowm.xchange.kraken.KrakenUtilsTest calls adaptTradesHistory which populates data into the list based on the order of values in the hashmap passed to it Kraken Adapters population before sorting it based on timestamp. Two entries which pass the filter for this test have the same timestamp and hence their order can be non deterministic. The test assumes the first element to be the one which appears first in the data file which may not be the case.

Fix

The filtered list is sorted based on ID to ensure the result order is consistent. No source code has been touched.

Verification of fix