Move extends validation into ServiceLoader class

This refactoring allows us to raise an error when there is no
'file' key specified in the .yml and no self.filename set. This
error was specific to the tests, as the tests are
the only place that constructs service dicts without sometimes
setting a filename.

Moving the function within the class as well as it is code that
is exclusively for the use of validating properties for the
ServiceLoader class.

Signed-off-by: Mazz Mosley <[email protected]>

compose/config.py

@@ -158,7 +158,7 @@ class ServiceLoader(object):
         if 'extends' not in service_dict:
             return service_dict
 
-        extends_options = validate_extends_options(service_dict['name'], service_dict['extends'])
+        extends_options = self.validate_extends_options(service_dict['name'], service_dict['extends'])
 
         if self.working_dir is None:
             raise Exception("No working_dir passed to ServiceLoader()")
@@ -194,25 +194,29 @@ class ServiceLoader(object):
     def signature(self, name):
         return (self.filename, name)
 
+    def validate_extends_options(self, service_name, extends_options):
+        error_prefix = "Invalid 'extends' configuration for %s:" % service_name
 
-def validate_extends_options(service_name, extends_options):
-    error_prefix = "Invalid 'extends' configuration for %s:" % service_name
+        if not isinstance(extends_options, dict):
+            raise ConfigurationError("%s must be a dictionary" % error_prefix)
 
-    if not isinstance(extends_options, dict):
-        raise ConfigurationError("%s must be a dictionary" % error_prefix)
-
-    if 'service' not in extends_options:
-        raise ConfigurationError(
-            "%s you need to specify a service, e.g. 'service: web'" % error_prefix
-        )
+        if 'service' not in extends_options:
+            raise ConfigurationError(
+                "%s you need to specify a service, e.g. 'service: web'" % error_prefix
+            )
 
-    for k, _ in extends_options.items():
-        if k not in ['file', 'service']:
+        if 'file' not in extends_options and self.filename is None:
             raise ConfigurationError(
-                "%s unsupported configuration option '%s'" % (error_prefix, k)
+                "%s you need to specify a 'file', e.g. 'file: something.yml'" % error_prefix
             )
 
-    return extends_options
+        for k, _ in extends_options.items():
+            if k not in ['file', 'service']:
+                raise ConfigurationError(
+                    "%s unsupported configuration option '%s'" % (error_prefix, k)
+                )
+
+        return extends_options
 
 
 def validate_extended_service_dict(service_dict, filename, service):

tests/unit/config_test.py

@@ -459,6 +459,14 @@ class ExtendsTest(unittest.TestCase):
 
         self.assertRaisesRegexp(config.ConfigurationError, 'what', load_config)
 
+    def test_extends_validation_no_file_key_no_filename_set(self):
+        dictionary = {'extends': {'service': 'web'}}
+
+        def load_config():
+            return config.make_service_dict('myweb', dictionary, working_dir='tests/fixtures/extends')
+
+        self.assertRaisesRegexp(config.ConfigurationError, 'file', load_config)
+
     def test_extends_validation_valid_config(self):
         dictionary = {'extends': {'service': 'web', 'file': 'common.yml'}}