tests and fixes in tljh-config

- add remove-item to cli
- fix setting non-string values (int, bool, float)
- show help when no action is given
- test coverage for cli

tests/test_config.py

@@ -1,11 +1,30 @@
 """
 Test configuration commandline tools
 """
-from tljh import config
-from contextlib import redirect_stdout
-import io
-import pytest
+
+from importlib import reload
+import os
 import tempfile
+from unittest import mock
+
+import pytest
+
+from tljh import config, configurer
+
+
+@pytest.fixture
+def tljh_dir(tmpdir):
+    """Fixture for setting up a temporary tljh dir"""
+    tljh_dir = str(tmpdir.join("tljh").mkdir())
+    with mock.patch.dict(
+        os.environ,
+        {"TLJH_INSTALL_PREFIX": tljh_dir}
+    ):
+        reload(config)
+        reload(configurer)
+        assert config.INSTALL_PREFIX == tljh_dir
+        os.makedirs(config.STATE_DIR)
+        yield tljh_dir
 
 
 def test_set_no_mutate():
@@ -15,12 +34,14 @@ def test_set_no_mutate():
     assert new_conf['a']['b'] == 'c'
     assert conf == {}
 
+
 def test_set_one_level():
     conf = {}
 
     new_conf = config.set_item_in_config(conf, 'a', 'b')
     assert new_conf['a'] == 'b'
 
+
 def test_set_multi_level():
     conf = {}
 
@@ -32,6 +53,7 @@ def test_set_multi_level():
         'f': 'g'
     }
 
+
 def test_set_overwrite():
     """
     We can overwrite already existing config items.
@@ -97,6 +119,7 @@ def test_remove_from_config():
         'a': {'b': {'c': ['d']}}
     }
 
+
 def test_remove_from_config_error():
     with pytest.raises(ValueError):
         config.remove_item_from_config({}, 'a.b.c', 'e')
@@ -108,7 +131,79 @@ def test_remove_from_config_error():
         config.remove_item_from_config({'a': ['b']}, 'a', 'e')
 
 
-def test_show_config():
+def test_reload_hub():
+    with mock.patch('tljh.systemd.restart_service') as restart_service:
+        config.reload_component('hub')
+    assert restart_service.called_with('jupyterhub')
+
+
+def test_reload_proxy(tljh_dir):
+    with mock.patch('tljh.systemd.restart_service') as restart_service:
+        config.reload_component('proxy')
+    assert restart_service.called_with('configurable-http-proxy')
+    assert restart_service.called_with('traefik')
+    assert os.path.exists(os.path.join(config.STATE_DIR, 'traefik.toml'))
+
+
+def test_cli_no_command(capsys):
+    config.main([])
+    captured = capsys.readouterr()
+    assert "usage:" in captured.out
+    assert "positional arguments:" in captured.out
+
+
+@pytest.mark.parametrize(
+    "arg, value",
+    [
+        ("true", True),
+        ("FALSE", False),
+    ],
+)
+def test_cli_set_bool(tljh_dir, arg, value):
+    config.main(["set", "https.enabled", arg])
+    cfg = configurer.load_config()
+    assert cfg['https']['enabled'] == value
+
+
+def test_cli_set_int(tljh_dir):
+    config.main(["set", "https.port", "123"])
+    cfg = configurer.load_config()
+    assert cfg['https']['port'] == 123
+
+
+def test_cli_add_float(tljh_dir):
+    config.main(["add-item", "foo.bar", "1.25"])
+    cfg = configurer.load_config()
+    assert cfg['foo']['bar'] == [1.25]
+
+
+def test_cli_remove_int(tljh_dir):
+    config.main(["add-item", "foo.bar", "1"])
+    config.main(["add-item", "foo.bar", "2"])
+    cfg = configurer.load_config()
+    assert cfg['foo']['bar'] == [1, 2]
+    config.main(["remove-item", "foo.bar", "1"])
+    cfg = configurer.load_config()
+    assert cfg['foo']['bar'] == [2]
+
+
+@pytest.mark.parametrize(
+    "value, expected",
+    [
+        ("1", 1),
+        ("1.25", 1.25),
+        ("x", "x"),
+        ("1x", "1x"),
+        ("1.2x", "1.2x"),
+        (None, None),
+        ("", ""),
+    ],
+)
+def test_parse_value(value, expected):
+    assert config.parse_value(value) == expected
+
+
+def test_show_config(capsys):
     """
     Test stdout output when showing config
     """
@@ -120,16 +215,9 @@ a:
   - 1
     """.strip()
 
