Browse Source

Merge pull request #2355 from dnephin/refactor_handle_error

Refactor process_error and include a filename as part of the validation error messages
Daniel Nephin 10 years ago
parent
commit
9f6a5a964a

+ 2 - 3
compose/config/config.py

@@ -65,7 +65,6 @@ ALLOWED_KEYS = DOCKER_CONFIG_KEYS + [
     'dockerfile',
     'expose',
     'external_links',
-    'name',
 ]
 
 
@@ -229,9 +228,9 @@ def load(config_details):
 
 
 def process_config_file(config_file, service_name=None):
-    validate_top_level_object(config_file.config)
+    validate_top_level_object(config_file)
     processed_config = interpolate_environment_variables(config_file.config)
-    validate_against_fields_schema(processed_config)
+    validate_against_fields_schema(processed_config, config_file.filename)
 
     if service_name and service_name not in processed_config:
         raise ConfigurationError(

+ 4 - 2
compose/config/fields_schema.json

@@ -2,15 +2,18 @@
   "$schema": "http://json-schema.org/draft-04/schema#",
 
   "type": "object",
+  "id": "fields_schema.json",
 
   "patternProperties": {
     "^[a-zA-Z0-9._-]+$": {
       "$ref": "#/definitions/service"
     }
   },
+  "additionalProperties": false,
 
   "definitions": {
     "service": {
+      "id": "#/definitions/service",
       "type": "object",
 
       "properties": {
@@ -167,6 +170,5 @@
       ]
     }
 
-  },
-  "additionalProperties": false
+  }
 }

+ 0 - 7
compose/config/interpolation.py

@@ -18,13 +18,6 @@ def interpolate_environment_variables(config):
 
 
 def process_service(service_name, service_dict, mapping):
-    if not isinstance(service_dict, dict):
-        raise ConfigurationError(
-            'Service "%s" doesn\'t have any configuration options. '
-            'All top level keys in your docker-compose.yml must map '
-            'to a dictionary of configuration options.' % service_name
-        )
-
     return dict(
         (key, interpolate_value(service_name, key, val, mapping))
         for (key, val) in service_dict.items()

+ 4 - 7
compose/config/service_schema.json

@@ -1,15 +1,17 @@
 {
   "$schema": "http://json-schema.org/draft-04/schema#",
+  "id": "service_schema.json",
 
   "type": "object",
 
   "allOf": [
     {"$ref": "fields_schema.json#/definitions/service"},
-    {"$ref": "#/definitions/service_constraints"}
+    {"$ref": "#/definitions/constraints"}
   ],
 
   "definitions": {
-    "service_constraints": {
+    "constraints": {
+      "id": "#/definitions/constraints",
       "anyOf": [
         {
           "required": ["build"],
@@ -21,13 +23,8 @@
             {"required": ["build"]},
             {"required": ["dockerfile"]}
           ]}
-        },
-        {
-          "required": ["extends"],
-          "not": {"required": ["build", "image"]}
         }
       ]
     }
   }
-
 }

+ 190 - 183
compose/config/validation.py

@@ -66,21 +66,38 @@ def format_boolean_in_environment(instance):
     return True
 
 
-def validate_service_names(config):
-    for service_name in config.keys():
+def validate_top_level_service_objects(config_file):
+    """Perform some high level validation of the service name and value.
+
+    This validation must happen before interpolation, which must happen
+    before the rest of validation, which is why it's separate from the
+    rest of the service validation.
+    """
+    for service_name, service_dict in config_file.config.items():
         if not isinstance(service_name, six.string_types):
             raise ConfigurationError(
-                "Service name: {} needs to be a string, eg '{}'".format(
+                "In file '{}' service name: {} needs to be a string, eg '{}'".format(
+                    config_file.filename,
                     service_name,
                     service_name))
 
+        if not isinstance(service_dict, dict):
+            raise ConfigurationError(
+                "In file '{}' service '{}' doesn\'t have any configuration options. "
+                "All top level keys in your docker-compose.yml must map "
+                "to a dictionary of configuration options.".format(
+                    config_file.filename,
+                    service_name))
+
 
-def validate_top_level_object(config):
-    if not isinstance(config, dict):
+def validate_top_level_object(config_file):
+    if not isinstance(config_file.config, dict):
         raise ConfigurationError(
-            "Top level object needs to be a dictionary. Check your .yml file "
-            "that you have defined a service at the top level.")
-    validate_service_names(config)
+            "Top level object in '{}' needs to be an object not '{}'. Check "
+            "that you have defined a service at the top level.".format(
+                config_file.filename,
+                type(config_file.config)))
+    validate_top_level_service_objects(config_file)
 
 
 def validate_extends_file_path(service_name, extends_options, filename):
