From 47675943eeebf859d1c2bdb3da416ffea280f626 Mon Sep 17 00:00:00 2001
From: kryofly <kryofly@space>
Date: Sat, 6 Jan 2018 07:39:05 +0100
Subject: [PATCH 1/4] split common command line args parsing

A new function parse_args_common() that only parses
common command line options. The returned object can
be composed to parse more arguments.
As is done by parse_args().
---
 freqtrade/main.py            |  3 ++-
 freqtrade/misc.py            | 47 +++++++++++++++++++++---------------
 freqtrade/tests/test_misc.py | 28 ++++++++++-----------
 3 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/freqtrade/main.py b/freqtrade/main.py
index 254bed33e..e0fd784d0 100755
--- a/freqtrade/main.py
+++ b/freqtrade/main.py
@@ -405,7 +405,8 @@ def main() -> None:
     :return: None
     """
     global _CONF
-    args = parse_args(sys.argv[1:])
+    args = parse_args(sys.argv[1:],
+                      'Simple High Frequency Trading Bot for crypto currencies')
     if not args:
         exit(0)
 
diff --git a/freqtrade/misc.py b/freqtrade/misc.py
index 9f414a72f..7ca5cf976 100644
--- a/freqtrade/misc.py
+++ b/freqtrade/misc.py
@@ -81,21 +81,12 @@ def throttle(func: Callable[..., Any], min_secs: float, *args, **kwargs) -> Any:
     return result
 
 
-def parse_args(args: List[str]):
+def parse_args_common(args: List[str], description: str):
     """