-
     with tempfile.NamedTemporaryFile() as tmp:
         tmp.write(conf.encode())
         tmp.flush()
-
-        out = io.StringIO()
-        with redirect_stdout(out):
-            config.show_config(tmp.name)
-
-    assert out.getvalue().strip() == conf
-
-
-
+        config.show_config(tmp.name)
+    out = capsys.readouterr().out
+    assert out.strip() == conf

tljh/config.py

@@ -12,11 +12,14 @@ tljh-config show firstlevel
 tljh-config show firstlevel.second_level
 """
 
-import os
-import sys
 import argparse
-from ruamel.yaml import YAML
 from copy import deepcopy
+import os
+import re
+import sys
+
+from ruamel.yaml import YAML
+yaml = YAML(typ='rt')
 
 
 INSTALL_PREFIX = os.environ.get('TLJH_INSTALL_PREFIX', '/opt/tljh')
@@ -26,9 +29,6 @@ STATE_DIR = os.path.join(INSTALL_PREFIX, 'state')
 CONFIG_FILE = os.path.join(INSTALL_PREFIX, 'config.yaml')
 
 
-yaml = YAML(typ='rt')
-
-
 def set_item_in_config(config, property_path, value):
     """
     Set key at property_path to value in config & return new config.
@@ -82,6 +82,7 @@ def add_item_to_config(config, property_path, value):
 
     return config_copy
 
+
 def remove_item_from_config(config, property_path, value):
     """
     Add an item to a list in config.
@@ -114,7 +115,7 @@ def show_config(config_path):
             config = yaml.load(f)
     except FileNotFoundError:
         config = {}
-    
+
     yaml.dump(config, sys.stdout)
 
 
@@ -153,6 +154,25 @@ def add_config_value(config_path, key_path, value):
     with open(config_path, 'w') as f:
         yaml.dump(config, f)
 
+
+def remove_config_value(config_path, key_path, value):
+    """
+    Remove value from list at key_path
+    """
+    # FIXME: Have a file lock here
+    # FIXME: Validate schema here
+    try:
+        with open(config_path) as f:
+            config = yaml.load(f)
+    except FileNotFoundError:
+        config = {}
+
+    config = remove_item_from_config(config, key_path, value)
+
+    with open(config_path, 'w') as f:
+        yaml.dump(config, f)
+
+
 def reload_component(component):
     """
     Reload a TLJH component.
@@ -172,11 +192,31 @@ def reload_component(component):
         print('Proxy reload with new configuration complete')
 
 
-def main():
+def parse_value(value_str):
+    """Parse a value string"""
+    if value_str is None:
+        return value_str
+    if re.match(r'^\d+$', value_str):
+        return int(value_str)
+    elif re.match(r'^\d+\.\d*$', value_str):
+        return float(value_str)
+    elif value_str.lower() == 'true':
+        return True
+    elif value_str.lower() == 'false':
+        return False
+    else:
+        # it's a string
+        return value_str
+
+
+def main(argv=None):
+    if argv is None:
+        argv = sys.argv[1:]
+
     argparser = argparse.ArgumentParser()
     argparser.add_argument(
         '--config-path',
-        default='/opt/tljh/config.yaml',
+        default=CONFIG_FILE,
         help='Path to TLJH config.yaml file'
     )
     subparsers = argparser.add_subparsers(dest='action')
@@ -237,19 +277,21 @@ def main():
         nargs='?'
     )
 
-    args = argparser.parse_args()
+    args = argparser.parse_args(argv)
 
     if args.action == 'show':
         show_config(args.config_path)
     elif args.action == 'set':
-        set_config_value(args.config_path, args.key_path, args.value)
+        set_config_value(args.config_path, args.key_path, parse_value(args.value))
     elif args.action == 'add-item':
-        add_config_value(args.config_path, args.key_path, args.value)
+        add_config_value(args.config_path, args.key_path, parse_value(args.value))
     elif args.action == 'remove-item':
-        add_config_value(args.config_path, args.key_path, args.value)
+        remove_config_value(args.config_path, args.key_path, parse_value(args.value))
     elif args.action == 'reload':
         reload_component(args.component)
-    
+    else:
+        argparser.print_help()
+
 
 if __name__ == '__main__':
     main()