From a4427068cc0024022ba5c33567eb69bd65b5a089 Mon Sep 17 00:00:00 2001 From: Min RK Date: Tue, 31 Jul 2018 13:24:37 +0200 Subject: [PATCH] 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 | 118 +++++++++++++++++++++++++++++++++++++------ tljh/config.py | 70 ++++++++++++++++++++----- 2 files changed, 159 insertions(+), 29 deletions(-) diff --git a/tests/test_config.py b/tests/test_config.py index 271ae79..aed0f49 100644 --- a/tests/test_config.py +++ b/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 diff --git a/tljh/config.py b/tljh/config.py index e0d3d93..d4d45d0 100644 --- a/tljh/config.py +++ b/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()