@@ -109,189 +126,171 @@ def anglicize_validator(validator):
     return 'a ' + validator
 
 
-def process_errors(errors, service_name=None):
+def handle_error_for_schema_with_id(error, service_name):
+    schema_id = error.schema['id']
+
+    if schema_id == 'fields_schema.json' and error.validator == 'additionalProperties':
+        return "Invalid service name '{}' - only {} characters are allowed".format(
+            # The service_name is the key to the json object
+            list(error.instance)[0],
+            VALID_NAME_CHARS)
+
+    if schema_id == '#/definitions/constraints':
+        if 'image' in error.instance and 'build' in error.instance:
+            return (
+                "Service '{}' has both an image and build path specified. "
+                "A service can either be built to image or use an existing "
+                "image, not both.".format(service_name))
+        if 'image' not in error.instance and 'build' not in error.instance:
+            return (
+                "Service '{}' has neither an image nor a build path "
+                "specified. Exactly one must be provided.".format(service_name))
+        if 'image' in error.instance and 'dockerfile' in error.instance:
+            return (
+                "Service '{}' has both an image and alternate Dockerfile. "
+                "A service can either be built to image or use an existing "
+                "image, not both.".format(service_name))
+
+    if schema_id == '#/definitions/service':
+        if error.validator == 'additionalProperties':
+            invalid_config_key = parse_key_from_error_msg(error)
+            return get_unsupported_config_msg(service_name, invalid_config_key)
+
+
+def handle_generic_service_error(error, service_name):
+    config_key = " ".join("'%s'" % k for k in error.path)
+    msg_format = None
+    error_msg = error.message
+
+    if error.validator == 'oneOf':
+        msg_format = "Service '{}' configuration key {} {}"
+        error_msg = _parse_oneof_validator(error)
+
+    elif error.validator == 'type':
+        msg_format = ("Service '{}' configuration key {} contains an invalid "
+                      "type, it should be {}")
+        error_msg = _parse_valid_types_from_validator(error.validator_value)
+
+    # TODO: no test case for this branch, there are no config options
+    # which exercise this branch
+    elif error.validator == 'required':
+        msg_format = "Service '{}' configuration key '{}' is invalid, {}"
+
+    elif error.validator == 'dependencies':
+        msg_format = "Service '{}' configuration key '{}' is invalid: {}"
+        config_key = list(error.validator_value.keys())[0]
+        required_keys = ",".join(error.validator_value[config_key])
+        error_msg = "when defining '{}' you must set '{}' as well".format(
+            config_key,
+            required_keys)
+
+    elif error.path:
+        msg_format = "Service '{}' configuration key {} value {}"
+
+    if msg_format:
+        return msg_format.format(service_name, config_key, error_msg)
+
+    return error.message
+
+
+def parse_key_from_error_msg(error):
+    return error.message.split("'")[1]
+
+
+def _parse_valid_types_from_validator(validator):
+    """A validator value can be either an array of valid types or a string of
+    a valid type. Parse the valid types and prefix with the correct article.
     """
-    jsonschema gives us an error tree full of information to explain what has
-    gone wrong. Process each error and pull out relevant information and re-write
-    helpful error messages that are relevant.
+    if not isinstance(validator, list):
+        return anglicize_validator(validator)
+
+    if len(validator) == 1:
+        return anglicize_validator(validator[0])
+
+    return "{}, or {}".format(
+        ", ".join([anglicize_validator(validator[0])] + validator[1:-1]),
+        anglicize_validator(validator[-1]))
+
+
+def _parse_oneof_validator(error):
+    """oneOf has multiple schemas, so we need to reason about which schema, sub
+    schema or constraint the validation is failing on.
+    Inspecting the context value of a ValidationError gives us information about
+    which sub schema failed and which kind of error it is.
     """
