From 0cb57bee0e1eb20dc28bc39ddd4d23c4d5c6dc38 Mon Sep 17 00:00:00 2001 From: kryofly Date: Wed, 10 Jan 2018 10:29:04 +0100 Subject: [PATCH] small refactor of check_handle_timedout --- freqtrade/main.py | 82 ++++++++++++++++++++++++------------ freqtrade/tests/test_main.py | 46 +++++++++++++++++++- 2 files changed, 100 insertions(+), 28 deletions(-) diff --git a/freqtrade/main.py b/freqtrade/main.py index 4e69e572d..51f87c82d 100755 --- a/freqtrade/main.py +++ b/freqtrade/main.py @@ -158,6 +158,53 @@ def _process(nb_assets: Optional[int] = 0) -> bool: return state_changed +# FIX: 20180110, why is cancel.order unconditionally here, whereas +# it is conditionally called in the +# handle_timedout_limit_sell()? +def handle_timedout_limit_buy(trade, order): + """Buy timeout - cancel order + :return: True if order was fully cancelled + """ + exchange.cancel_order(trade.open_order_id) + if order['remaining'] == order['amount']: + # if trade is not partially completed, just delete the trade + Trade.session.delete(trade) + # FIX? do we really need to flush, caller of + # check_handle_timedout will flush afterwards + Trade.session.flush() + logger.info('Buy order timeout for %s.', trade) + return True + else: + # if trade is partially complete, edit the stake details for the trade + # and close the order + trade.amount = order['amount'] - order['remaining'] + trade.stake_amount = trade.amount * trade.open_rate + trade.open_order_id = None + logger.info('Partial buy order timeout for %s.', trade) + return False + + +# FIX: 20180110, should cancel_order() be cond. or unconditionally called? +def handle_timedout_limit_sell(trade, order): + """ + Sell timeout - cancel order and update trade + :return: True if order was fully cancelled + """ + if order['remaining'] == order['amount']: + # if trade is not partially completed, just cancel the trade + exchange.cancel_order(trade.open_order_id) + trade.close_rate = None + trade.close_profit = None + trade.close_date = None + trade.is_open = True + trade.open_order_id = None + logger.info('Sell order timeout for %s.', trade) + return True + else: + # TODO: figure out how to handle partially complete sell orders + return False + + def check_handle_timedout(timeoutvalue: int) -> None: """ Check if any orders are timed out and cancel if neccessary @@ -171,35 +218,16 @@ def check_handle_timedout(timeoutvalue: int) -> None: ordertime = arrow.get(order['opened']) if order['type'] == "LIMIT_BUY" and ordertime < timeoutthreashold: - # Buy timeout - cancel order - exchange.cancel_order(trade.open_order_id) - if order['remaining'] == order['amount']: - # if trade is not partially completed, just delete the trade - Trade.session.delete(trade) - Trade.session.flush() - logger.info('Buy order timeout for %s.', trade) - else: - # if trade is partially complete, edit the stake details for the trade - # and close the order - trade.amount = order['amount'] - order['remaining'] - trade.stake_amount = trade.amount * trade.open_rate - trade.open_order_id = None - logger.info('Partial buy order timeout for %s.', trade) + handle_timedout_limit_buy(trade, order) + elif order['type'] == "LIMIT_SELL" and ordertime < timeoutthreashold: - # Sell timeout - cancel order and update trade - if order['remaining'] == order['amount']: - # if trade is not partially completed, just cancel the trade - exchange.cancel_order(trade.open_order_id) - trade.close_rate = None - trade.close_profit = None - trade.close_date = None - trade.is_open = True - trade.open_order_id = None - logger.info('Sell order timeout for %s.', trade) + if handle_timedout_limit_sell(trade, order): + # BUG? if there is more trades that are + # timed out, shouldn't we collect and + # then return all of them? + # Also the function signature is return None. + # But we return True here. return True - else: - # TODO: figure out how to handle partially complete sell orders - pass def execute_sell(trade: Trade, limit: float) -> None: diff --git a/freqtrade/tests/test_main.py b/freqtrade/tests/test_main.py index 110c40862..fc0053f78 100644 --- a/freqtrade/tests/test_main.py +++ b/freqtrade/tests/test_main.py @@ -18,11 +18,11 @@ from freqtrade.misc import get_state, State from freqtrade.persistence import Trade import freqtrade.main as main - # Test that main() can start backtesting or hyperopt. # and also ensure we can pass some specific arguments # argument parsing is done in test_misc.py + def test_parse_args_backtesting(mocker): backtesting_mock = mocker.patch( 'freqtrade.optimize.backtesting.start', MagicMock()) @@ -261,6 +261,20 @@ def test_create_trade_no_pairs_after_blacklist(default_conf, ticker, mocker): create_trade(default_conf['stake_amount']) +def test_create_trade_no_signal(default_conf, ticker, mocker): + default_conf['dry_run'] = True + mocker.patch.dict('freqtrade.main._CONF', default_conf) + mocker.patch('freqtrade.main.get_signal', MagicMock(return_value=False)) + mocker.patch.multiple('freqtrade.exchange', + get_ticker_history=MagicMock(return_value=20)) + mocker.patch.multiple('freqtrade.main.exchange', + get_balance=MagicMock(return_value=20)) + stake_amount = 10 + Trade.query = MagicMock() + Trade.query.filter = MagicMock() + assert not create_trade(stake_amount) + + def test_handle_trade(default_conf, limit_buy_order, limit_sell_order, mocker): mocker.patch.dict('freqtrade.main._CONF', default_conf) mocker.patch('freqtrade.main.get_signal', side_effect=lambda s, t: True) @@ -414,6 +428,21 @@ def test_check_handle_timedout_buy(default_conf, ticker, health, limit_buy_order assert len(trades) == 0 +def test_handle_timedout_limit_buy(default_conf, mocker): + cancel_order = MagicMock() + mocker.patch('freqtrade.exchange.cancel_order', cancel_order) + Trade.session = MagicMock() + trade = MagicMock() + order = {} + order['remaining'] = 1 + order['amount'] = 1 + assert main.handle_timedout_limit_buy(trade, order) + assert cancel_order.call_count == 1 + order['amount'] = 2 + assert not main.handle_timedout_limit_buy(trade, order) + assert cancel_order.call_count == 2 + + def test_check_handle_timedout_sell(default_conf, ticker, health, limit_sell_order_old, mocker): mocker.patch.dict('freqtrade.main._CONF', default_conf) cancel_order_mock = MagicMock() @@ -446,6 +475,21 @@ def test_check_handle_timedout_sell(default_conf, ticker, health, limit_sell_ord assert trade_sell.is_open is True +def test_handle_timedout_limit_sell(default_conf, mocker): + cancel_order = MagicMock() + mocker.patch('freqtrade.exchange.cancel_order', cancel_order) + trade = MagicMock() + order = {} + order['remaining'] = 1 + order['amount'] = 1 + assert main.handle_timedout_limit_sell(trade, order) + assert cancel_order.call_count == 1 + order['amount'] = 2 + assert not main.handle_timedout_limit_sell(trade, order) + # Assert cancel_order was not called (callcount remains unchanged) + assert cancel_order.call_count == 1 + + def test_check_handle_timedout_partial(default_conf, ticker, limit_buy_order_old_partial, health, mocker): mocker.patch.dict('freqtrade.main._CONF', default_conf)