Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move file validations out of the config module #5627

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions readthedocs/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@
validate_bool,
validate_choice,
validate_dict,
validate_directory,
validate_file,
validate_path,
validate_list,
validate_string,
)
Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
)
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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'):
Expand Down Expand Up @@ -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'):
Expand Down
94 changes: 1 addition & 93 deletions readthedocs/config/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
{
Expand Down Expand Up @@ -1378,7 +1332,7 @@ def test_python_install_extra_requirements_allow_empty(self, tmpdir):
{
'python': {
'install': [{
'path': '',
'path': '.',
'method': 'pip',
'extra_requirements': [],
}],
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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}},
Expand Down
48 changes: 2 additions & 46 deletions readthedocs/config/tests/test_validation.py
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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):
Expand All @@ -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:

Expand Down
36 changes: 6 additions & 30 deletions readthedocs/config/validation.py
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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',
Expand Down Expand Up @@ -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):
Expand Down