-    def _parse_key_from_error_msg(error):
-        return error.message.split("'")[1]
-
-    def _clean_error_message(message):
-        return message.replace("u'", "'")
-
-    def _parse_valid_types_from_validator(validator):
-        """
-        A validator value can be either an array of valid types or a string of
-        a valid type. Parse the valid types and prefix with the correct article.
-        """
-        if isinstance(validator, list):
-            if len(validator) >= 2:
-                first_type = anglicize_validator(validator[0])
-                last_type = anglicize_validator(validator[-1])
-                types_from_validator = ", ".join([first_type] + validator[1:-1])
-
-                msg = "{} or {}".format(
-                    types_from_validator,
-                    last_type
-                )
-            else:
-                msg = "{}".format(anglicize_validator(validator[0]))
-        else:
-            msg = "{}".format(anglicize_validator(validator))
-
-        return msg
-
-    def _parse_oneof_validator(error):
-        """
-        oneOf has multiple schemas, so we need to reason about which schema, sub
-        schema or constraint the validation is failing on.
-        Inspecting the context value of a ValidationError gives us information about
-        which sub schema failed and which kind of error it is.
-        """
-        required = [context for context in error.context if context.validator == 'required']
-        if required:
-            return required[0].message
-
-        additionalProperties = [context for context in error.context if context.validator == 'additionalProperties']
-        if additionalProperties:
-            invalid_config_key = _parse_key_from_error_msg(additionalProperties[0])
+    types = []
+    for context in error.context:
+
+        if context.validator == 'required':
+            return context.message
+
+        if context.validator == 'additionalProperties':
+            invalid_config_key = parse_key_from_error_msg(context)
             return "contains unsupported option: '{}'".format(invalid_config_key)
 
