From 074a4ef65b73512b4589ed5700570578acbedb04 Mon Sep 17 00:00:00 2001 From: Gerald Lonlas Date: Sun, 21 Jan 2018 13:51:11 -0800 Subject: [PATCH 1/7] Change CryptoToFiatConverter._is_supported_fiat to Static method We do not need to load the whole object to know if a pair is in a static list. Unittest passed from 1.25s to 0.08s --- freqtrade/fiat_convert.py | 8 ++++---- freqtrade/tests/test_fiat_convert.py | 11 +++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/freqtrade/fiat_convert.py b/freqtrade/fiat_convert.py index 0132e531d..6c5383860 100644 --- a/freqtrade/fiat_convert.py +++ b/freqtrade/fiat_convert.py @@ -87,7 +87,7 @@ class CryptoToFiatConverter(): fiat_symbol = fiat_symbol.upper() # Check if the fiat convertion you want is supported - if not self._is_supported_fiat(fiat=fiat_symbol): + if not CryptoToFiatConverter.is_supported_fiat(fiat=fiat_symbol): raise ValueError('The fiat {} is not supported.'.format(fiat_symbol)) # Get the pair that interest us and return the price in fiat @@ -131,7 +131,7 @@ class CryptoToFiatConverter(): return price - def _is_supported_fiat(self, fiat: str) -> bool: + def is_supported_fiat(fiat: str) -> bool: """ Check if the FIAT your want to convert to is supported :param fiat: FIAT to check (e.g USD) @@ -140,7 +140,7 @@ class CryptoToFiatConverter(): fiat = fiat.upper() - return fiat in self.SUPPORTED_FIAT + return fiat in CryptoToFiatConverter.SUPPORTED_FIAT def _find_price(self, crypto_symbol: str, fiat_symbol: str) -> float: """ @@ -150,7 +150,7 @@ class CryptoToFiatConverter(): :return: float, price of the crypto-currency in Fiat """ # Check if the fiat convertion you want is supported - if not self._is_supported_fiat(fiat=fiat_symbol): + if not CryptoToFiatConverter.is_supported_fiat(fiat=fiat_symbol): raise ValueError('The fiat {} is not supported.'.format(fiat_symbol)) try: return float( diff --git a/freqtrade/tests/test_fiat_convert.py b/freqtrade/tests/test_fiat_convert.py index ddc1c8e29..48b305438 100644 --- a/freqtrade/tests/test_fiat_convert.py +++ b/freqtrade/tests/test_fiat_convert.py @@ -36,12 +36,11 @@ def test_pair_convertion_object(): assert pair_convertion.price == 30000.123 -def test_fiat_convert_is_supported(): - fiat_convert = CryptoToFiatConverter() - assert fiat_convert._is_supported_fiat(fiat='USD') is True - assert fiat_convert._is_supported_fiat(fiat='usd') is True - assert fiat_convert._is_supported_fiat(fiat='abc') is False - assert fiat_convert._is_supported_fiat(fiat='ABC') is False +def test_fiat_convert_is_supported(mocker): + assert CryptoToFiatConverter.is_supported_fiat(fiat='USD') is True + assert CryptoToFiatConverter.is_supported_fiat(fiat='usd') is True + assert CryptoToFiatConverter.is_supported_fiat(fiat='abc') is False + assert CryptoToFiatConverter.is_supported_fiat(fiat='ABC') is False def test_fiat_convert_add_pair(): From 6899b331304479b06c878546978aff25a853ed7b Mon Sep 17 00:00:00 2001 From: Gerald Lonlas Date: Sun, 21 Jan 2018 14:00:05 -0800 Subject: [PATCH 2/7] Speed up unit test test_fiat_convert_get_price and test_fiat_convert_find_price Change the mock of the unit test_fiat_convert_get_price Duration get optimized: - test_fiat_convert_get_price from 1.83s to 0.08s - test_fiat_convert_find_price from 1.72s to 0.08s --- freqtrade/tests/test_fiat_convert.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/freqtrade/tests/test_fiat_convert.py b/freqtrade/tests/test_fiat_convert.py index 48b305438..3e3e82ae9 100644 --- a/freqtrade/tests/test_fiat_convert.py +++ b/freqtrade/tests/test_fiat_convert.py @@ -63,10 +63,12 @@ def test_fiat_convert_add_pair(): def test_fiat_convert_find_price(mocker): api_mock = MagicMock(return_value={ - 'price_usd': 12345.0, + 'ticker': MagicMock(return_value={ + 'price_usd': 12345.0, 'price_eur': 13000.2 + }) }) - mocker.patch('freqtrade.fiat_convert.Pymarketcap.ticker', api_mock) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', api_mock) fiat_convert = CryptoToFiatConverter() with pytest.raises(ValueError, match=r'The fiat ABC is not supported.'): @@ -82,10 +84,12 @@ def test_fiat_convert_find_price(mocker): def test_fiat_convert_get_price(mocker): api_mock = MagicMock(return_value={ - 'price_usd': 28000.0, - 'price_eur': 15000.0 + 'ticker': MagicMock(return_value={ + 'price_usd': 28000.0, + 'price_eur': 15000.0 + }) }) - mocker.patch('freqtrade.fiat_convert.Pymarketcap.ticker', api_mock) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', api_mock) mocker.patch('freqtrade.fiat_convert.CryptoToFiatConverter._find_price', return_value=28000.0) fiat_convert = CryptoToFiatConverter() From 31f1fed0818021b28d21985e0544da88c7e48c7c Mon Sep 17 00:00:00 2001 From: Gerald Lonlas Date: Sun, 21 Jan 2018 14:06:33 -0800 Subject: [PATCH 3/7] Speed up unit test test_fiat_convert_add_pair Duration optimized from 1.85s to 0.08s --- freqtrade/tests/test_fiat_convert.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/freqtrade/tests/test_fiat_convert.py b/freqtrade/tests/test_fiat_convert.py index 3e3e82ae9..11362bc2a 100644 --- a/freqtrade/tests/test_fiat_convert.py +++ b/freqtrade/tests/test_fiat_convert.py @@ -43,7 +43,9 @@ def test_fiat_convert_is_supported(mocker): assert CryptoToFiatConverter.is_supported_fiat(fiat='ABC') is False -def test_fiat_convert_add_pair(): +def test_fiat_convert_add_pair(mocker): + api_mock = MagicMock(return_value={}) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', api_mock) fiat_convert = CryptoToFiatConverter() assert len(fiat_convert._pairs) == 0 From de3586d57f0705411e0e87295b7595da35a6ef95 Mon Sep 17 00:00:00 2001 From: Gerald Lonlas Date: Sun, 21 Jan 2018 14:40:31 -0800 Subject: [PATCH 4/7] Move fiat_converter.convert_amount logic at the end of create_trade() It is more important to save the trade in the DB than failing because of the currency convertion. We do not know if Pymarketcap() will not fail one day. Make sure the trade is saved before. --- freqtrade/main.py | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/freqtrade/main.py b/freqtrade/main.py index 27f3dfd9a..bcc0d33df 100755 --- a/freqtrade/main.py +++ b/freqtrade/main.py @@ -328,13 +328,26 @@ def create_trade(stake_amount: float, interval: int) -> bool: break else: return False - # Calculate amount buy_limit = get_target_bid(exchange.get_ticker(pair)) amount = stake_amount / buy_limit order_id = exchange.buy(pair, buy_limit, amount) + # Fee is applied twice because we make a LIMIT_BUY and LIMIT_SELL + trade = Trade( + pair=pair, + stake_amount=stake_amount, + amount=amount, + fee=exchange.get_fee(), + open_rate=buy_limit, + open_date=datetime.utcnow(), + exchange=exchange.get_name().upper(), + open_order_id=order_id + ) + Trade.session.add(trade) + Trade.session.flush() + fiat_converter = CryptoToFiatConverter() stake_amount_fiat = fiat_converter.convert_amount( stake_amount, @@ -350,19 +363,7 @@ def create_trade(stake_amount: float, interval: int) -> bool: buy_limit, stake_amount, _CONF['stake_currency'], stake_amount_fiat, _CONF['fiat_display_currency'] )) - # Fee is applied twice because we make a LIMIT_BUY and LIMIT_SELL - trade = Trade( - pair=pair, - stake_amount=stake_amount, - amount=amount, - fee=exchange.get_fee(), - open_rate=buy_limit, - open_date=datetime.utcnow(), - exchange=exchange.get_name().upper(), - open_order_id=order_id - ) - Trade.session.add(trade) - Trade.session.flush() + return True From 8d7a7b95e2c4b171f86d3bc48bebb33a0e2ccd9b Mon Sep 17 00:00:00 2001 From: Gerald Lonlas Date: Sun, 21 Jan 2018 14:58:42 -0800 Subject: [PATCH 5/7] Mock freqtrade.fiat_convert.Pymarketcap to speedup tests --- freqtrade/tests/test_main.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/freqtrade/tests/test_main.py b/freqtrade/tests/test_main.py index a61342480..16d0d6e25 100644 --- a/freqtrade/tests/test_main.py +++ b/freqtrade/tests/test_main.py @@ -58,6 +58,7 @@ def test_process_trade_creation(default_conf, ticker, limit_buy_order, health, m get_wallet_health=health, buy=MagicMock(return_value='mocked_limit_buy'), get_order=MagicMock(return_value=limit_buy_order)) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) trades = Trade.query.filter(Trade.is_open.is_(True)).all() @@ -123,6 +124,7 @@ def test_process_trade_handling(default_conf, ticker, limit_buy_order, health, m get_wallet_health=health, buy=MagicMock(return_value='mocked_limit_buy'), get_order=MagicMock(return_value=limit_buy_order)) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) trades = Trade.query.filter(Trade.is_open.is_(True)).all() @@ -144,6 +146,8 @@ def test_create_trade(default_conf, ticker, limit_buy_order, mocker): validate_pairs=MagicMock(), get_ticker=ticker, buy=MagicMock(return_value='mocked_limit_buy')) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) + # Save state of current whitelist whitelist = copy.deepcopy(default_conf['exchange']['pair_whitelist']) @@ -165,7 +169,6 @@ def test_create_trade(default_conf, ticker, limit_buy_order, mocker): assert whitelist == default_conf['exchange']['pair_whitelist'] - def test_create_trade_minimal_amount(default_conf, ticker, mocker): mocker.patch.dict('freqtrade.main._CONF', default_conf) mocker.patch.multiple('freqtrade.rpc', init=MagicMock(), send_msg=MagicMock()) @@ -176,6 +179,7 @@ def test_create_trade_minimal_amount(default_conf, ticker, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) min_stake_amount = 0.0005 create_trade(min_stake_amount, int(default_conf['ticker_interval'])) @@ -278,6 +282,7 @@ def test_handle_overlpapping_signals(default_conf, ticker, mocker, caplog): get_ticker=ticker, buy=MagicMock(return_value='mocked_limit_buy')) mocker.patch('freqtrade.main.min_roi_reached', return_value=False) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) create_trade(0.001, int(default_conf['ticker_interval'])) @@ -324,6 +329,7 @@ def test_handle_trade_roi(default_conf, ticker, mocker, caplog): get_ticker=ticker, buy=MagicMock(return_value='mocked_limit_buy')) mocker.patch('freqtrade.main.min_roi_reached', return_value=True) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) create_trade(0.001, int(default_conf['ticker_interval'])) @@ -356,6 +362,7 @@ def test_handle_trade_experimental(default_conf, ticker, mocker, caplog): get_ticker=ticker, buy=MagicMock(return_value='mocked_limit_buy')) mocker.patch('freqtrade.main.min_roi_reached', return_value=False) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) create_trade(0.001, int(default_conf['ticker_interval'])) @@ -380,6 +387,7 @@ def test_close_trade(default_conf, ticker, limit_buy_order, limit_sell_order, mo validate_pairs=MagicMock(), get_ticker=ticker, buy=MagicMock(return_value='mocked_limit_buy')) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) # Create trade and sell it init(default_conf, create_engine('sqlite://')) @@ -595,6 +603,7 @@ def test_execute_sell_without_conf_sell_down(default_conf, ticker, ticker_sell_d mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) # Create some test data @@ -627,6 +636,7 @@ def test_execute_sell_without_conf_sell_up(default_conf, ticker, ticker_sell_up, mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) # Create some test data @@ -668,6 +678,7 @@ def test_sell_profit_only_enable_profit(default_conf, limit_buy_order, mocker): 'last': 0.00002172 }), buy=MagicMock(return_value='mocked_limit_buy')) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) create_trade(0.001, int(default_conf['ticker_interval'])) @@ -696,6 +707,7 @@ def test_sell_profit_only_disable_profit(default_conf, limit_buy_order, mocker): 'last': 0.00002172 }), buy=MagicMock(return_value='mocked_limit_buy')) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) create_trade(0.001, int(default_conf['ticker_interval'])) @@ -724,6 +736,7 @@ def test_sell_profit_only_enable_loss(default_conf, limit_buy_order, mocker): 'last': 0.00000172 }), buy=MagicMock(return_value='mocked_limit_buy')) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) create_trade(0.001, int(default_conf['ticker_interval'])) @@ -752,6 +765,7 @@ def test_sell_profit_only_disable_loss(default_conf, limit_buy_order, mocker): 'last': 0.00000172 }), buy=MagicMock(return_value='mocked_limit_buy')) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) create_trade(0.001, int(default_conf['ticker_interval'])) From b192c23574f98ba29e0e283658eaa16c8b248575 Mon Sep 17 00:00:00 2001 From: Gerald Lonlas Date: Sun, 21 Jan 2018 15:10:54 -0800 Subject: [PATCH 6/7] Mock freqtrade.fiat_convert.Pymarketcap to speedup Telegram tests --- freqtrade/tests/rpc/test_rpc_telegram.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/freqtrade/tests/rpc/test_rpc_telegram.py b/freqtrade/tests/rpc/test_rpc_telegram.py index 9c99be342..065a038ec 100644 --- a/freqtrade/tests/rpc/test_rpc_telegram.py +++ b/freqtrade/tests/rpc/test_rpc_telegram.py @@ -87,6 +87,7 @@ def test_status_handle(default_conf, update, ticker, mocker): mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) update_state(State.STOPPED) @@ -124,6 +125,7 @@ def test_status_table_handle(default_conf, update, ticker, mocker): validate_pairs=MagicMock(), get_ticker=ticker, buy=MagicMock(return_value='mocked_order_id')) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) update_state(State.STOPPED) _status_table(bot=MagicMock(), update=update) @@ -386,6 +388,7 @@ def test_performance_handle( mocker.patch.multiple('freqtrade.main.exchange', validate_pairs=MagicMock(), get_ticker=ticker) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) # Create some test data @@ -493,6 +496,7 @@ def test_count_handle(default_conf, update, ticker, mocker): validate_pairs=MagicMock(), get_ticker=ticker, buy=MagicMock(return_value='mocked_order_id')) + mocker.patch('freqtrade.fiat_convert.Pymarketcap', MagicMock()) init(default_conf, create_engine('sqlite://')) update_state(State.STOPPED) _count(bot=MagicMock(), update=update) From 8c9fc76957bbce2845b5ba356ae8df335042dc92 Mon Sep 17 00:00:00 2001 From: Gerald Lonlas Date: Sun, 21 Jan 2018 15:19:52 -0800 Subject: [PATCH 7/7] Of course Flake8, I forgot you --- freqtrade/tests/test_fiat_convert.py | 2 +- freqtrade/tests/test_main.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/freqtrade/tests/test_fiat_convert.py b/freqtrade/tests/test_fiat_convert.py index 11362bc2a..9de624964 100644 --- a/freqtrade/tests/test_fiat_convert.py +++ b/freqtrade/tests/test_fiat_convert.py @@ -67,7 +67,7 @@ def test_fiat_convert_find_price(mocker): api_mock = MagicMock(return_value={ 'ticker': MagicMock(return_value={ 'price_usd': 12345.0, - 'price_eur': 13000.2 + 'price_eur': 13000.2 }) }) mocker.patch('freqtrade.fiat_convert.Pymarketcap', api_mock) diff --git a/freqtrade/tests/test_main.py b/freqtrade/tests/test_main.py index 16d0d6e25..2bd8da075 100644 --- a/freqtrade/tests/test_main.py +++ b/freqtrade/tests/test_main.py @@ -169,6 +169,7 @@ def test_create_trade(default_conf, ticker, limit_buy_order, mocker): assert whitelist == default_conf['exchange']['pair_whitelist'] + def test_create_trade_minimal_amount(default_conf, ticker, mocker): mocker.patch.dict('freqtrade.main._CONF', default_conf) mocker.patch.multiple('freqtrade.rpc', init=MagicMock(), send_msg=MagicMock())