From fd208e67518f452219f2ffb98df9fccef578fa5d Mon Sep 17 00:00:00 2001 From: Steven Cartmell Date: Mon, 4 Sep 2017 17:00:53 +0100 Subject: [PATCH 1/6] Add JSON schema based validation to mbed config script - Added app and lib JSON schema definition files which specify the valid keys and values that can be used in mbed library and application configuration files. The primary different between the app and lib schema is that the lib config requires a name key. - Modified the expected error code in some of the test cases. The error message is now issued by the JSON validator. - Added some validation code to the config script which checks the validity of the mbed_app.json file when it is initially loaded. - Added some validation code to config script which checks each of the mbed_lib.json scripts when they are loaded. - Removed manual checks for allowable config keys from within the mbed_app and mbed_lib json files. - Removed the check_dict_types() function which was no longer being called. --- tools/config/__init__.py | 58 ++++++++---------- tools/config/schema_app.json | 110 +++++++++++++++++++++++++++++++++++ tools/config/schema_lib.json | 110 +++++++++++++++++++++++++++++++++++ 3 files changed, 243 insertions(+), 35 deletions(-) create mode 100644 tools/config/schema_app.json create mode 100644 tools/config/schema_lib.json diff --git a/tools/config/__init__.py b/tools/config/__init__.py index 249afb4c8ac..16405d4c8bd 100644 --- a/tools/config/__init__.py +++ b/tools/config/__init__.py @@ -16,6 +16,7 @@ """ from copy import deepcopy +import json import os from os.path import dirname, abspath, exists, join, isabs import sys @@ -24,6 +25,7 @@ from intelhex import IntelHex from jinja2 import FileSystemLoader, StrictUndefined from jinja2.environment import Environment +from jsonschema import validate, Draft4Validator # Implementation of mbed configuration mechanism from tools.utils import json_file_to_dict, intelhex_offset from tools.arm_pack_manager import Cache @@ -341,12 +343,6 @@ def _process_macros(mlist, macros, unit_name, unit_kind): macros[macro.macro_name] = macro -def check_dict_types(dict, type_dict, dict_loc): - for key, value in dict.iteritems(): - if not isinstance(value, type_dict[key]): - raise ConfigException("The value of %s.%s is not of type %s" % - (dict_loc, key, type_dict[key].__name__)) - Region = namedtuple("Region", "name start size active filename") class Config(object): @@ -357,17 +353,8 @@ class Config(object): __mbed_app_config_name = "mbed_app.json" __mbed_lib_config_name = "mbed_lib.json" - # Allowed keys in configuration dictionaries, and their types - # (targets can have any kind of keys, so this validation is not applicable - # to them) - __allowed_keys = { - "library": {"name": str, "config": dict, "target_overrides": dict, - "macros": list, "__config_path": str}, - "application": {"config": dict, "target_overrides": dict, - "macros": list, "__config_path": str, - "artifact_name": str} - } - + __unused_overrides = set(["target.bootloader_img", "target.restrict_size", + "target.mbed_app_start", "target.mbed_app_size"]) # Allowed features in configurations __allowed_features = [ @@ -420,15 +407,15 @@ def __init__(self, tgt, top_level_dirs=None, app_config=None): ConfigException("Could not parse mbed app configuration from %s" % self.app_config_location)) - # Check the keys in the application configuration data - unknown_keys = set(self.app_config_data.keys()) - \ - set(self.__allowed_keys["application"].keys()) - if unknown_keys: - raise ConfigException("Unknown key(s) '%s' in %s" % - (",".join(unknown_keys), - self.__mbed_app_config_name)) - check_dict_types(self.app_config_data, self.__allowed_keys["application"], - "app-config") + # Validate the format of the JSON file based on the schema_app.json + schema_path = os.path.join(os.path.dirname(__file__), "schema_app.json") + validator = Draft4Validator(json_file_to_dict(schema_path)) + + errors = sorted(validator.iter_errors(self.app_config_data)) + + if errors: + raise ConfigException(",".join(x.message for x in errors)) + # Update the list of targets with the ones defined in the application # config, if applicable self.lib_config_data = {} @@ -480,9 +467,16 @@ def add_config_files(self, flist): cfg["__config_path"] = full_path - if "name" not in cfg: - raise ConfigException( - "Library configured at %s has no name field." % full_path) + # Validate the format of the JSON file based on the schema_lib.json + schema_path = os.path.join(os.path.dirname(__file__), + "schema_lib.json") + validator = Draft4Validator(json_file_to_dict(schema_path)) + + errors = sorted(validator.iter_errors(cfg)) + + if errors: + raise ConfigException(",".join(x.message for x in errors)) + # If there's already a configuration for a module with the same # name, exit with error if self.lib_config_data.has_key(cfg["name"]): @@ -781,12 +775,6 @@ def get_lib_config_data(self): """ all_params, macros = {}, {} for lib_name, lib_data in self.lib_config_data.items(): - unknown_keys = (set(lib_data.keys()) - - set(self.__allowed_keys["library"].keys())) - if unknown_keys: - raise ConfigException("Unknown key(s) '%s' in %s" % - (",".join(unknown_keys), lib_name)) - check_dict_types(lib_data, self.__allowed_keys["library"], lib_name) all_params.update(self._process_config_and_overrides(lib_data, {}, lib_name, "library")) diff --git a/tools/config/schema_app.json b/tools/config/schema_app.json new file mode 100644 index 00000000000..aeeac243e3b --- /dev/null +++ b/tools/config/schema_app.json @@ -0,0 +1,110 @@ +{ + "$schema": "http://json-schema.org/draft-06/schema#", + "title": "Mbed Library Schema", + "description": "Configuration file for an mbed library", + "type": "object", + "$id": "http://example.com/root.json", + "definitions": { + "config_parameter_long": { + "type": "object", + "properties": { + "help": { + "description": "An optional help message that describes the purpose of the parameter", + "type": "string" + }, + "value": { + "description": "An optional field that defines the value of the parameter", + "type": [ + "integer", + "string", + "boolean" + ] + }, + "required": { + "description": "An optional field that specifies whether the parameter must be given a value before compiling the code. (False by default)", + "type": "boolean" + }, + "macro_name": { + "description": "An optional field for the macro defined at compile time for this configuration parameter. The system will automatically figure out the macro name from the configuration parameter, but this field will override it", + "type": "string" + } + } + }, + "config_parameter_short": { + "type": [ + "string", + "integer", + "boolean" + ] + }, + "config_parameter_base": { + "oneOf": [ + { + "$ref": "#/definitions/config_parameter_long" + }, + { + "$ref": "#/definitions/config_parameter_short" + } + ] + }, + "target_override_entry": { + "type": "object", + "patternProperties": { + "^\\S+$": {} + }, + "additionalProperties": false + } + }, + "properties": { + "name": { + "description": "Name of the library", + "type": "string", + "items": { + "type": "string" + } + }, + "config": { + "description": "List of configuration parameters", + "type": "object", + "patternProperties": { + "^[^ ]+$": { + "$ref": "#/definitions/config_parameter_base" + } + }, + "additionalProperties": false + }, + "target_overrides": { + "description": "List of overrides for specific targets", + "type": "object", + "patternProperties": { + "\\*": { + "type": "object", + "patternProperties": { + ".*\\..*": {} + }, + "additionalProperties": false + }, + "^\\S+$": { + "$ref": "#/definitions/target_override_entry" + } + }, + "additionalProperties": false + }, + "macros": { + "description": "A list of extra macros that will be defined when compiling a project that includes this library.", + "type": "array", + "items": { + "type": "string", + "pattern": "^([A-Za-z0-9_]+|[A-Za-z0-9_]+=[0-9]+|[A-Za-z0-9_]+=\\\".*\\\")$" + } + }, + "__config_path": { + "description": "Path to configuration file", + "type": "string" + }, + "artifact_name": { + "type": "string" + } + }, + "additionalProperties": false +} diff --git a/tools/config/schema_lib.json b/tools/config/schema_lib.json new file mode 100644 index 00000000000..70a4f411d13 --- /dev/null +++ b/tools/config/schema_lib.json @@ -0,0 +1,110 @@ +{ + "$schema": "http://json-schema.org/draft-06/schema#", + "title": "Mbed Library Schema", + "description": "Configuration file for an mbed library", + "type": "object", + "$id": "http://example.com/root.json", + "definitions": { + "config_parameter_long": { + "type": "object", + "properties": { + "help": { + "description": "An optional help message that describes the purpose of the parameter", + "type": "string" + }, + "value": { + "description": "An optional field that defines the value of the parameter", + "type": [ + "integer", + "string", + "boolean" + ] + }, + "required": { + "description": "An optional field that specifies whether the parameter must be given a value before compiling the code. (False by default)", + "type": "boolean" + }, + "macro_name": { + "description": "An optional field for the macro defined at compile time for this configuration parameter. The system will automatically figure out the macro name from the configuration parameter, but this field will override it", + "type": "string" + } + } + }, + "config_parameter_short": { + "type": [ + "string", + "integer", + "boolean" + ] + }, + "config_parameter_base": { + "oneOf": [ + { + "$ref": "#/definitions/config_parameter_long" + }, + { + "$ref": "#/definitions/config_parameter_short" + } + ] + }, + "target_override_entry": { + "type": "object", + "patternProperties": { + "^\\S+$": {} + }, + "additionalProperties": false + } + }, + "properties": { + "name": { + "description": "Name of the library", + "type": "string", + "items": { + "type": "string" + } + }, + "config": { + "description": "List of configuration parameters", + "type": "object", + "patternProperties": { + "^[^ ]+$": { + "$ref": "#/definitions/config_parameter_base" + } + }, + "additionalProperties": false + }, + "target_overrides": { + "description": "List of overrides for specific targets", + "type": "object", + "patternProperties": { + "\\*": { + "type": "object", + "patternProperties": { + ".*\\..*": {} + }, + "additionalProperties": false + }, + "^\\S+$": { + "$ref": "#/definitions/target_override_entry" + } + }, + "additionalProperties": false + }, + "macros": { + "description": "A list of extra macros that will be defined when compiling a project that includes this library.", + "type": "array", + "items": { + "type": "string", + "pattern": "^([A-Za-z0-9_]+|[A-Za-z0-9_]+=[0-9]+|[A-Za-z0-9_]+=\\\".*\\\")$" + } + }, + "__config_path": { + "description": "Path to configuration file", + "type": "string" + } + }, + "required": [ + "name" + ], + "additionalProperties": false +} From d208421efc61c3d39605397c535640f0d1014bd1 Mon Sep 17 00:00:00 2001 From: Steven Cartmell Date: Tue, 5 Sep 2017 15:38:47 +0100 Subject: [PATCH 2/6] Add JSON Schema library to requirements.txt --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 0afa9e04670..f98cd36c92a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,3 +12,4 @@ mbed-greentea>=0.2.24 beautifulsoup4>=4 fuzzywuzzy>=0.11 pyelftools>=0.24 +jsonschema>=2.6 From 3ff7de9771621d3119ac4bac950823cb58322372 Mon Sep 17 00:00:00 2001 From: Steven Cartmell Date: Wed, 6 Sep 2017 11:24:13 +0100 Subject: [PATCH 3/6] Remove config_path setting and amend regex in mbed config JSON schema - Removed the config_path setting from schema and moved the addition of the config_path value after the validation is done. - Altered the macro validation regex to be more lenient. Now verifies that if '=' is used in the macro definition that something comes after it. --- tools/config/__init__.py | 4 ++-- tools/config/schema_app.json | 6 +----- tools/config/schema_lib.json | 6 +----- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/tools/config/__init__.py b/tools/config/__init__.py index 16405d4c8bd..b6aa033786f 100644 --- a/tools/config/__init__.py +++ b/tools/config/__init__.py @@ -465,8 +465,6 @@ def add_config_files(self, flist): sys.stderr.write(str(exc) + "\n") continue - cfg["__config_path"] = full_path - # Validate the format of the JSON file based on the schema_lib.json schema_path = os.path.join(os.path.dirname(__file__), "schema_lib.json") @@ -477,6 +475,8 @@ def add_config_files(self, flist): if errors: raise ConfigException(",".join(x.message for x in errors)) + cfg["__config_path"] = full_path + # If there's already a configuration for a module with the same # name, exit with error if self.lib_config_data.has_key(cfg["name"]): diff --git a/tools/config/schema_app.json b/tools/config/schema_app.json index aeeac243e3b..4fd2611f3f0 100644 --- a/tools/config/schema_app.json +++ b/tools/config/schema_app.json @@ -95,13 +95,9 @@ "type": "array", "items": { "type": "string", - "pattern": "^([A-Za-z0-9_]+|[A-Za-z0-9_]+=[0-9]+|[A-Za-z0-9_]+=\\\".*\\\")$" + "pattern": "(^[\\w]+$|^[\\w]+=.+$)" } }, - "__config_path": { - "description": "Path to configuration file", - "type": "string" - }, "artifact_name": { "type": "string" } diff --git a/tools/config/schema_lib.json b/tools/config/schema_lib.json index 70a4f411d13..5d218e17ad8 100644 --- a/tools/config/schema_lib.json +++ b/tools/config/schema_lib.json @@ -95,12 +95,8 @@ "type": "array", "items": { "type": "string", - "pattern": "^([A-Za-z0-9_]+|[A-Za-z0-9_]+=[0-9]+|[A-Za-z0-9_]+=\\\".*\\\")$" + "pattern": "(^[\\w]+$|^[\\w]+=.+$)" } - }, - "__config_path": { - "description": "Path to configuration file", - "type": "string" } }, "required": [ From 151f2fa3a6686d24c87d1a25183e2da4a009f1ff Mon Sep 17 00:00:00 2001 From: Steven Cartmell Date: Wed, 6 Sep 2017 12:55:57 +0100 Subject: [PATCH 4/6] Refactor common JSON schema definitions into a seperate file - Moved all common definitions from schema_app and schema_lib into a separate definitions file. - Changed the calling code to resolve multiple schema files correctly. --- tools/config/__init__.py | 20 +++++--- tools/config/definitions.json | 93 +++++++++++++++++++++++++++++++++++ tools/config/schema_app.json | 89 ++------------------------------- tools/config/schema_lib.json | 89 ++------------------------------- 4 files changed, 115 insertions(+), 176 deletions(-) create mode 100644 tools/config/definitions.json diff --git a/tools/config/__init__.py b/tools/config/__init__.py index b6aa033786f..161246c65b4 100644 --- a/tools/config/__init__.py +++ b/tools/config/__init__.py @@ -25,7 +25,7 @@ from intelhex import IntelHex from jinja2 import FileSystemLoader, StrictUndefined from jinja2.environment import Environment -from jsonschema import validate, Draft4Validator +from jsonschema import Draft4Validator, RefResolver # Implementation of mbed configuration mechanism from tools.utils import json_file_to_dict, intelhex_offset from tools.arm_pack_manager import Cache @@ -408,8 +408,12 @@ def __init__(self, tgt, top_level_dirs=None, app_config=None): % self.app_config_location)) # Validate the format of the JSON file based on the schema_app.json - schema_path = os.path.join(os.path.dirname(__file__), "schema_app.json") - validator = Draft4Validator(json_file_to_dict(schema_path)) + schema_root = os.path.dirname(__file__) + schema_path = os.path.join(schema_root, "schema_app.json") + schema_file = json_file_to_dict(schema_path) + + resolver = RefResolver("file://{}/".format(schema_root), schema_file) + validator = Draft4Validator(schema_file, resolver=resolver) errors = sorted(validator.iter_errors(self.app_config_data)) @@ -466,9 +470,13 @@ def add_config_files(self, flist): continue # Validate the format of the JSON file based on the schema_lib.json - schema_path = os.path.join(os.path.dirname(__file__), - "schema_lib.json") - validator = Draft4Validator(json_file_to_dict(schema_path)) + schema_root = os.path.dirname(__file__) + schema_path = os.path.join(schema_root, "schema_lib.json") + schema_file = json_file_to_dict(schema_path) + + resolver = RefResolver("file://{}/".format(schema_root), + schema_file) + validator = Draft4Validator(schema_file, resolver=resolver) errors = sorted(validator.iter_errors(cfg)) diff --git a/tools/config/definitions.json b/tools/config/definitions.json new file mode 100644 index 00000000000..ed7a4e69ece --- /dev/null +++ b/tools/config/definitions.json @@ -0,0 +1,93 @@ +{ + "name_definition": { + "description": "Name of the library", + "type": "string", + "items": { + "type": "string" + } + }, + "macro_definition": { + "description": "A list of extra macros that will be defined when compiling a project that includes this library.", + "type": "array", + "items": { + "type": "string", + "pattern": "(^[\\w]+$|^[\\w]+=.+$)" + } + }, + "config_definition": { + "description": "List of configuration parameters", + "type": "object", + "patternProperties": { + "^[^ ]+$": { + "$ref": "#/config_parameter_base" + } + }, + "additionalProperties": false + }, + "target_overrides_definition": { + "description": "List of overrides for specific targets", + "type": "object", + "patternProperties": { + "\\*": { + "type": "object", + "patternProperties": { + ".*\\..*": {} + }, + "additionalProperties": false + }, + "^\\S+$": { + "$ref": "#/target_override_entry" + } + }, + "additionalProperties": false + }, + "config_parameter_long": { + "type": "object", + "properties": { + "help": { + "description": "An optional help message that describes the purpose of the parameter", + "type": "string" + }, + "value": { + "description": "An optional field that defines the value of the parameter", + "type": [ + "integer", + "string", + "boolean" + ] + }, + "required": { + "description": "An optional field that specifies whether the parameter must be given a value before compiling the code. (False by default)", + "type": "boolean" + }, + "macro_name": { + "description": "An optional field for the macro defined at compile time for this configuration parameter. The system will automatically figure out the macro name from the configuration parameter, but this field will override it", + "type": "string" + } + } + }, + "config_parameter_short": { + "type": [ + "string", + "integer", + "boolean" + ] + }, + "config_parameter_base": { + "oneOf": [ + { + "$ref": "#/config_parameter_long" + }, + { + "$ref": "#/config_parameter_short" + } + ] + }, + "target_override_entry": { + "type": "object", + "patternProperties": { + "^\\S+$": {} + }, + "additionalProperties": false + } +} \ No newline at end of file diff --git a/tools/config/schema_app.json b/tools/config/schema_app.json index 4fd2611f3f0..0617cda5fa6 100644 --- a/tools/config/schema_app.json +++ b/tools/config/schema_app.json @@ -4,99 +4,18 @@ "description": "Configuration file for an mbed library", "type": "object", "$id": "http://example.com/root.json", - "definitions": { - "config_parameter_long": { - "type": "object", - "properties": { - "help": { - "description": "An optional help message that describes the purpose of the parameter", - "type": "string" - }, - "value": { - "description": "An optional field that defines the value of the parameter", - "type": [ - "integer", - "string", - "boolean" - ] - }, - "required": { - "description": "An optional field that specifies whether the parameter must be given a value before compiling the code. (False by default)", - "type": "boolean" - }, - "macro_name": { - "description": "An optional field for the macro defined at compile time for this configuration parameter. The system will automatically figure out the macro name from the configuration parameter, but this field will override it", - "type": "string" - } - } - }, - "config_parameter_short": { - "type": [ - "string", - "integer", - "boolean" - ] - }, - "config_parameter_base": { - "oneOf": [ - { - "$ref": "#/definitions/config_parameter_long" - }, - { - "$ref": "#/definitions/config_parameter_short" - } - ] - }, - "target_override_entry": { - "type": "object", - "patternProperties": { - "^\\S+$": {} - }, - "additionalProperties": false - } - }, "properties": { "name": { - "description": "Name of the library", - "type": "string", - "items": { - "type": "string" - } + "$ref": "file:definitions.json#/name_definition" }, "config": { - "description": "List of configuration parameters", - "type": "object", - "patternProperties": { - "^[^ ]+$": { - "$ref": "#/definitions/config_parameter_base" - } - }, - "additionalProperties": false + "$ref": "file:definitions.json#/config_definition" }, "target_overrides": { - "description": "List of overrides for specific targets", - "type": "object", - "patternProperties": { - "\\*": { - "type": "object", - "patternProperties": { - ".*\\..*": {} - }, - "additionalProperties": false - }, - "^\\S+$": { - "$ref": "#/definitions/target_override_entry" - } - }, - "additionalProperties": false + "$ref": "file:definitions.json#/target_overrides_definition" }, "macros": { - "description": "A list of extra macros that will be defined when compiling a project that includes this library.", - "type": "array", - "items": { - "type": "string", - "pattern": "(^[\\w]+$|^[\\w]+=.+$)" - } + "$ref": "file:definitions.json#/macro_definition" }, "artifact_name": { "type": "string" diff --git a/tools/config/schema_lib.json b/tools/config/schema_lib.json index 5d218e17ad8..e9213329509 100644 --- a/tools/config/schema_lib.json +++ b/tools/config/schema_lib.json @@ -4,99 +4,18 @@ "description": "Configuration file for an mbed library", "type": "object", "$id": "http://example.com/root.json", - "definitions": { - "config_parameter_long": { - "type": "object", - "properties": { - "help": { - "description": "An optional help message that describes the purpose of the parameter", - "type": "string" - }, - "value": { - "description": "An optional field that defines the value of the parameter", - "type": [ - "integer", - "string", - "boolean" - ] - }, - "required": { - "description": "An optional field that specifies whether the parameter must be given a value before compiling the code. (False by default)", - "type": "boolean" - }, - "macro_name": { - "description": "An optional field for the macro defined at compile time for this configuration parameter. The system will automatically figure out the macro name from the configuration parameter, but this field will override it", - "type": "string" - } - } - }, - "config_parameter_short": { - "type": [ - "string", - "integer", - "boolean" - ] - }, - "config_parameter_base": { - "oneOf": [ - { - "$ref": "#/definitions/config_parameter_long" - }, - { - "$ref": "#/definitions/config_parameter_short" - } - ] - }, - "target_override_entry": { - "type": "object", - "patternProperties": { - "^\\S+$": {} - }, - "additionalProperties": false - } - }, "properties": { "name": { - "description": "Name of the library", - "type": "string", - "items": { - "type": "string" - } + "$ref": "file:definitions.json#/name_definition" }, "config": { - "description": "List of configuration parameters", - "type": "object", - "patternProperties": { - "^[^ ]+$": { - "$ref": "#/definitions/config_parameter_base" - } - }, - "additionalProperties": false + "$ref": "file:definitions.json#/config_definition" }, "target_overrides": { - "description": "List of overrides for specific targets", - "type": "object", - "patternProperties": { - "\\*": { - "type": "object", - "patternProperties": { - ".*\\..*": {} - }, - "additionalProperties": false - }, - "^\\S+$": { - "$ref": "#/definitions/target_override_entry" - } - }, - "additionalProperties": false + "$ref": "file:definitions.json#/target_overrides_definition" }, "macros": { - "description": "A list of extra macros that will be defined when compiling a project that includes this library.", - "type": "array", - "items": { - "type": "string", - "pattern": "(^[\\w]+$|^[\\w]+=.+$)" - } + "$ref": "file:definitions.json#/macro_definition" } }, "required": [ From 7010010ffcaa9f7c9ec59aca641936893c8d7798 Mon Sep 17 00:00:00 2001 From: Steven Cartmell Date: Tue, 12 Sep 2017 11:10:10 +0100 Subject: [PATCH 5/6] Remove placeholder $id from JSON schema and fix description --- tools/config/schema_app.json | 3 +-- tools/config/schema_lib.json | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/config/schema_app.json b/tools/config/schema_app.json index 0617cda5fa6..7596762d4a1 100644 --- a/tools/config/schema_app.json +++ b/tools/config/schema_app.json @@ -1,9 +1,8 @@ { "$schema": "http://json-schema.org/draft-06/schema#", "title": "Mbed Library Schema", - "description": "Configuration file for an mbed library", + "description": "Configuration file for an mbed application", "type": "object", - "$id": "http://example.com/root.json", "properties": { "name": { "$ref": "file:definitions.json#/name_definition" diff --git a/tools/config/schema_lib.json b/tools/config/schema_lib.json index e9213329509..7944d8af309 100644 --- a/tools/config/schema_lib.json +++ b/tools/config/schema_lib.json @@ -3,7 +3,6 @@ "title": "Mbed Library Schema", "description": "Configuration file for an mbed library", "type": "object", - "$id": "http://example.com/root.json", "properties": { "name": { "$ref": "file:definitions.json#/name_definition" From b1ea3881befd953fd99e8404be74a3a591edc281 Mon Sep 17 00:00:00 2001 From: Steven Cartmell Date: Tue, 12 Sep 2017 16:02:17 +0100 Subject: [PATCH 6/6] Fix test errors produced by JSON schema after rebase - Modified the expected error code in some of the test cases. The error message is now issued by the JSON validator. - Stop validation code being called if no application configuration is given to validate. - Change test mock code to check for specific calls, instead of number of calls. - Derive absolute paths for JSON schema files before loading them, which seems to work better with the URI path resolution of the schema parser. --- tools/config/__init__.py | 33 ++++++++++++------- tools/config/definitions.json | 8 +++-- tools/config/schema_app.json | 8 ++--- tools/config/schema_lib.json | 8 ++--- tools/test/config/config_test.py | 7 ++-- tools/test/config/invalid_key/test_data.json | 2 +- .../config/invalid_key_lib/test_data.json | 2 +- 7 files changed, 40 insertions(+), 28 deletions(-) diff --git a/tools/config/__init__.py b/tools/config/__init__.py index 161246c65b4..15a958b83b4 100644 --- a/tools/config/__init__.py +++ b/tools/config/__init__.py @@ -16,7 +16,9 @@ """ from copy import deepcopy +from six import moves import json +import six import os from os.path import dirname, abspath, exists, join, isabs import sys @@ -407,18 +409,23 @@ def __init__(self, tgt, top_level_dirs=None, app_config=None): ConfigException("Could not parse mbed app configuration from %s" % self.app_config_location)) - # Validate the format of the JSON file based on the schema_app.json - schema_root = os.path.dirname(__file__) - schema_path = os.path.join(schema_root, "schema_app.json") - schema_file = json_file_to_dict(schema_path) - resolver = RefResolver("file://{}/".format(schema_root), schema_file) - validator = Draft4Validator(schema_file, resolver=resolver) + if self.app_config_location is not None: + # Validate the format of the JSON file based on schema_app.json + schema_root = os.path.dirname(os.path.abspath(__file__)) + schema_path = os.path.join(schema_root, "schema_app.json") + schema = json_file_to_dict(schema_path) - errors = sorted(validator.iter_errors(self.app_config_data)) + url = moves.urllib.request.pathname2url(schema_path) + uri = moves.urllib_parse.urljoin("file://", url) - if errors: - raise ConfigException(",".join(x.message for x in errors)) + resolver = RefResolver(uri, schema) + validator = Draft4Validator(schema, resolver=resolver) + + errors = sorted(validator.iter_errors(self.app_config_data)) + + if errors: + raise ConfigException(",".join(x.message for x in errors)) # Update the list of targets with the ones defined in the application # config, if applicable @@ -470,12 +477,14 @@ def add_config_files(self, flist): continue # Validate the format of the JSON file based on the schema_lib.json - schema_root = os.path.dirname(__file__) + schema_root = os.path.dirname(os.path.abspath(__file__)) schema_path = os.path.join(schema_root, "schema_lib.json") schema_file = json_file_to_dict(schema_path) - resolver = RefResolver("file://{}/".format(schema_root), - schema_file) + url = moves.urllib.request.pathname2url(schema_path) + uri = moves.urllib_parse.urljoin("file://", url) + + resolver = RefResolver(uri, schema_file) validator = Draft4Validator(schema_file, resolver=resolver) errors = sorted(validator.iter_errors(cfg)) diff --git a/tools/config/definitions.json b/tools/config/definitions.json index ed7a4e69ece..e18d2687e17 100644 --- a/tools/config/definitions.json +++ b/tools/config/definitions.json @@ -53,7 +53,8 @@ "type": [ "integer", "string", - "boolean" + "boolean", + "null" ] }, "required": { @@ -70,7 +71,8 @@ "type": [ "string", "integer", - "boolean" + "boolean", + "null" ] }, "config_parameter_base": { @@ -90,4 +92,4 @@ }, "additionalProperties": false } -} \ No newline at end of file +} diff --git a/tools/config/schema_app.json b/tools/config/schema_app.json index 7596762d4a1..981100ce596 100644 --- a/tools/config/schema_app.json +++ b/tools/config/schema_app.json @@ -5,16 +5,16 @@ "type": "object", "properties": { "name": { - "$ref": "file:definitions.json#/name_definition" + "$ref": "definitions.json#/name_definition" }, "config": { - "$ref": "file:definitions.json#/config_definition" + "$ref": "definitions.json#/config_definition" }, "target_overrides": { - "$ref": "file:definitions.json#/target_overrides_definition" + "$ref": "definitions.json#/target_overrides_definition" }, "macros": { - "$ref": "file:definitions.json#/macro_definition" + "$ref": "definitions.json#/macro_definition" }, "artifact_name": { "type": "string" diff --git a/tools/config/schema_lib.json b/tools/config/schema_lib.json index 7944d8af309..b8556c961e6 100644 --- a/tools/config/schema_lib.json +++ b/tools/config/schema_lib.json @@ -5,16 +5,16 @@ "type": "object", "properties": { "name": { - "$ref": "file:definitions.json#/name_definition" + "$ref": "definitions.json#/name_definition" }, "config": { - "$ref": "file:definitions.json#/config_definition" + "$ref": "definitions.json#/config_definition" }, "target_overrides": { - "$ref": "file:definitions.json#/target_overrides_definition" + "$ref": "definitions.json#/target_overrides_definition" }, "macros": { - "$ref": "file:definitions.json#/macro_definition" + "$ref": "definitions.json#/macro_definition" } }, "required": [ diff --git a/tools/test/config/config_test.py b/tools/test/config/config_test.py index 157dcb72d32..c845daef8b0 100644 --- a/tools/test/config/config_test.py +++ b/tools/test/config/config_test.py @@ -108,7 +108,8 @@ def test_init_app_config(target): config = Config(target, app_config=app_config) - mock_json_file_to_dict.assert_called_with(app_config) + mock_json_file_to_dict.assert_any_call("app_config") + assert config.app_config_data == mock_return @@ -149,7 +150,7 @@ def test_init_no_app_config_with_dir(target): config = Config(target, [directory]) mock_isfile.assert_called_with(path) - mock_json_file_to_dict.assert_called_once_with(path) + mock_json_file_to_dict.assert_any_call(path) assert config.app_config_data == mock_return @@ -171,5 +172,5 @@ def test_init_override_app_config(target): config = Config(target, [directory], app_config=app_config) - mock_json_file_to_dict.assert_called_once_with(app_config) + mock_json_file_to_dict.assert_any_call(app_config) assert config.app_config_data == mock_return diff --git a/tools/test/config/invalid_key/test_data.json b/tools/test/config/invalid_key/test_data.json index 564461ad6e3..c86542f1aab 100644 --- a/tools/test/config/invalid_key/test_data.json +++ b/tools/test/config/invalid_key/test_data.json @@ -1,5 +1,5 @@ { "K64F": { - "exception_msg": "Unknown key(s)" + "exception_msg": "Additional properties are not allowed ('unknown_key' was unexpected)" } } diff --git a/tools/test/config/invalid_key_lib/test_data.json b/tools/test/config/invalid_key_lib/test_data.json index 564461ad6e3..c86542f1aab 100644 --- a/tools/test/config/invalid_key_lib/test_data.json +++ b/tools/test/config/invalid_key_lib/test_data.json @@ -1,5 +1,5 @@ { "K64F": { - "exception_msg": "Unknown key(s)" + "exception_msg": "Additional properties are not allowed ('unknown_key' was unexpected)" } }