-        constraint = [context for context in error.context if len(context.path) > 0]
-        if constraint:
-            valid_types = _parse_valid_types_from_validator(constraint[0].validator_value)
-            invalid_config_key = "".join(
-                "'{}' ".format(fragment) for fragment in constraint[0].path
+        if context.path:
+            invalid_config_key = " ".join(
+                "'{}' ".format(fragment) for fragment in context.path
                 if isinstance(fragment, six.string_types)
             )
-            msg = "{}contains {}, which is an invalid type, it should be {}".format(
+            return "{}contains {}, which is an invalid type, it should be {}".format(
                 invalid_config_key,
-                constraint[0].instance,
-                valid_types
-            )
-            return msg
+                context.instance,
+                _parse_valid_types_from_validator(context.validator_value))
 
-        uniqueness = [context for context in error.context if context.validator == 'uniqueItems']
-        if uniqueness:
-            msg = "contains non unique items, please remove duplicates from {}".format(
-                uniqueness[0].instance
-            )
-            return msg
-
-        types = [context.validator_value for context in error.context if context.validator == 'type']
-        valid_types = _parse_valid_types_from_validator(types)
-
-        msg = "contains an invalid type, it should be {}".format(valid_types)
-
-        return msg
-
-    root_msgs = []
-    invalid_keys = []
-    required = []
-    type_errors = []
-    other_errors = []
-
-    for error in errors:
-        # handle root level errors
-        if len(error.path) == 0 and not service_name:
-            if error.validator == 'type':
-                msg = "Top level object needs to be a dictionary. Check your .yml file that you have defined a service at the top level."
-                root_msgs.append(msg)
-            elif error.validator == 'additionalProperties':
-                invalid_service_name = _parse_key_from_error_msg(error)
-                msg = "Invalid service name '{}' - only {} characters are allowed".format(invalid_service_name, VALID_NAME_CHARS)
-                root_msgs.append(msg)
-            else:
-                root_msgs.append(_clean_error_message(error.message))
-
-        else:
-            if not service_name:
-                # field_schema errors will have service name on the path
-                service_name = error.path[0]
-                error.path.popleft()
-            else:
-                # service_schema errors have the service name passed in, as that
-                # is not available on error.path or necessarily error.instance
-                service_name = service_name
-
-            if error.validator == 'additionalProperties':
-                invalid_config_key = _parse_key_from_error_msg(error)
-                invalid_keys.append(get_unsupported_config_msg(service_name, invalid_config_key))
-            elif error.validator == 'anyOf':
-                if 'image' in error.instance and 'build' in error.instance:
-                    required.append(
-                        "Service '{}' has both an image and build path specified. "
-                        "A service can either be built to image or use an existing "
-                        "image, not both.".format(service_name))
-                elif 'image' not in error.instance and 'build' not in error.instance:
-                    required.append(
-                        "Service '{}' has neither an image nor a build path "
-                        "specified. Exactly one must be provided.".format(service_name))
-                elif 'image' in error.instance and 'dockerfile' in error.instance:
-                    required.append(
-                        "Service '{}' has both an image and alternate Dockerfile. "
-                        "A service can either be built to image or use an existing "
-                        "image, not both.".format(service_name))
-                else:
-                    required.append(_clean_error_message(error.message))
-            elif error.validator == 'oneOf':
-                config_key = error.path[0]
-                msg = _parse_oneof_validator(error)
-
-                type_errors.append("Service '{}' configuration key '{}' {}".format(
-                    service_name, config_key, msg)
-                )
-            elif error.validator == 'type':
-                msg = _parse_valid_types_from_validator(error.validator_value)
-
-                if len(error.path) > 0:
-                    config_key = " ".join(["'%s'" % k for k in error.path])
-                    type_errors.append(
-                        "Service '{}' configuration key {} contains an invalid "
-                        "type, it should be {}".format(
-                            service_name,
-                            config_key,
-                            msg))
-                else:
-                    root_msgs.append(
-                        "Service \"{}\" doesn't have any configuration options. "
-                        "All top level keys in your docker-compose.yml must map "
-                        "to a dictionary of configuration options.'".format(service_name))
-            elif error.validator == 'required':
-                config_key = error.path[0]
-                required.append(
-                    "Service '{}' option '{}' is invalid, {}".format(
-                        service_name,
-                        config_key,
-                        _clean_error_message(error.message)))
-            elif error.validator == 'dependencies':
-                dependency_key = list(error.validator_value.keys())[0]
-                required_keys = ",".join(error.validator_value[dependency_key])
-                required.append("Invalid '{}' configuration for '{}' service: when defining '{}' you must set '{}' as well".format(
-                    dependency_key, service_name, dependency_key, required_keys))
-            else:
-                config_key = " ".join(["'%s'" % k for k in error.path])
-                err_msg = "Service '{}' configuration key {} value {}".format(service_name, config_key, error.message)
-                other_errors.append(err_msg)
-
-    return "\n".join(root_msgs + invalid_keys + required + type_errors + other_errors)
-
-
-def validate_against_fields_schema(config):
-    schema_filename = "fields_schema.json"
-    format_checkers = ["ports", "environment"]
-    return _validate_against_schema(config, schema_filename, format_checkers)
+        if context.validator == 'uniqueItems':
+            return "contains non unique items, please remove duplicates from {}".format(
+                context.instance)
 
+        if context.validator == 'type':
+            types.append(context.validator_value)
 
-def validate_against_service_schema(config, service_name):
-    schema_filename = "service_schema.json"
-    format_checkers = ["ports"]
-    return _validate_against_schema(config, schema_filename, format_checkers, service_name)
+    valid_types = _parse_valid_types_from_validator(types)
+    return "contains an invalid type, it should be {}".format(valid_types)
+
+
+def process_errors(errors, service_name=None):
+    """jsonschema gives us an error tree full of information to explain what has
+    gone wrong. Process each error and pull out relevant information and re-write
+    helpful error messages that are relevant.
+    """
+    def format_error_message(error, service_name):
+        if not service_name and error.path:
+            # field_schema errors will have service name on the path
+            service_name = error.path.popleft()
+
+        if 'id' in error.schema:
+            error_msg = handle_error_for_schema_with_id(error, service_name)
+            if error_msg:
+                return error_msg
+
+        return handle_generic_service_error(error, service_name)
+
+    return '\n'.join(format_error_message(error, service_name) for error in errors)
 
 
-def _validate_against_schema(config, schema_filename, format_checker=[], service_name=None):
+def validate_against_fields_schema(config, filename):
+    _validate_against_schema(
+        config,
+        "fields_schema.json",
+        format_checker=["ports", "environment"],
+        filename=filename)
+
+
+def validate_against_service_schema(config, service_name):
+    _validate_against_schema(
+        config,
+        "service_schema.json",
+        format_checker=["ports"],
+        service_name=service_name)
+
+
+def _validate_against_schema(
+        config,
+        schema_filename,
+        format_checker=(),
+        service_name=None,
+        filename=None):
     config_source_dir = os.path.dirname(os.path.abspath(__file__))
 
     if sys.platform == "win32":
@@ -307,9 +306,17 @@ def _validate_against_schema(config, schema_filename, format_checker=[], service
         schema = json.load(schema_fh)
 
     resolver = RefResolver(resolver_full_path, schema)
-    validation_output = Draft4Validator(schema, resolver=resolver, format_checker=FormatChecker(format_checker))
+    validation_output = Draft4Validator(
+        schema,
+        resolver=resolver,
+        format_checker=FormatChecker(format_checker))
 
     errors = [error for error in sorted(validation_output.iter_errors(config), key=str)]
-    if errors:
-        error_msg = process_errors(errors, service_name)
-        raise ConfigurationError("Validation failed, reason(s):\n{}".format(error_msg))
+    if not errors:
+        return
+
+    error_msg = process_errors(errors, service_name)
+    file_msg = " in file '{}'".format(filename) if filename else ''
+    raise ConfigurationError("Validation failed{}, reason(s):\n{}".format(
+        file_msg,
+        error_msg))

+ 67 - 61
tests/unit/config/config_test.py

@@ -78,14 +78,12 @@ class ConfigTest(unittest.TestCase):
 
     def test_config_invalid_service_names(self):
         for invalid_name in ['?not?allowed', ' ', '', '!', '/', '\xe2']:
-            with pytest.raises(ConfigurationError):
-                config.load(
-                    build_config_details(
-                        {invalid_name: {'image': 'busybox'}},
-                        'working_dir',
-                        'filename.yml'
-                    )
-                )
+            with pytest.raises(ConfigurationError) as exc:
+                config.load(build_config_details(
+                    {invalid_name: {'image': 'busybox'}},
+                    'working_dir',
+                    'filename.yml'))
+            assert 'Invalid service name \'%s\'' % invalid_name in exc.exconly()
 
     def test_load_with_invalid_field_name(self):
         config_details = build_config_details(
@@ -96,9 +94,21 @@ class ConfigTest(unittest.TestCase):
             config.load(config_details)
         error_msg = "Unsupported config option for 'web' service: 'name'"
         assert error_msg in exc.exconly()
+        assert "Validation failed in file 'filename.yml'" in exc.exconly()
+
+    def test_load_invalid_service_definition(self):
+        config_details = build_config_details(
+            {'web': 'wrong'},
+            'working_dir',
+            'filename.yml')
+        with pytest.raises(ConfigurationError) as exc:
+            config.load(config_details)
+        error_msg = "service 'web' doesn't have any configuration options"
+        assert error_msg in exc.exconly()
 
     def test_config_integer_service_name_raise_validation_error(self):
-        expected_error_msg = "Service name: 1 needs to be a string, eg '1'"
+        expected_error_msg = ("In file 'filename.yml' service name: 1 needs to "
+                              "be a string, eg '1'")
         with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
             config.load(
                 build_config_details(
@@ -148,25 +158,26 @@ class ConfigTest(unittest.TestCase):
 
     def test_load_with_multiple_files_and_empty_override(self):
         base_file = config.ConfigFile(
-            'base.yaml',
+            'base.yml',
             {'web': {'image': 'example/web'}})
-        override_file = config.ConfigFile('override.yaml', None)
+        override_file = config.ConfigFile('override.yml', None)
         details = config.ConfigDetails('.', [base_file, override_file])
 
         with pytest.raises(ConfigurationError) as exc:
             config.load(details)
-        assert 'Top level object needs to be a dictionary' in exc.exconly()
+        error_msg = "Top level object in 'override.yml' needs to be an object"
+        assert error_msg in exc.exconly()
 
     def test_load_with_multiple_files_and_empty_base(self):
-        base_file = config.ConfigFile('base.yaml', None)
+        base_file = config.ConfigFile('base.yml', None)
         override_file = config.ConfigFile(
-            'override.yaml',
+            'override.yml',
             {'web': {'image': 'example/web'}})
         details = config.ConfigDetails('.', [base_file, override_file])
 
         with pytest.raises(ConfigurationError) as exc:
             config.load(details)
-        assert 'Top level object needs to be a dictionary' in exc.exconly()
+        assert "Top level object in 'base.yml' needs to be an object" in exc.exconly()
 
     def test_load_with_multiple_files_and_extends_in_override_file(self):
         base_file = config.ConfigFile(
@@ -217,17 +228,17 @@ class ConfigTest(unittest.TestCase):
 
         with pytest.raises(ConfigurationError) as exc:
             config.load(details)
-        assert 'Service "bogus" doesn\'t have any configuration' in exc.exconly()
+        assert "service 'bogus' doesn't have any configuration" in exc.exconly()
+        assert "In file 'override.yaml'" in exc.exconly()
 
     def test_config_valid_service_names(self):
         for valid_name in ['_', '-', '.__.', '_what-up.', 'what_.up----', 'whatup']:
-            config.load(
+            services = config.load(
                 build_config_details(
                     {valid_name: {'image': 'busybox'}},
                     'tests/fixtures/extends',
-                    'common.yml'
-                )
-            )
+                    'common.yml'))
+            assert services[0]['name'] == valid_name
 
     def test_config_invalid_ports_format_validation(self):
         expected_error_msg = "Service 'web' configuration key 'ports' contains an invalid type"
@@ -292,7 +303,8 @@ class ConfigTest(unittest.TestCase):
             )
 
     def test_invalid_config_not_a_dictionary(self):
-        expected_error_msg = "Top level object needs to be a dictionary."
+        expected_error_msg = ("Top level object in 'filename.yml' needs to be "
+                              "an object.")
         with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
             config.load(
                 build_config_details(
@@ -374,12 +386,13 @@ class ConfigTest(unittest.TestCase):
             )
 
     def test_config_ulimits_invalid_keys_validation_error(self):
-        expected_error_msg = "Service 'web' configuration key 'ulimits' contains unsupported option: 'not_soft_or_hard'"
+        expected = ("Service 'web' configuration key 'ulimits' 'nofile' contains "
+                    "unsupported option: 'not_soft_or_hard'")
 
-        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
-            config.load(
-                build_config_details(
-                    {'web': {
+        with pytest.raises(ConfigurationError) as exc:
+            config.load(build_config_details(
+                {
+                    'web': {
                         'image': 'busybox',
                         'ulimits': {
                             'nofile': {
@@ -388,50 +401,43 @@ class ConfigTest(unittest.TestCase):
                                 "hard": 20000,
                             }
                         }
-                    }},
-                    'working_dir',
-                    'filename.yml'
-                )
-            )
+                    }
+                },
+                'working_dir',
+                'filename.yml'))
+        assert expected in exc.exconly()
 
     def test_config_ulimits_required_keys_validation_error(self):
-        expected_error_msg = "Service 'web' configuration key 'ulimits' u?'hard' is a required property"
 
-        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
-            config.load(
-                build_config_details(
-                    {'web': {
+        with pytest.raises(ConfigurationError) as exc:
+            config.load(build_config_details(
+                {
+                    'web': {
                         'image': 'busybox',
-                        'ulimits': {
-                            'nofile': {
-                                "soft": 10000,
-                            }
-                        }
-                    }},
-                    'working_dir',
-                    'filename.yml'
-                )
-            )
+                        'ulimits': {'nofile': {"soft": 10000}}
+                    }
+                },
+                'working_dir',
+                'filename.yml'))
+        assert "Service 'web' configuration key 'ulimits' 'nofile'" in exc.exconly()
+        assert "'hard' is a required property" in exc.exconly()
 
     def test_config_ulimits_soft_greater_than_hard_error(self):
-        expected_error_msg = "cannot contain a 'soft' value higher than 'hard' value"
+        expected = "cannot contain a 'soft' value higher than 'hard' value"
 
-        with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):
-            config.load(
-                build_config_details(
-                    {'web': {
+        with pytest.raises(ConfigurationError) as exc:
+            config.load(build_config_details(
+                {
+                    'web': {
                         'image': 'busybox',
                         'ulimits': {
-                            'nofile': {
-                                "soft": 10000,
-                                "hard": 1000
-                            }
+                            'nofile': {"soft": 10000, "hard": 1000}
                         }
-                    }},
-                    'working_dir',
-                    'filename.yml'
-                )
-            )
+                    }
+                },
+                'working_dir',
+                'filename.yml'))
+        assert expected in exc.exconly()
 
     def test_valid_config_which_allows_two_type_definitions(self):
         expose_values = [["8000"], [8000]]
@@ -857,7 +863,7 @@ class MemoryOptionsTest(unittest.TestCase):
         a mem_limit
         """
         expected_error_msg = (
-            "Invalid 'memswap_limit' configuration for 'foo' service: when "
+            "Service 'foo' configuration key 'memswap_limit' is invalid: when "
             "defining 'memswap_limit' you must set 'mem_limit' as well"
         )
         with self.assertRaisesRegexp(ConfigurationError, expected_error_msg):

+ 2 - 0
tox.ini

@@ -43,4 +43,6 @@ directory = coverage-html
 [flake8]
 # Allow really long lines for now
 max-line-length = 140
+# Set this high for now
+max-complexity = 20
 exclude = compose/packages