-    Parses given arguments and returns an argparse Namespace instance.
-    Returns None if a sub command has been selected and executed.
+    Parses given common arguments and returns them as a parsed object.
     """
     parser = argparse.ArgumentParser(
-        description='Simple High Frequency Trading Bot for crypto currencies'
-    )
-    parser.add_argument(
-        '-c', '--config',
-        help='specify configuration file (default: config.json)',
-        dest='config',
-        default='config.json',
-        type=str,
-        metavar='PATH',
+        description=description
     )
     parser.add_argument(
         '-v', '--verbose',
@@ -110,6 +101,30 @@ def parse_args(args: List[str]):
         action='version',
         version='%(prog)s {}'.format(__version__),
     )
+    parser.add_argument(
+        '-c', '--config',
+        help='specify configuration file (default: config.json)',
+        dest='config',
+        default='config.json',
+        type=str,
+        metavar='PATH',
+    )
+    return parser
+
+
+def parse_args(args: List[str], description: str):
+    """
+    Parses given arguments and returns an argparse Namespace instance.
+    Returns None if a sub command has been selected and executed.
+    """
+    parser = parse_args_common(args, description)
+    parser.add_argument(
+        '--dry-run-db',
+        help='Force dry run to use a local DB "tradesv3.dry_run.sqlite" instead of memory DB. Work only if dry_run is \
+             enabled.',  # noqa
+        action='store_true',
+        dest='dry_run_db',
+    )
     parser.add_argument(
         '--dynamic-whitelist',
         help='dynamically generate and update whitelist based on 24h BaseVolume (Default 20 currencies)',  # noqa
@@ -119,13 +134,7 @@ def parse_args(args: List[str]):
         metavar='INT',
         nargs='?',
     )
-    parser.add_argument(
-        '--dry-run-db',
-        help='Force dry run to use a local DB "tradesv3.dry_run.sqlite" instead of memory DB. Work only if dry_run is \
-             enabled.',  # noqa
-        action='store_true',
-        dest='dry_run_db',
-    )
+
     build_subcommands(parser)
     parsed_args = parser.parse_args(args)
 
diff --git a/freqtrade/tests/test_misc.py b/freqtrade/tests/test_misc.py
index cd529039a..8c5cc1b24 100644
--- a/freqtrade/tests/test_misc.py
+++ b/freqtrade/tests/test_misc.py
@@ -39,7 +39,7 @@ def test_throttle_with_assets():
 
 
 def test_parse_args_defaults():
-    args = parse_args([])
+    args = parse_args([], '')
     assert args is not None
     assert args.config == 'config.json'
     assert args.dynamic_whitelist is None
@@ -48,46 +48,46 @@ def test_parse_args_defaults():
 
 def test_parse_args_invalid():
     with pytest.raises(SystemExit, match=r'2'):
-        parse_args(['-c'])
+        parse_args(['-c'], '')
 
 
 def test_parse_args_config():
-    args = parse_args(['-c', '/dev/null'])
+    args = parse_args(['-c', '/dev/null'], '')
     assert args is not None
     assert args.config == '/dev/null'
 
-    args = parse_args(['--config', '/dev/null'])
+    args = parse_args(['--config', '/dev/null'], '')
     assert args is not None
     assert args.config == '/dev/null'
 
 
 def test_parse_args_verbose():
-    args = parse_args(['-v'])
+    args = parse_args(['-v'], '')
     assert args is not None
     assert args.loglevel == 10
 
 
 def test_parse_args_dynamic_whitelist():
-    args = parse_args(['--dynamic-whitelist'])
+    args = parse_args(['--dynamic-whitelist'], '')
     assert args is not None
     assert args.dynamic_whitelist is 20
 
 
 def test_parse_args_dynamic_whitelist_10():
-    args = parse_args(['--dynamic-whitelist', '10'])
+    args = parse_args(['--dynamic-whitelist', '10'], '')
     assert args is not None
     assert args.dynamic_whitelist is 10
 
 
 def test_parse_args_dynamic_whitelist_invalid_values():
     with pytest.raises(SystemExit, match=r'2'):
-        parse_args(['--dynamic-whitelist', 'abc'])
+        parse_args(['--dynamic-whitelist', 'abc'], '')
 
 
 def test_parse_args_backtesting(mocker):
     backtesting_mock = mocker.patch(
         'freqtrade.optimize.backtesting.start', MagicMock())
-    args = parse_args(['backtesting'])
+    args = parse_args(['backtesting'], '')
     assert args is None
     assert backtesting_mock.call_count == 1
 
@@ -102,10 +102,10 @@ def test_parse_args_backtesting(mocker):
 
 def test_parse_args_backtesting_invalid():
     with pytest.raises(SystemExit, match=r'2'):
-        parse_args(['backtesting --ticker-interval'])
+        parse_args(['backtesting --ticker-interval'], '')
 
     with pytest.raises(SystemExit, match=r'2'):
-        parse_args(['backtesting --ticker-interval', 'abc'])
+        parse_args(['backtesting --ticker-interval', 'abc'], '')
 
 
 def test_parse_args_backtesting_custom(mocker):
@@ -116,7 +116,7 @@ def test_parse_args_backtesting_custom(mocker):
         'backtesting',
         '--live',
         '--ticker-interval', '1',
-        '--refresh-pairs-cached'])
+        '--refresh-pairs-cached'], '')
     assert args is None
     assert backtesting_mock.call_count == 1
 
@@ -133,7 +133,7 @@ def test_parse_args_backtesting_custom(mocker):
 def test_parse_args_hyperopt(mocker):
     hyperopt_mock = mocker.patch(
         'freqtrade.optimize.hyperopt.start', MagicMock())
-    args = parse_args(['hyperopt'])
+    args = parse_args(['hyperopt'], '')
     assert args is None
     assert hyperopt_mock.call_count == 1
 
@@ -147,7 +147,7 @@ def test_parse_args_hyperopt(mocker):
 def test_parse_args_hyperopt_custom(mocker):
     hyperopt_mock = mocker.patch(
         'freqtrade.optimize.hyperopt.start', MagicMock())
-    args = parse_args(['-c', 'test_conf.json', 'hyperopt', '--epochs', '20'])
+    args = parse_args(['-c', 'test_conf.json', 'hyperopt', '--epochs', '20'], '')
     assert args is None
     assert hyperopt_mock.call_count == 1
 

From e4500af736d96a2bae8a2bd9e52035fa81229e46 Mon Sep 17 00:00:00 2001
From: kryofly <kryofly@space>
Date: Sat, 6 Jan 2018 08:27:44 +0100
Subject: [PATCH 2/4] test case for common CLI parsing

Rearrange current tests.
---
 freqtrade/tests/test_misc.py | 53 ++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/freqtrade/tests/test_misc.py b/freqtrade/tests/test_misc.py
index 8c5cc1b24..3271b1a1e 100644
--- a/freqtrade/tests/test_misc.py
+++ b/freqtrade/tests/test_misc.py
@@ -1,13 +1,15 @@
 # pragma pylint: disable=missing-docstring,C0103
 import json
 import time
+import argparse
 from copy import deepcopy
 from unittest.mock import MagicMock
 
 import pytest
 from jsonschema import ValidationError
 
-from freqtrade.misc import throttle, parse_args, load_config
+from freqtrade.misc import throttle, parse_args, load_config,\
+     parse_args_common
 
 
 def test_throttle():
@@ -38,44 +40,59 @@ def test_throttle_with_assets():
     assert result == -1
 
 
+# Parse common command-line-arguments
+# used for all tools
+
+
+def test_parse_args_none():
+    args = parse_args_common([], '')
+    assert isinstance(args, argparse.ArgumentParser)
+
+
 def test_parse_args_defaults():
     args = parse_args([], '')
-    assert args is not None
     assert args.config == 'config.json'
     assert args.dynamic_whitelist is None
     assert args.loglevel == 20
 
 
+def test_parse_args_config():
+    args = parse_args(['-c', '/dev/null'], '')
+    assert args.config == '/dev/null'
+
+    args = parse_args(['--config', '/dev/null'], '')
+    assert args.config == '/dev/null'
+
+
+def test_parse_args_verbose():
+    args = parse_args(['-v'], '')
+    assert args.loglevel == 10
+
+    args = parse_args(['--verbose'], '')
+    assert args.loglevel == 10
+
+
+def test_parse_args_version():
+    with pytest.raises(SystemExit, match=r'0'):
+        parse_args(['--version'], '')
+
+
 def test_parse_args_invalid():
     with pytest.raises(SystemExit, match=r'2'):
         parse_args(['-c'], '')
 
 
-def test_parse_args_config():
-    args = parse_args(['-c', '/dev/null'], '')
-    assert args is not None
-    assert args.config == '/dev/null'
-
-    args = parse_args(['--config', '/dev/null'], '')
-    assert args is not None
-    assert args.config == '/dev/null'
-
-
-def test_parse_args_verbose():
-    args = parse_args(['-v'], '')
-    assert args is not None
-    assert args.loglevel == 10
+# Parse command-line-arguments
+# used for main, backtesting and hyperopt
 
 
 def test_parse_args_dynamic_whitelist():
     args = parse_args(['--dynamic-whitelist'], '')
-    assert args is not None
     assert args.dynamic_whitelist is 20
 
 
 def test_parse_args_dynamic_whitelist_10():
     args = parse_args(['--dynamic-whitelist', '10'], '')
-    assert args is not None
     assert args.dynamic_whitelist is 10
 
 

From e6e57e47cfd89fea05b4ac61e72a0df93e959ce3 Mon Sep 17 00:00:00 2001
From: kryofly <kryofly@space>
Date: Sat, 6 Jan 2018 08:38:47 +0100
Subject: [PATCH 3/4] plot script can take arguments

---
 scripts/plot_dataframe.py | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/scripts/plot_dataframe.py b/scripts/plot_dataframe.py
index 0d193726d..44961891c 100755
--- a/scripts/plot_dataframe.py
+++ b/scripts/plot_dataframe.py
@@ -1,17 +1,33 @@
 #!/usr/bin/env python3
 
+import sys
+import argparse
 import matplotlib  # Install PYQT5 manually if you want to test this helper function
 matplotlib.use("Qt5Agg")
 import matplotlib.pyplot as plt
 from freqtrade import exchange, analyze
+from freqtrade.misc import parse_args_common
 
 
-def plot_analyzed_dataframe(pair: str) -> None:
+def plot_parse_args(args ):
+    parser = parse_args_common(args, 'Graph utility')
+    parser.add_argument(
+        '-p', '--pair',
+        help = 'What currency pair',
+        dest = 'pair',
+        default = 'BTC_ETH',
+        type = str,
+    )
+    return parser.parse_args(args)
+
+
+def plot_analyzed_dataframe(args) -> None:
     """
     Calls analyze() and plots the returned dataframe
     :param pair: pair as str
     :return: None
     """
+    pair = args.pair
 
     # Init Bittrex to use public API
     exchange._API = exchange.Bittrex({'key': '', 'secret': ''})
@@ -50,4 +66,5 @@ def plot_analyzed_dataframe(pair: str) -> None:
 
 
 if __name__ == '__main__':
-    plot_analyzed_dataframe('BTC_ETH')
+    args = plot_parse_args(sys.argv[1:])
+    plot_analyzed_dataframe(args)

From 984204e380dc14e591a81f30993a152ebb5bbaf4 Mon Sep 17 00:00:00 2001
From: kryofly <kryofly@space>
Date: Sat, 6 Jan 2018 11:21:09 +0100
Subject: [PATCH 4/4] let parse_args only parse, no continuation

This removes parse_args() from the call stack
It pushes down the test-mocking one level [from parse_args() to main()].
Moves parse_args into a more generic 'modules' parsing direction.
---
 freqtrade/main.py            |  9 ++++--
 freqtrade/misc.py            |  9 +-----
 freqtrade/tests/test_main.py | 33 ++++++++++++++++++++++
 freqtrade/tests/test_misc.py | 53 ++++--------------------------------
 4 files changed, 46 insertions(+), 58 deletions(-)

diff --git a/freqtrade/main.py b/freqtrade/main.py
index e0fd784d0..9368fa844 100755
--- a/freqtrade/main.py
+++ b/freqtrade/main.py
@@ -399,15 +399,18 @@ def cleanup() -> None:
     exit(0)
 
 
-def main() -> None:
+def main(sysargv=sys.argv[1:]) -> None:
     """
     Loads and validates the config and handles the main loop
     :return: None
     """
     global _CONF
-    args = parse_args(sys.argv[1:],
+    args = parse_args(sysargv,
                       'Simple High Frequency Trading Bot for crypto currencies')
-    if not args:
+
+    # A subcommand has been issued
+    if hasattr(args, 'func'):
+        args.func(args)
         exit(0)
 
     # Initialize logger
diff --git a/freqtrade/misc.py b/freqtrade/misc.py
index 7ca5cf976..fada452cc 100644
--- a/freqtrade/misc.py
+++ b/freqtrade/misc.py
@@ -136,14 +136,7 @@ def parse_args(args: List[str], description: str):
     )
 
     build_subcommands(parser)
-    parsed_args = parser.parse_args(args)
-
-    # No subcommand as been selected
-    if not hasattr(parsed_args, 'func'):
-        return parsed_args
-
-    parsed_args.func(parsed_args)
-    return None
+    return parser.parse_args(args)
 
 
 def build_subcommands(parser: argparse.ArgumentParser) -> None:
diff --git a/freqtrade/tests/test_main.py b/freqtrade/tests/test_main.py
index 004126268..fab2aa5fd 100644
--- a/freqtrade/tests/test_main.py
+++ b/freqtrade/tests/test_main.py
@@ -15,6 +15,39 @@ from freqtrade.main import create_trade, handle_trade, init, \
     get_target_bid, _process, execute_sell, check_handle_timedout
 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())
+    with pytest.raises(SystemExit, match=r'0'):
+        main.main(['backtesting'])
+    assert backtesting_mock.call_count == 1
+    call_args = backtesting_mock.call_args[0][0]
+    assert call_args.config == 'config.json'
+    assert call_args.live is False
+    assert call_args.loglevel == 20
+    assert call_args.subparser == 'backtesting'
+    assert call_args.func is not None
+    assert call_args.ticker_interval == 5
+
+
+def test_main_start_hyperopt(mocker):
+    hyperopt_mock = mocker.patch(
+        'freqtrade.optimize.hyperopt.start', MagicMock())
+    with pytest.raises(SystemExit, match=r'0'):
+        main.main(['hyperopt'])
+    assert hyperopt_mock.call_count == 1
+    call_args = hyperopt_mock.call_args[0][0]
+    assert call_args.config == 'config.json'
+    assert call_args.loglevel == 20
+    assert call_args.subparser == 'hyperopt'
+    assert call_args.func is not None
 
 
 def test_process_trade_creation(default_conf, ticker, limit_buy_order, health, mocker):
diff --git a/freqtrade/tests/test_misc.py b/freqtrade/tests/test_misc.py
index 3271b1a1e..0b85000cb 100644
--- a/freqtrade/tests/test_misc.py
+++ b/freqtrade/tests/test_misc.py
@@ -3,7 +3,6 @@ import json
 import time
 import argparse
 from copy import deepcopy
-from unittest.mock import MagicMock
 
 import pytest
 from jsonschema import ValidationError
@@ -101,22 +100,6 @@ def test_parse_args_dynamic_whitelist_invalid_values():
         parse_args(['--dynamic-whitelist', 'abc'], '')
 
 
-def test_parse_args_backtesting(mocker):
-    backtesting_mock = mocker.patch(
-        'freqtrade.optimize.backtesting.start', MagicMock())
-    args = parse_args(['backtesting'], '')
-    assert args is None
-    assert backtesting_mock.call_count == 1
-
-    call_args = backtesting_mock.call_args[0][0]
-    assert call_args.config == 'config.json'
-    assert call_args.live is False
-    assert call_args.loglevel == 20
-    assert call_args.subparser == 'backtesting'
-    assert call_args.func is not None
-    assert call_args.ticker_interval == 5
-
-
 def test_parse_args_backtesting_invalid():
     with pytest.raises(SystemExit, match=r'2'):
         parse_args(['backtesting --ticker-interval'], '')
@@ -125,19 +108,14 @@ def test_parse_args_backtesting_invalid():
         parse_args(['backtesting --ticker-interval', 'abc'], '')
 
 
-def test_parse_args_backtesting_custom(mocker):
-    backtesting_mock = mocker.patch(
-        'freqtrade.optimize.backtesting.start', MagicMock())
-    args = parse_args([
+def test_parse_args_backtesting_custom():
+    args = [
         '-c', 'test_conf.json',
         'backtesting',
         '--live',
         '--ticker-interval', '1',
-        '--refresh-pairs-cached'], '')
-    assert args is None
-    assert backtesting_mock.call_count == 1
-
-    call_args = backtesting_mock.call_args[0][0]
+        '--refresh-pairs-cached']
+    call_args = parse_args(args, '')
     assert call_args.config == 'test_conf.json'
     assert call_args.live is True
     assert call_args.loglevel == 20
@@ -147,28 +125,9 @@ def test_parse_args_backtesting_custom(mocker):
     assert call_args.refresh_pairs is True
 
 
-def test_parse_args_hyperopt(mocker):
-    hyperopt_mock = mocker.patch(
-        'freqtrade.optimize.hyperopt.start', MagicMock())
-    args = parse_args(['hyperopt'], '')
-    assert args is None
-    assert hyperopt_mock.call_count == 1
-
-    call_args = hyperopt_mock.call_args[0][0]
-    assert call_args.config == 'config.json'
-    assert call_args.loglevel == 20
-    assert call_args.subparser == 'hyperopt'
-    assert call_args.func is not None
-
-
 def test_parse_args_hyperopt_custom(mocker):
-    hyperopt_mock = mocker.patch(
-        'freqtrade.optimize.hyperopt.start', MagicMock())
-    args = parse_args(['-c', 'test_conf.json', 'hyperopt', '--epochs', '20'], '')
-    assert args is None
-    assert hyperopt_mock.call_count == 1
-
-    call_args = hyperopt_mock.call_args[0][0]
+    args = ['-c', 'test_conf.json', 'hyperopt', '--epochs', '20']
+    call_args = parse_args(args, '')
     assert call_args.config == 'test_conf.json'
     assert call_args.epochs == 20
     assert call_args.loglevel == 20