Merge pull request #5627 from stsewd/move-file-validations-out-of-the-config-module

Move file validations out of the config module

Author: ericholscher

readthedocs/config/config.py

@@ -30,8 +30,7 @@
     validate_bool,
     validate_choice,
     validate_dict,
-    validate_directory,
-    validate_file,
+    validate_path,
     validate_list,
     validate_string,
 )
@@ -499,7 +498,7 @@ def validate_conda(self):
             with self.catch_validation_error('conda.file'):
                 if 'file' not in raw_conda:
                     raise ValidationError('file', VALUE_NOT_FOUND)
-                conda_environment = validate_file(
+                conda_environment = validate_path(
                     raw_conda['file'],
                     self.base_path,
                 )
@@ -516,7 +515,7 @@ def validate_requirements_file(self):
         if not requirements_file:
             return None
         with self.catch_validation_error('requirements_file'):
-            requirements_file = validate_file(
+            requirements_file = validate_path(
                 requirements_file,
                 self.base_path,
             )
@@ -685,7 +684,7 @@ def validate_conda(self):
         conda = {}
         with self.catch_validation_error('conda.environment'):
             environment = self.pop_config('conda.environment', raise_ex=True)
-            conda['environment'] = validate_file(environment, self.base_path)
+            conda['environment'] = validate_path(environment, self.base_path)
         return conda
 
     def validate_build(self):
@@ -791,15 +790,15 @@ def validate_python_install(self, index):
         if 'requirements' in raw_install:
             requirements_key = key + '.requirements'
             with self.catch_validation_error(requirements_key):
-                requirements = validate_file(
+                requirements = validate_path(
                     self.pop_config(requirements_key),
                     self.base_path,
                 )
                 python_install['requirements'] = requirements
         elif 'path' in raw_install:
             path_key = key + '.path'
             with self.catch_validation_error(path_key):
-                path = validate_directory(
+                path = validate_path(
                     self.pop_config(path_key),
                     self.base_path,
                 )
@@ -876,7 +875,7 @@ def validate_mkdocs(self):
         with self.catch_validation_error('mkdocs.configuration'):
             configuration = self.pop_config('mkdocs.configuration', None)
             if configuration is not None:
-                configuration = validate_file(configuration, self.base_path)
+                configuration = validate_path(configuration, self.base_path)
             mkdocs['configuration'] = configuration
 
         with self.catch_validation_error('mkdocs.fail_on_warning'):
@@ -923,7 +922,7 @@ def validate_sphinx(self):
                 configuration,
             )
             if configuration is not None:
-                configuration = validate_file(configuration, self.base_path)
+                configuration = validate_path(configuration, self.base_path)
             sphinx['configuration'] = configuration
 
         with self.catch_validation_error('sphinx.fail_on_warning'):

readthedocs/config/tests/test_config.py

@@ -860,16 +860,6 @@ def test_conda_check_valid(self, tmpdir):
         build.validate()
         assert build.conda.environment == 'environment.yml'
 
-    def test_conda_check_invalid(self, tmpdir):
-        apply_fs(tmpdir, {'environment.yml': ''})
-        build = self.get_build_config(
-            {'conda': {'environment': 'no_existing_environment.yml'}},
-            source_file=str(tmpdir.join('readthedocs.yml')),
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'conda.environment'
-
     @pytest.mark.parametrize('value', [3, [], 'invalid'])
     def test_conda_check_invalid_value(self, value):
         build = self.get_build_config({'conda': value})
@@ -1057,22 +1047,6 @@ def test_python_install_check_default(self, tmpdir):
         assert install[0].method == PIP
         assert install[0].extra_requirements == []
 
-    def test_python_install_path_check_invalid(self, tmpdir):
-        build = self.get_build_config(
-            {
-                'python': {
-                    'install': [{
-                        'path': 'noexists',
-                        'method': 'pip',
-                    }],
-                },
-            },
-            source_file=str(tmpdir.join('readthedocs.yml')),
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'python.install.0.path'
-
     @pytest.mark.parametrize('value', ['invalid', 'apt'])
     def test_python_install_method_check_invalid(self, value, tmpdir):
         build = self.get_build_config(
@@ -1108,26 +1082,6 @@ def test_python_install_requirements_check_valid(self, tmpdir):
         assert isinstance(install[0], PythonInstallRequirements)
         assert install[0].requirements == 'requirements.txt'
 
-    def test_python_install_requirements_check_invalid(self, tmpdir):
-        apply_fs(tmpdir, {'requirements.txt': ''})
-        requirements_file = 'invalid'
-        build = self.get_build_config(
-            {
-                'python': {
-                    'install': [{
-                        'path': '.',
-                        'requirements': requirements_file,
-                    }],
-                },
-            },
-            source_file=str(tmpdir.join('readthedocs.yml')),
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'python.install.0.requirements'
-        error_msg = 'path {} does not exist'.format(requirements_file)
-        assert error_msg in str(excinfo.value)
-
     def test_python_install_requirements_does_not_allow_null(self, tmpdir):
         build = self.get_build_config(
             {
@@ -1378,7 +1332,7 @@ def test_python_install_extra_requirements_allow_empty(self, tmpdir):
             {
                 'python': {
                     'install': [{
-                        'path': '',
+                        'path': '.',
                         'method': 'pip',
                         'extra_requirements': [],
                     }],
@@ -1427,32 +1381,6 @@ def test_python_install_several_respects_order(self, tmpdir):
 
         assert install[2].requirements == 'three.txt'
 
-    def test_python_install_reports_correct_invalid_index(self, tmpdir):
-        apply_fs(tmpdir, {
-            'one': {},
-            'two': {},
-        })
-        build = self.get_build_config(
-            {
-                'python': {
-                    'install': [{
-                        'path': 'one',
-                        'method': 'pip',
-                        'extra_requirements': [],
-                    }, {
-                        'path': 'two',
-                        'method': 'setuptools',
-                    }, {
-                        'requirements': 'three.txt',
-                    }],
-                },
-            },
-            source_file=str(tmpdir.join('readthedocs.yml')),
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'python.install.2.requirements'
-
     @pytest.mark.parametrize('value', [True, False])
     def test_python_system_packages_check_valid(self, value):
         build = self.get_build_config({
@@ -1565,16 +1493,6 @@ def test_sphinx_configuration_check_valid(self, tmpdir):
         build.validate()
         assert build.sphinx.configuration == 'conf.py'
 
-    def test_sphinx_configuration_check_invalid(self, tmpdir):
-        apply_fs(tmpdir, {'conf.py': ''})
-        build = self.get_build_config(
-            {'sphinx': {'configuration': 'invalid.py'}},
-            source_file=str(tmpdir.join('readthedocs.yml')),
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'sphinx.configuration'
-
     def test_sphinx_cant_be_used_with_mkdocs(self, tmpdir):
         apply_fs(tmpdir, {'conf.py': ''})
         build = self.get_build_config(
@@ -1677,16 +1595,6 @@ def test_mkdocs_configuration_check_valid(self, tmpdir):
         assert build.doctype == 'mkdocs'
         assert build.sphinx is None
 
-    def test_mkdocs_configuration_check_invalid(self, tmpdir):
-        apply_fs(tmpdir, {'mkdocs.yml': ''})
-        build = self.get_build_config(
-            {'mkdocs': {'configuration': 'invalid.yml'}},
-            source_file=str(tmpdir.join('readthedocs.yml')),
-        )
-        with raises(InvalidConfig) as excinfo:
-            build.validate()
-        assert excinfo.value.key == 'mkdocs.configuration'
-
     def test_mkdocs_configuration_allow_null(self):
         build = self.get_build_config(
             {'mkdocs': {'configuration': None}},

readthedocs/config/tests/test_validation.py

@@ -1,19 +1,16 @@
+import os
+
 from mock import patch
 from pytest import raises
 
 from readthedocs.config.validation import (
     INVALID_BOOL,
     INVALID_CHOICE,
-    INVALID_DIRECTORY,
-    INVALID_FILE,
     INVALID_LIST,
-    INVALID_PATH,
     INVALID_STRING,
     ValidationError,
     validate_bool,
     validate_choice,
-    validate_directory,
-    validate_file,
     validate_list,
     validate_path,
     validate_string,
@@ -80,42 +77,6 @@ def test_it_rejects_string_types(self):
         assert excinfo.value.code == INVALID_LIST
 
 
-class TestValidateDirectory:
-
-    def test_it_uses_validate_path(self, tmpdir):
-        patcher = patch('readthedocs.config.validation.validate_path')
-        with patcher as validate_path:
-            path = str(tmpdir.mkdir('a directory'))
-            validate_path.return_value = path
-            validate_directory(path, str(tmpdir))
-            validate_path.assert_called_with(path, str(tmpdir))
-
-    def test_it_rejects_files(self, tmpdir):
-        tmpdir.join('file').write('content')
-        with raises(ValidationError) as excinfo:
-            validate_directory('file', str(tmpdir))
-        assert excinfo.value.code == INVALID_DIRECTORY
-
-
-class TestValidateFile:
-
-    def test_it_uses_validate_path(self, tmpdir):
-        patcher = patch('readthedocs.config.validation.validate_path')
-        with patcher as validate_path:
-            path = tmpdir.join('a file')
-            path.write('content')
-            path = str(path)
-            validate_path.return_value = path
-            validate_file(path, str(tmpdir))
-            validate_path.assert_called_with(path, str(tmpdir))
-
-    def test_it_rejects_directories(self, tmpdir):
-        tmpdir.mkdir('directory')
-        with raises(ValidationError) as excinfo:
-            validate_file('directory', str(tmpdir))
-        assert excinfo.value.code == INVALID_FILE
-
-
 class TestValidatePath:
 
     def test_it_accepts_relative_path(self, tmpdir):
@@ -140,11 +101,6 @@ def test_it_only_accepts_strings(self):
             validate_path(None, '')
         assert excinfo.value.code == INVALID_STRING
 
-    def test_it_rejects_non_existent_path(self, tmpdir):
-        with raises(ValidationError) as excinfo:
-            validate_path('does not exist', str(tmpdir))
-        assert excinfo.value.code == INVALID_PATH
-
 
 class TestValidateString:
 

readthedocs/config/validation.py

@@ -1,13 +1,12 @@
 """Validations for the RTD configuration file."""
+
 import os
 
 
 INVALID_BOOL = 'invalid-bool'
 INVALID_CHOICE = 'invalid-choice'
 INVALID_LIST = 'invalid-list'
 INVALID_DICT = 'invalid-dictionary'
-INVALID_DIRECTORY = 'invalid-directory'
-INVALID_FILE = 'invalid-file'
 INVALID_PATH = 'invalid-path'
 INVALID_STRING = 'invalid-string'
 VALUE_NOT_FOUND = 'value-not-found'
@@ -20,8 +19,6 @@ class ValidationError(Exception):
     messages = {
         INVALID_BOOL: 'expected one of (0, 1, true, false), got {value}',
         INVALID_CHOICE: 'expected one of ({choices}), got {value}',
-        INVALID_DIRECTORY: '{value} is not a directory',
-        INVALID_FILE: '{value} is not a file',
         INVALID_DICT: '{value} is not a dictionary',
         INVALID_PATH: 'path {value} does not exist',
         INVALID_STRING: 'expected string',
@@ -77,35 +74,14 @@ def validate_bool(value):
     return bool(value)
 
 
-def validate_directory(value, base_path):
-    """Check that ``value`` is a directory."""
-    path = os.path.join(
-        base_path,
-        validate_path(value, base_path)
-    )
-    if not os.path.isdir(path):
-        raise ValidationError(value, INVALID_DIRECTORY)
-    return os.path.relpath(path, base_path)
-
-
-def validate_file(value, base_path):
-    """Check that ``value`` is a file."""
-    path = os.path.join(
-        base_path,
-        validate_path(value, base_path)
-    )
-    if not os.path.isfile(path):
-        raise ValidationError(value, INVALID_FILE)
-    return os.path.relpath(path, base_path)
-
-
 def validate_path(value, base_path):
-    """Check that ``value`` is an existent file in ``base_path``."""
+    """Check that ``value`` is a valid path name and normamlize it."""
     string_value = validate_string(value)
-    pathed_value = os.path.join(base_path, string_value)
-    if not os.path.exists(pathed_value):
+    if not string_value:
         raise ValidationError(value, INVALID_PATH)
-    return os.path.relpath(pathed_value, base_path)
+    full_path = os.path.join(base_path, string_value)
+    rel_path = os.path.relpath(full_path, base_path)
+    return rel_path
 
 
 def validate_string(value):