From e666c6850e6a0da1bfe078c13d228bb5ee596a69 Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 11 Mar 2019 20:20:30 +0100 Subject: [PATCH 1/4] Fix tests so Market orders should not send timeInForce --- freqtrade/tests/exchange/test_exchange.py | 27 ++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/freqtrade/tests/exchange/test_exchange.py b/freqtrade/tests/exchange/test_exchange.py index fce9cba14..26b5297ea 100644 --- a/freqtrade/tests/exchange/test_exchange.py +++ b/freqtrade/tests/exchange/test_exchange.py @@ -594,8 +594,6 @@ def test_buy_prod(default_conf, mocker, exchange_name): def test_buy_considers_time_in_force(default_conf, mocker): api_mock = MagicMock() order_id = 'test_prod_buy_{}'.format(randint(0, 10 ** 6)) - order_type = 'market' - time_in_force = 'ioc' api_mock.create_order = MagicMock(return_value={ 'id': order_id, 'info': { @@ -607,6 +605,25 @@ def test_buy_considers_time_in_force(default_conf, mocker): mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y) exchange = get_patched_exchange(mocker, default_conf, api_mock) + order_type = 'limit' + time_in_force = 'ioc' + + order = exchange.buy(pair='ETH/BTC', ordertype=order_type, + amount=1, rate=200, time_in_force=time_in_force) + + assert 'id' in order + assert 'info' in order + assert order['id'] == order_id + assert api_mock.create_order.call_args[0][0] == 'ETH/BTC' + assert api_mock.create_order.call_args[0][1] == order_type + assert api_mock.create_order.call_args[0][2] == 'buy' + assert api_mock.create_order.call_args[0][3] == 1 + assert api_mock.create_order.call_args[0][4] == 200 + assert api_mock.create_order.call_args[0][5] == {'timeInForce': 'ioc'} + + order_type = 'market' + time_in_force = 'ioc' + order = exchange.buy(pair='ETH/BTC', ordertype=order_type, amount=1, rate=200, time_in_force=time_in_force) @@ -618,13 +635,13 @@ def test_buy_considers_time_in_force(default_conf, mocker): assert api_mock.create_order.call_args[0][2] == 'buy' assert api_mock.create_order.call_args[0][3] == 1 assert api_mock.create_order.call_args[0][4] is None - assert api_mock.create_order.call_args[0][5] == {'timeInForce': 'ioc'} + assert api_mock.create_order.call_args[0][5] == {} def test_buy_kraken_trading_agreement(default_conf, mocker): api_mock = MagicMock() order_id = 'test_prod_buy_{}'.format(randint(0, 10 ** 6)) - order_type = 'market' + order_type = 'limit' time_in_force = 'ioc' api_mock.create_order = MagicMock(return_value={ 'id': order_id, @@ -648,7 +665,7 @@ def test_buy_kraken_trading_agreement(default_conf, mocker): assert api_mock.create_order.call_args[0][1] == order_type assert api_mock.create_order.call_args[0][2] == 'buy' assert api_mock.create_order.call_args[0][3] == 1 - assert api_mock.create_order.call_args[0][4] is None + assert api_mock.create_order.call_args[0][4] == 200 assert api_mock.create_order.call_args[0][5] == {'timeInForce': 'ioc', 'trading_agreement': 'agree'} From c0f276a8920b8cc1919e1b1206d81b577c583dc0 Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 11 Mar 2019 20:22:51 +0100 Subject: [PATCH 2/4] Move kraken specific tests to their own file --- freqtrade/tests/exchange/test_exchange.py | 68 +---------------------- freqtrade/tests/exchange/test_kraken.py | 67 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 65 deletions(-) create mode 100644 freqtrade/tests/exchange/test_kraken.py diff --git a/freqtrade/tests/exchange/test_exchange.py b/freqtrade/tests/exchange/test_exchange.py index 26b5297ea..d24dd1757 100644 --- a/freqtrade/tests/exchange/test_exchange.py +++ b/freqtrade/tests/exchange/test_exchange.py @@ -4,7 +4,7 @@ import copy import logging from datetime import datetime from random import randint -from unittest.mock import Mock, MagicMock, PropertyMock +from unittest.mock import MagicMock, Mock, PropertyMock import arrow import ccxt @@ -12,11 +12,10 @@ import pytest from pandas import DataFrame from freqtrade import DependencyException, OperationalException, TemporaryError -from freqtrade.exchange import Exchange, Kraken, Binance +from freqtrade.exchange import Binance, Exchange, Kraken from freqtrade.exchange.exchange import API_RETRY_COUNT -from freqtrade.tests.conftest import get_patched_exchange, log_has, log_has_re from freqtrade.resolvers.exchange_resolver import ExchangeResolver - +from freqtrade.tests.conftest import get_patched_exchange, log_has, log_has_re # Make sure to always keep one exchange here which is NOT subclassed!! EXCHANGES = ['bittrex', 'binance', 'kraken', ] @@ -638,67 +637,6 @@ def test_buy_considers_time_in_force(default_conf, mocker): assert api_mock.create_order.call_args[0][5] == {} -def test_buy_kraken_trading_agreement(default_conf, mocker): - api_mock = MagicMock() - order_id = 'test_prod_buy_{}'.format(randint(0, 10 ** 6)) - order_type = 'limit' - time_in_force = 'ioc' - api_mock.create_order = MagicMock(return_value={ - 'id': order_id, - 'info': { - 'foo': 'bar' - } - }) - default_conf['dry_run'] = False - - mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y) - mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y) - exchange = get_patched_exchange(mocker, default_conf, api_mock, id="kraken") - - order = exchange.buy(pair='ETH/BTC', ordertype=order_type, - amount=1, rate=200, time_in_force=time_in_force) - - assert 'id' in order - assert 'info' in order - assert order['id'] == order_id - assert api_mock.create_order.call_args[0][0] == 'ETH/BTC' - assert api_mock.create_order.call_args[0][1] == order_type - assert api_mock.create_order.call_args[0][2] == 'buy' - assert api_mock.create_order.call_args[0][3] == 1 - assert api_mock.create_order.call_args[0][4] == 200 - assert api_mock.create_order.call_args[0][5] == {'timeInForce': 'ioc', - 'trading_agreement': 'agree'} - - -def test_sell_kraken_trading_agreement(default_conf, mocker): - api_mock = MagicMock() - order_id = 'test_prod_sell_{}'.format(randint(0, 10 ** 6)) - order_type = 'market' - api_mock.create_order = MagicMock(return_value={ - 'id': order_id, - 'info': { - 'foo': 'bar' - } - }) - default_conf['dry_run'] = False - - mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y) - mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y) - exchange = get_patched_exchange(mocker, default_conf, api_mock, id="kraken") - - order = exchange.sell(pair='ETH/BTC', ordertype=order_type, amount=1, rate=200) - - assert 'id' in order - assert 'info' in order - assert order['id'] == order_id - assert api_mock.create_order.call_args[0][0] == 'ETH/BTC' - assert api_mock.create_order.call_args[0][1] == order_type - assert api_mock.create_order.call_args[0][2] == 'sell' - assert api_mock.create_order.call_args[0][3] == 1 - assert api_mock.create_order.call_args[0][4] is None - assert api_mock.create_order.call_args[0][5] == {'trading_agreement': 'agree'} - - def test_sell_dry_run(default_conf, mocker): default_conf['dry_run'] = True exchange = get_patched_exchange(mocker, default_conf) diff --git a/freqtrade/tests/exchange/test_kraken.py b/freqtrade/tests/exchange/test_kraken.py new file mode 100644 index 000000000..8b81a08a9 --- /dev/null +++ b/freqtrade/tests/exchange/test_kraken.py @@ -0,0 +1,67 @@ +# pragma pylint: disable=missing-docstring, C0103, bad-continuation, global-statement +# pragma pylint: disable=protected-access +from random import randint +from unittest.mock import MagicMock + +from freqtrade.tests.conftest import get_patched_exchange + + +def test_buy_kraken_trading_agreement(default_conf, mocker): + api_mock = MagicMock() + order_id = 'test_prod_buy_{}'.format(randint(0, 10 ** 6)) + order_type = 'limit' + time_in_force = 'ioc' + api_mock.create_order = MagicMock(return_value={ + 'id': order_id, + 'info': { + 'foo': 'bar' + } + }) + default_conf['dry_run'] = False + + mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y) + mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y) + exchange = get_patched_exchange(mocker, default_conf, api_mock, id="kraken") + + order = exchange.buy(pair='ETH/BTC', ordertype=order_type, + amount=1, rate=200, time_in_force=time_in_force) + + assert 'id' in order + assert 'info' in order + assert order['id'] == order_id + assert api_mock.create_order.call_args[0][0] == 'ETH/BTC' + assert api_mock.create_order.call_args[0][1] == order_type + assert api_mock.create_order.call_args[0][2] == 'buy' + assert api_mock.create_order.call_args[0][3] == 1 + assert api_mock.create_order.call_args[0][4] == 200 + assert api_mock.create_order.call_args[0][5] == {'timeInForce': 'ioc', + 'trading_agreement': 'agree'} + + +def test_sell_kraken_trading_agreement(default_conf, mocker): + api_mock = MagicMock() + order_id = 'test_prod_sell_{}'.format(randint(0, 10 ** 6)) + order_type = 'market' + api_mock.create_order = MagicMock(return_value={ + 'id': order_id, + 'info': { + 'foo': 'bar' + } + }) + default_conf['dry_run'] = False + + mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y) + mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y) + exchange = get_patched_exchange(mocker, default_conf, api_mock, id="kraken") + + order = exchange.sell(pair='ETH/BTC', ordertype=order_type, amount=1, rate=200) + + assert 'id' in order + assert 'info' in order + assert order['id'] == order_id + assert api_mock.create_order.call_args[0][0] == 'ETH/BTC' + assert api_mock.create_order.call_args[0][1] == order_type + assert api_mock.create_order.call_args[0][2] == 'sell' + assert api_mock.create_order.call_args[0][3] == 1 + assert api_mock.create_order.call_args[0][4] is None + assert api_mock.create_order.call_args[0][5] == {'trading_agreement': 'agree'} From 4705b7da0edb6ee405aba89c59dd42a22b6824cb Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 11 Mar 2019 20:30:16 +0100 Subject: [PATCH 3/4] Add time_in_force test for sell --- freqtrade/tests/exchange/test_exchange.py | 60 +++++++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/freqtrade/tests/exchange/test_exchange.py b/freqtrade/tests/exchange/test_exchange.py index d24dd1757..ff36ab91c 100644 --- a/freqtrade/tests/exchange/test_exchange.py +++ b/freqtrade/tests/exchange/test_exchange.py @@ -590,7 +590,8 @@ def test_buy_prod(default_conf, mocker, exchange_name): amount=1, rate=200, time_in_force=time_in_force) -def test_buy_considers_time_in_force(default_conf, mocker): +@pytest.mark.parametrize("exchange_name", EXCHANGES) +def test_buy_considers_time_in_force(default_conf, mocker, exchange_name): api_mock = MagicMock() order_id = 'test_prod_buy_{}'.format(randint(0, 10 ** 6)) api_mock.create_order = MagicMock(return_value={ @@ -602,7 +603,7 @@ def test_buy_considers_time_in_force(default_conf, mocker): default_conf['dry_run'] = False mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y) mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y) - exchange = get_patched_exchange(mocker, default_conf, api_mock) + exchange = get_patched_exchange(mocker, default_conf, api_mock, id=exchange_name) order_type = 'limit' time_in_force = 'ioc' @@ -618,7 +619,8 @@ def test_buy_considers_time_in_force(default_conf, mocker): assert api_mock.create_order.call_args[0][2] == 'buy' assert api_mock.create_order.call_args[0][3] == 1 assert api_mock.create_order.call_args[0][4] == 200 - assert api_mock.create_order.call_args[0][5] == {'timeInForce': 'ioc'} + assert "timeInForce" in api_mock.create_order.call_args[0][5] + assert api_mock.create_order.call_args[0][5]["timeInForce"] == time_in_force order_type = 'market' time_in_force = 'ioc' @@ -634,7 +636,8 @@ def test_buy_considers_time_in_force(default_conf, mocker): assert api_mock.create_order.call_args[0][2] == 'buy' assert api_mock.create_order.call_args[0][3] == 1 assert api_mock.create_order.call_args[0][4] is None - assert api_mock.create_order.call_args[0][5] == {} + # Market orders should not send timeInForce!! + assert "timeInForce" not in api_mock.create_order.call_args[0][5] def test_sell_dry_run(default_conf, mocker): @@ -705,6 +708,55 @@ def test_sell_prod(default_conf, mocker, exchange_name): exchange.sell(pair='ETH/BTC', ordertype=order_type, amount=1, rate=200) +@pytest.mark.parametrize("exchange_name", EXCHANGES) +def test_sell_considers_time_in_force(default_conf, mocker, exchange_name): + api_mock = MagicMock() + order_id = 'test_prod_sell_{}'.format(randint(0, 10 ** 6)) + api_mock.create_order = MagicMock(return_value={ + 'id': order_id, + 'info': { + 'foo': 'bar' + } + }) + default_conf['dry_run'] = False + mocker.patch('freqtrade.exchange.Exchange.symbol_amount_prec', lambda s, x, y: y) + mocker.patch('freqtrade.exchange.Exchange.symbol_price_prec', lambda s, x, y: y) + exchange = get_patched_exchange(mocker, default_conf, api_mock, id=exchange_name) + + order_type = 'limit' + time_in_force = 'ioc' + + order = exchange.sell(pair='ETH/BTC', ordertype=order_type, + amount=1, rate=200, time_in_force=time_in_force) + + assert 'id' in order + assert 'info' in order + assert order['id'] == order_id + assert api_mock.create_order.call_args[0][0] == 'ETH/BTC' + assert api_mock.create_order.call_args[0][1] == order_type + assert api_mock.create_order.call_args[0][2] == 'sell' + assert api_mock.create_order.call_args[0][3] == 1 + assert api_mock.create_order.call_args[0][4] == 200 + assert "timeInForce" in api_mock.create_order.call_args[0][5] + assert api_mock.create_order.call_args[0][5]["timeInForce"] == time_in_force + + order_type = 'market' + time_in_force = 'ioc' + order = exchange.sell(pair='ETH/BTC', ordertype=order_type, + amount=1, rate=200, time_in_force=time_in_force) + + assert 'id' in order + assert 'info' in order + assert order['id'] == order_id + assert api_mock.create_order.call_args[0][0] == 'ETH/BTC' + assert api_mock.create_order.call_args[0][1] == order_type + assert api_mock.create_order.call_args[0][2] == 'sell' + assert api_mock.create_order.call_args[0][3] == 1 + assert api_mock.create_order.call_args[0][4] is None + # Market orders should not send timeInForce!! + assert "timeInForce" not in api_mock.create_order.call_args[0][5] + + def test_get_balance_dry_run(default_conf, mocker): default_conf['dry_run'] = True From 0eb9dd5fe5d8381e2bce26effe66b3309d6ff85a Mon Sep 17 00:00:00 2001 From: Matthias Date: Mon, 11 Mar 2019 20:30:36 +0100 Subject: [PATCH 4/4] Don't use timeInForce for market orders --- freqtrade/exchange/exchange.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/freqtrade/exchange/exchange.py b/freqtrade/exchange/exchange.py index f6fb0a58a..32d952542 100644 --- a/freqtrade/exchange/exchange.py +++ b/freqtrade/exchange/exchange.py @@ -352,7 +352,7 @@ class Exchange(object): return dry_order params = self._params.copy() - if time_in_force != 'gtc': + if time_in_force != 'gtc' and ordertype != 'market': params.update({'timeInForce': time_in_force}) return self.create_order(pair, ordertype, 'buy', amount, rate, params) @@ -365,7 +365,7 @@ class Exchange(object): return dry_order params = self._params.copy() - if time_in_force != 'gtc': + if time_in_force != 'gtc' and ordertype != 'market': params.update({'timeInForce': time_in_force}) return self.create_order(pair, ordertype, 'sell', amount, rate, params)