Merge pull request #253 from kryofly/sell_signal

execute sell if get_signal OR ROI reached

freqtrade/main.py

@@ -179,17 +179,20 @@ def handle_trade(trade: Trade) -> bool:
     current_rate = exchange.get_ticker(trade.pair)['bid']
 
     # Check if minimal roi has been reached
-    if not min_roi_reached(trade, current_rate, datetime.utcnow()):
-        return False
+    if min_roi_reached(trade, current_rate, datetime.utcnow()):
+        logger.debug('Executing sell due to ROI ...')
+        execute_sell(trade, current_rate)
+        return True
 
     # Check if sell signal has been enabled and triggered
     if _CONF.get('experimental', {}).get('use_sell_signal'):
         logger.debug('Checking sell_signal ...')
-        if not get_signal(trade.pair, SignalType.SELL):
-            return False
+        if get_signal(trade.pair, SignalType.SELL):
+            logger.debug('Executing sell due to sell signal ...')
+            execute_sell(trade, current_rate)
+            return True
 
-    execute_sell(trade, current_rate)
-    return True
+    return False
 
 
 def get_target_bid(ticker: Dict[str, float]) -> float:

freqtrade/tests/test_main.py

@@ -217,7 +217,7 @@ def test_handle_trade(default_conf, limit_buy_order, limit_sell_order, mocker):
     assert trade.close_date is not None
 
 
-def test_handle_trade_experimental(default_conf, ticker, mocker, caplog):
+def test_handle_trade_roi(default_conf, ticker, limit_buy_order, mocker, caplog):
     default_conf.update({'experimental': {'use_sell_signal': True}})
     mocker.patch.dict('freqtrade.main._CONF', default_conf)
 
@@ -235,10 +235,46 @@ def test_handle_trade_experimental(default_conf, ticker, mocker, caplog):
     trade = Trade.query.first()
     trade.is_open = True
 
+    # FIX: sniffing logs, suggest handle_trade should not execute_sell
+    #      instead that responsibility should be moved out of handle_trade(),
+    #      we might just want to check if we are in a sell condition without
+    #      executing
+    # if ROI is reached we must sell
+    mocker.patch('freqtrade.main.get_signal', side_effect=lambda s, t: False)
+    assert handle_trade(trade)
+    assert ('freqtrade', logging.DEBUG, 'Executing sell due to ROI ...') in caplog.record_tuples
+    # if ROI is reached we must sell even if sell-signal is not signalled
+    mocker.patch('freqtrade.main.get_signal', side_effect=lambda s, t: True)
+    assert handle_trade(trade)
+    assert ('freqtrade', logging.DEBUG, 'Executing sell due to ROI ...') in caplog.record_tuples
+
+
+def test_handle_trade_experimental(default_conf, ticker, limit_buy_order, mocker, caplog):
+    default_conf.update({'experimental': {'use_sell_signal': True}})
+    mocker.patch.dict('freqtrade.main._CONF', default_conf)
+
+    mocker.patch('freqtrade.main.get_signal', side_effect=lambda s, t: True)
+    mocker.patch.multiple('freqtrade.rpc', init=MagicMock(), send_msg=MagicMock())
+    mocker.patch.multiple('freqtrade.main.exchange',
+                          validate_pairs=MagicMock(),
+                          get_ticker=ticker,
+                          buy=MagicMock(return_value='mocked_limit_buy'))
+    mocker.patch('freqtrade.main.min_roi_reached', return_value=False)
+
+    init(default_conf, create_engine('sqlite://'))
+    create_trade(0.001)
+
+    trade = Trade.query.first()
+    trade.is_open = True
+
     mocker.patch('freqtrade.main.get_signal', side_effect=lambda s, t: False)
     value_returned = handle_trade(trade)
     assert ('freqtrade', logging.DEBUG, 'Checking sell_signal ...') in caplog.record_tuples
     assert value_returned is False
+    mocker.patch('freqtrade.main.get_signal', side_effect=lambda s, t: True)
+    assert handle_trade(trade)
+    s = 'Executing sell due to sell signal ...'
+    assert ('freqtrade', logging.DEBUG, s) in caplog.record_tuples
 
 
 def test_close_trade(default_conf, ticker, limit_buy_order, limit_sell_order, mocker):