diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index dc36b441d67..eb745d03c7f 100644 --- a/readthedocs/config/config.py +++ b/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,7 +790,7 @@ 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, ) @@ -799,7 +798,7 @@ def validate_python_install(self, index): 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'): diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index cbddfe3914a..5c0db8f8fb6 100644 --- a/readthedocs/config/tests/test_config.py +++ b/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}}, diff --git a/readthedocs/config/tests/test_validation.py b/readthedocs/config/tests/test_validation.py index fc66740ead9..c46331c5d8d 100644 --- a/readthedocs/config/tests/test_validation.py +++ b/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: diff --git a/readthedocs/config/validation.py b/readthedocs/config/validation.py index 788f13d0d8b..095bf444630 100644 --- a/readthedocs/config/validation.py +++ b/readthedocs/config/validation.py @@ -1,4 +1,5 @@ """Validations for the RTD configuration file.""" + import os @@ -6,8 +7,6 @@ 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):