From 91dbe76d587a8528c6bb6005adfd4d1da57ae5aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Thu, 7 May 2020 09:03:26 +0200 Subject: [PATCH 1/4] Add messagebus message to clear patch data --- mycroft/configuration/config.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/mycroft/configuration/config.py b/mycroft/configuration/config.py index 4322dfdb92f7..3d17882e8411 100644 --- a/mycroft/configuration/config.py +++ b/mycroft/configuration/config.py @@ -233,11 +233,12 @@ def load_config_stack(configs=None, cache=False): def set_config_update_handlers(bus): """Setup websocket handlers to update config. - Args: + Arguments: bus: Message bus client instance """ bus.on("configuration.updated", Configuration.updated) bus.on("configuration.patch", Configuration.patch) + bus.on("configuration.patch.clear", Configuration.patch_clear) @staticmethod def updated(message): @@ -259,3 +260,14 @@ def patch(message): config = message.data.get("config", {}) merge_dict(Configuration.__patch, config) Configuration.load_config_stack(cache=True) + + @staticmethod + def patch_clear(message): + """Clear the config patch space. + + Arguments: + message: Messagebus message should contain a config + in the data payload. + """ + Configuration.__patch = {} + Configuration.load_config_stack(cache=True) From cd1c3b7c1d5464dba73d443b6f99a4c1d053d8e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Thu, 7 May 2020 15:11:51 +0200 Subject: [PATCH 2/4] Cleanup docstrings in configuration module --- mycroft/configuration/config.py | 90 ++++++++++++++++----------------- 1 file changed, 43 insertions(+), 47 deletions(-) diff --git a/mycroft/configuration/config.py b/mycroft/configuration/config.py index 3d17882e8411..dd82eae651f4 100644 --- a/mycroft/configuration/config.py +++ b/mycroft/configuration/config.py @@ -28,8 +28,8 @@ def is_remote_list(values): - ''' check if this list corresponds to a backend formatted collection of - dictionaries ''' + """Check if list corresponds to a backend formatted collection of dicts + """ for v in values: if not isinstance(v, dict): return False @@ -39,13 +39,11 @@ def is_remote_list(values): def translate_remote(config, setting): - """ - Translate config names from server to equivalents usable - in mycroft-core. + """Translate config names from server to equivalents for mycroft-core. - Args: - config: base config to populate - settings: remote settings to be translated + Arguments: + config: base config to populate + settings: remote settings to be translated """ IGNORED_SETTINGS = ["uuid", "@type", "active", "user", "device"] @@ -69,12 +67,11 @@ def translate_remote(config, setting): def translate_list(config, values): - """ - Translate list formated by mycroft server. + """Translate list formated by mycroft server. - Args: - config (dict): target config - values (list): list from mycroft server config + Arguments: + config (dict): target config + values (list): list from mycroft server config """ for v in values: module = v["@type"] @@ -85,9 +82,7 @@ def translate_list(config, values): class LocalConf(dict): - """ - Config dict from file. - """ + """Config dictionary from file.""" def __init__(self, path): super(LocalConf, self).__init__() if path: @@ -95,11 +90,10 @@ def __init__(self, path): self.load_local(path) def load_local(self, path): - """ - Load local json file into self. + """Load local json file into self. - Args: - path (str): file to load + Arguments: + path (str): file to load """ if exists(path) and isfile(path): try: @@ -115,10 +109,10 @@ def load_local(self, path): LOG.debug("Configuration '{}' not defined, skipping".format(path)) def store(self, path=None): - """ - Cache the received settings locally. The cache will be used if - the remote is unreachable to load settings that are as close - to the user's as possible + """Cache the received settings locally. + + The cache will be used if the remote is unreachable to load settings + that are as close to the user's as possible. """ path = path or self.path with open(path, 'w') as f: @@ -129,9 +123,7 @@ def merge(self, conf): class RemoteConf(LocalConf): - """ - Config dict fetched from mycroft.ai - """ + """Config dictionary fetched from mycroft.ai.""" def __init__(self, cache=None): super(RemoteConf, self).__init__(None) @@ -176,18 +168,23 @@ def __init__(self, cache=None): class Configuration: + """Namespace for operations on the configuration singleton.""" __config = {} # Cached config __patch = {} # Patch config that skills can update to override config @staticmethod def get(configs=None, cache=True): - """ - Get configuration, returns cached instance if available otherwise - builds a new configuration dict. + """Get configuration - Args: - configs (list): List of configuration dicts - cache (boolean): True if the result should be cached + Returns cached instance if available otherwise builds a new + configuration dict. + + Arguments: + configs (list): List of configuration dicts + cache (boolean): True if the result should be cached + + Returns: + (dict) configuration dictionary. """ if Configuration.__config: return Configuration.__config @@ -196,14 +193,14 @@ def get(configs=None, cache=True): @staticmethod def load_config_stack(configs=None, cache=False): - """ - load a stack of config dicts into a single dict + """Load a stack of config dicts into a single dict - Args: - configs (list): list of dicts to load - cache (boolean): True if result should be cached + Arguments: + configs (list): list of dicts to load + cache (boolean): True if result should be cached - Returns: merged dict of all configuration files + Returns: + (dict) merged dict of all configuration files """ if not configs: configs = [LocalConf(DEFAULT_CONFIG), RemoteConf(), @@ -242,20 +239,19 @@ def set_config_update_handlers(bus): @staticmethod def updated(message): - """ - handler for configuration.updated, triggers an update - of cached config. + """Handler for configuration.updated, + + Triggers an update of cached config. """ Configuration.load_config_stack(cache=True) @staticmethod def patch(message): - """ - patch the volatile dict usable by skills + """Patch the volatile dict usable by skills - Args: - message: Messagebus message should contain a config - in the data payload. + Arguments: + message: Messagebus message should contain a config + in the data payload. """ config = message.data.get("config", {}) merge_dict(Configuration.__patch, config) From 46c8a6b51fc04e425fa205974468f49b8f79711f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Fri, 1 May 2020 16:48:48 +0200 Subject: [PATCH 3/4] Add Voight Kampff support setting configurations Adds the "Given the user's {config} is {value}" step implementation This will patch the configuration with a section from a dictionary that can either be a global (shipped in mycroft/res/{lang}/configurations.json) or shipped with the test definition. The file should be named the same as the feature file but instead of ".feature" the extension should be ".config.json". mycroft/res/text/en-us/configurations.json contains a couple of pre-defined configurations that can be applied - units (metric/imperial) - location (Stockholm) After each scenario any applied patch will be cleared --- mycroft/res/text/en-us/configurations.json | 33 ++++++ .../voight_kampff/features/environment.py | 21 ++++ .../features/steps/configuration.py | 105 ++++++++++++++++++ .../voight_kampff/test_setup.py | 8 ++ 4 files changed, 167 insertions(+) create mode 100644 mycroft/res/text/en-us/configurations.json create mode 100644 test/integrationtests/voight_kampff/features/steps/configuration.py diff --git a/mycroft/res/text/en-us/configurations.json b/mycroft/res/text/en-us/configurations.json new file mode 100644 index 000000000000..b446e7cae44f --- /dev/null +++ b/mycroft/res/text/en-us/configurations.json @@ -0,0 +1,33 @@ +{ + "unit system": { + "metric": {"system_unit": "metric"}, + "imperial": {"system_unit": "imperial"} + }, + "location": { + "stockholm": { + "location": { + "city": { + "name": "Stockholm", + "state": { + "code": "SE.18", + "country": { + "code": "SE", + "name": "Sweden" + }, + "name": "Stockholm" + } + }, + "coordinate": { + "latitude": 59.38306, + "longitude": 16.66667 + }, + "timezone": { + "code": "Europe/Stockholm", + "dst_offset": 7200000, + "name": "Europe/Stockholm", + "offset": 3600000 + } + } + } + } +} diff --git a/test/integrationtests/voight_kampff/features/environment.py b/test/integrationtests/voight_kampff/features/environment.py index fa1d87011063..6685aa5c0890 100644 --- a/test/integrationtests/voight_kampff/features/environment.py +++ b/test/integrationtests/voight_kampff/features/environment.py @@ -19,6 +19,7 @@ from msm import MycroftSkillsManager from mycroft.audio import wait_while_speaking +from mycroft.configuration import Configuration from mycroft.messagebus.client import MessageBusClient from mycroft.messagebus import Message from mycroft.util import create_daemon @@ -92,6 +93,9 @@ def before_all(context): context.bus = bus context.matched_message = None context.log = log + context.original_config = {} + context.config = Configuration.get() + Configuration.set_config_update_handlers(bus) def before_feature(context, feature): @@ -110,9 +114,26 @@ def after_feature(context, feature): sleep(1) +def reset_config(context): + """Reset configuration with changes stored in original_config of context. + """ + context.log.info('Resetting patched configuration...') + + context.bus.emit(Message('configuration.patch.clear')) + key = list(context.original_config)[0] + while context.config[key] != context.original_config[key]: + sleep(0.5) + context.original_config = {} + + def after_scenario(context, scenario): + """Wait for mycroft completion and reset any changed state.""" # TODO wait for skill handler complete sleep(0.5) wait_while_speaking() context.bus.clear_messages() context.matched_message = None + + if context.original_config: + # something has changed, reset changes by done in the context + reset_config(context) diff --git a/test/integrationtests/voight_kampff/features/steps/configuration.py b/test/integrationtests/voight_kampff/features/steps/configuration.py new file mode 100644 index 000000000000..89f224d084a4 --- /dev/null +++ b/test/integrationtests/voight_kampff/features/steps/configuration.py @@ -0,0 +1,105 @@ +# Copyright 2020 Mycroft AI Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +import json +from os.path import join, exists +import time + +from behave import given +from mycroft.messagebus import Message +from mycroft.util import resolve_resource_file + + +def patch_config(context, patch): + """Apply patch to config and wait for it to take effect. + + Arguments: + context: Behave context for test + patch: patch to apply + """ + # store originals in context + for key in patch: + # If this patch is redefining an already changed key don't update + if key not in context.original_config: + context.original_config[key] = context.config.get(key) + + # Patch config + patch_config_msg = Message('configuration.patch', {'config': patch}) + context.bus.emit(patch_config_msg) + + # Wait until one of the keys has been updated + key = list(patch.keys())[0] + while context.config.get(key) != patch[key]: + time.sleep(0.5) + + +def get_config_file_definition(configs_path, config, value): + """Read config definition file and return the matching patch dict. + + Arguments: + configs_path: path to the configuration patch json file + config: config value to fetch from the file + value: predefined value to fetch + + Returns: + Patch dictionary or None. + """ + with open(configs_path) as f: + configs = json.load(f) + return configs.get(config, {}).get(value) + + +def get_global_config_definition(context, config, value): + """Get config definitions included with Mycroft. + + Arguments: + context: behave test context + config: config value to fetch from the file + value: predefined value to fetch + + Returns: + Patch dictionary or None. + """ + configs_path = resolve_resource_file(join('text', context.lang, + 'configurations.json')) + return get_config_file_definition(configs_path, config, value) + + +def get_feature_config_definition(context, config, value): + """Get config feature specific config defintion + + Arguments: + context: behave test context + config: config value to fetch from the file + value: predefined value to fetch + + Returns: + Patch dictionary or None. + """ + feature_config = context.feature.filename.replace('.feature', + '.config.json') + if exists(feature_config): + return get_config_file_definition(feature_config, config, value) + else: + return None + + +@given('the user\'s {config} is {value}') +def given_config(context, config, value): + """Patch the configuration with a specific config.""" + config = config.strip('"') + value = value.strip('"') + patch_dict = (get_feature_config_definition(context, config, value) or + get_global_config_definition(context, config, value)) + patch_config(context, patch_dict) diff --git a/test/integrationtests/voight_kampff/test_setup.py b/test/integrationtests/voight_kampff/test_setup.py index a3754169c517..6415c9b3a87c 100644 --- a/test/integrationtests/voight_kampff/test_setup.py +++ b/test/integrationtests/voight_kampff/test_setup.py @@ -38,6 +38,13 @@ FEATURE_DIR = join(dirname(__file__), 'features') + '/' +def copy_config_definition_files(source, destination): + """Copy all feature files from source to destination.""" + # Copy feature files to the feature directory + for f in glob(join(source, '*.config.json')): + shutil.copyfile(f, join(destination, basename(f))) + + def copy_feature_files(source, destination): """Copy all feature files from source to destination.""" # Copy feature files to the feature directory @@ -141,6 +148,7 @@ def collect_test_cases(msm, skills): behave_dir = join(skill.path, 'test', 'behave') if exists(behave_dir): copy_feature_files(behave_dir, FEATURE_DIR) + copy_config_definition_files(behave_dir, FEATURE_DIR) step_dir = join(behave_dir, 'steps') if exists(step_dir): From e08f630bea23aefb63f676ca0cd086d1ec7ee9d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Thu, 7 May 2020 12:21:10 +0200 Subject: [PATCH 4/4] Minor header fixes in voight_kampff - Copyright year - typo --- .../voight_kampff/features/steps/utterance_responses.py | 2 +- test/integrationtests/voight_kampff/test_setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integrationtests/voight_kampff/features/steps/utterance_responses.py b/test/integrationtests/voight_kampff/features/steps/utterance_responses.py index 4bd36056bcb0..cc4629a5eff8 100644 --- a/test/integrationtests/voight_kampff/features/steps/utterance_responses.py +++ b/test/integrationtests/voight_kampff/features/steps/utterance_responses.py @@ -1,4 +1,4 @@ -# Copyright 2017 Mycroft AI Inc. +# Copyright 2020 Mycroft AI Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/test/integrationtests/voight_kampff/test_setup.py b/test/integrationtests/voight_kampff/test_setup.py index 6415c9b3a87c..a2b6f522de9a 100644 --- a/test/integrationtests/voight_kampff/test_setup.py +++ b/test/integrationtests/voight_kampff/test_setup.py @@ -32,7 +32,7 @@ be found and executed by the behave framework. The script also ensures that the skills marked for testing are installed and -that anyi specified extra skills also gets installed into the environment. +that any specified extra skills also gets installed into the environment. """ FEATURE_DIR = join(dirname(__file__), 'features') + '/'