From 8348cf75c805f3989dbc3770b98b088bca0c5f7c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Jun 2018 12:25:02 -0500 Subject: [PATCH 01/12] Move tests for config.py --- readthedocs/config/tests/test_config.py | 582 ++++++++++++++++++++++++ 1 file changed, 582 insertions(+) create mode 100644 readthedocs/config/tests/test_config.py diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py new file mode 100644 index 00000000000..d181795defc --- /dev/null +++ b/readthedocs/config/tests/test_config.py @@ -0,0 +1,582 @@ +from mock import patch +from mock import DEFAULT +from pytest import raises +import os + +from ..utils import apply_fs +from .config import ConfigError +from .config import InvalidConfig +from .config import load +from .config import BuildConfig +from .config import ProjectConfig +from .config import TYPE_REQUIRED +from .config import NAME_REQUIRED +from .config import NAME_INVALID +from .config import PYTHON_INVALID +from .validation import INVALID_BOOL +from .validation import INVALID_CHOICE +from .validation import INVALID_DIRECTORY +from .validation import INVALID_LIST +from .validation import INVALID_PATH +from .validation import INVALID_STRING + + +env_config = { + 'output_base': '/tmp' +} + + +minimal_config = { + 'name': 'docs', + 'type': 'sphinx', +} + + +config_with_explicit_empty_list = { + 'readthedocs.yml': ''' +name: docs +type: sphinx +formats: [] +''' +} + + +minimal_config_dir = { + 'readthedocs.yml': '''\ +name: docs +type: sphinx +''' +} + + +multiple_config_dir = { + 'readthedocs.yml': ''' +name: first +type: sphinx +--- +name: second +type: sphinx + ''', + 'nested': minimal_config_dir, +} + + +def get_build_config(config, env_config=None, source_file='readthedocs.yml', + source_position=0): + return BuildConfig( + env_config or {}, + config, + source_file=source_file, + source_position=source_position) + + +def test_load_no_config_file(tmpdir): + base = str(tmpdir) + with raises(ConfigError): + load(base, env_config) + + +def test_load_empty_config_file(tmpdir): + apply_fs(tmpdir, { + 'readthedocs.yml': '' + }) + base = str(tmpdir) + with raises(ConfigError): + load(base, env_config) + + +def test_minimal_config(tmpdir): + apply_fs(tmpdir, minimal_config_dir) + base = str(tmpdir) + config = load(base, env_config) + assert isinstance(config, ProjectConfig) + assert len(config) == 1 + build = config[0] + assert isinstance(build, BuildConfig) + + +def test_build_config_has_source_file(tmpdir): + base = str(apply_fs(tmpdir, minimal_config_dir)) + build = load(base, env_config)[0] + assert build.source_file == os.path.join(base, 'readthedocs.yml') + assert build.source_position == 0 + + +def test_build_config_has_source_position(tmpdir): + base = str(apply_fs(tmpdir, multiple_config_dir)) + builds = load(base, env_config) + assert len(builds) == 2 + first, second = filter( + lambda b: not b.source_file.endswith('nested/readthedocs.yml'), + builds) + assert first.source_position == 0 + assert second.source_position == 1 + + +def test_build_config_has_list_with_single_null_value(tmpdir): + base = str(apply_fs(tmpdir, config_with_explicit_empty_list)) + build = load(base, env_config)[0] + assert isinstance(build, BuildConfig) + assert 'formats' not in build + + +def test_config_requires_name(): + build = BuildConfig({}, + {}, + source_file=None, + source_position=None) + with raises(InvalidConfig) as excinfo: + build.validate_name() + assert excinfo.value.key == 'name' + assert excinfo.value.code == NAME_REQUIRED + + +def test_build_requires_valid_name(): + build = BuildConfig({}, + {'name': 'with/slashes'}, + source_file=None, + source_position=None) + with raises(InvalidConfig) as excinfo: + build.validate_name() + assert excinfo.value.key == 'name' + assert excinfo.value.code == NAME_INVALID + + +def test_config_requires_type(): + build = BuildConfig({}, + {'name': 'docs'}, + source_file=None, + source_position=None) + with raises(InvalidConfig) as excinfo: + build.validate_type() + assert excinfo.value.key == 'type' + assert excinfo.value.code == TYPE_REQUIRED + + +def test_build_requires_valid_type(): + build = BuildConfig({}, + {'type': 'unknown'}, + source_file=None, + source_position=None) + with raises(InvalidConfig) as excinfo: + build.validate_type() + assert excinfo.value.key == 'type' + assert excinfo.value.code == INVALID_CHOICE + + +def test_empty_python_section_is_valid(): + build = get_build_config({'python': {}}) + build.validate_python() + assert 'python' in build + + +def test_python_section_must_be_dict(): + build = get_build_config({'python': 123}) + with raises(InvalidConfig) as excinfo: + build.validate_python() + assert excinfo.value.key == 'python' + assert excinfo.value.code == PYTHON_INVALID + + +def test_use_system_site_packages_defaults_to_false(): + build = get_build_config({'python': {}}) + build.validate_python() + # Default is False. + assert not build['python']['use_system_site_packages'] + + +def test_python_pip_install_default(): + build = get_build_config({'python': {}}) + build.validate_python() + # Default is False. + assert build['python']['pip_install'] is False + + +def describe_validate_python_extra_requirements(): + + def it_defaults_to_list(): + build = get_build_config({'python': {}}) + build.validate_python() + # Default is an empty list. + assert build['python']['extra_requirements'] == [] + + def it_validates_is_a_list(): + build = get_build_config( + {'python': {'extra_requirements': 'invalid'}}) + with raises(InvalidConfig) as excinfo: + build.validate_python() + assert excinfo.value.key == 'python.extra_requirements' + assert excinfo.value.code == PYTHON_INVALID + + @patch('readthedocs_build.config.config.validate_string') + def it_uses_validate_string(validate_string): + validate_string.return_value = True + build = get_build_config( + {'python': {'extra_requirements': ['tests']}}) + build.validate_python() + validate_string.assert_any_call('tests') + + +def describe_validate_use_system_site_packages(): + def it_defaults_to_false(): + build = get_build_config({'python': {}}) + build.validate_python() + assert build['python']['setup_py_install'] is False + + def it_validates_value(): + build = get_build_config( + {'python': {'use_system_site_packages': 'invalid'}}) + with raises(InvalidConfig) as excinfo: + build.validate_python() + excinfo.value.key = 'python.use_system_site_packages' + excinfo.value.code = INVALID_BOOL + + @patch('readthedocs_build.config.config.validate_bool') + def it_uses_validate_bool(validate_bool): + validate_bool.return_value = True + build = get_build_config( + {'python': {'use_system_site_packages': 'to-validate'}}) + build.validate_python() + validate_bool.assert_any_call('to-validate') + + +def describe_validate_setup_py_install(): + + def it_defaults_to_false(): + build = get_build_config({'python': {}}) + build.validate_python() + assert build['python']['setup_py_install'] is False + + def it_validates_value(): + build = get_build_config({'python': {'setup_py_install': 'this-is-string'}}) + with raises(InvalidConfig) as excinfo: + build.validate_python() + assert excinfo.value.key == 'python.setup_py_install' + assert excinfo.value.code == INVALID_BOOL + + @patch('readthedocs_build.config.config.validate_bool') + def it_uses_validate_bool(validate_bool): + validate_bool.return_value = True + build = get_build_config( + {'python': {'setup_py_install': 'to-validate'}}) + build.validate_python() + validate_bool.assert_any_call('to-validate') + + +def describe_validate_python_version(): + + def it_defaults_to_a_valid_version(): + build = get_build_config({'python': {}}) + build.validate_python() + assert build['python']['version'] is 2 + + def it_supports_other_versions(): + build = get_build_config({'python': {'version': 3.5}}) + build.validate_python() + assert build['python']['version'] is 3.5 + + def it_validates_versions_out_of_range(): + build = get_build_config({'python': {'version': 1.0}}) + with raises(InvalidConfig) as excinfo: + build.validate_python() + assert excinfo.value.key == 'python.version' + assert excinfo.value.code == INVALID_CHOICE + + def it_validates_wrong_type(): + build = get_build_config({'python': {'version': 'this-is-string'}}) + with raises(InvalidConfig) as excinfo: + build.validate_python() + assert excinfo.value.key == 'python.version' + assert excinfo.value.code == INVALID_CHOICE + + def it_validates_wrong_type_right_value(): + build = get_build_config({'python': {'version': '3.5'}}) + build.validate_python() + assert build['python']['version'] == 3.5 + + build = get_build_config({'python': {'version': '3'}}) + build.validate_python() + assert build['python']['version'] == 3 + + def it_validates_env_supported_versions(): + build = get_build_config( + {'python': {'version': 3.6}}, + env_config={'python': {'supported_versions': [3.5]}} + ) + with raises(InvalidConfig) as excinfo: + build.validate_python() + assert excinfo.value.key == 'python.version' + assert excinfo.value.code == INVALID_CHOICE + + build = get_build_config( + {'python': {'version': 3.6}}, + env_config={'python': {'supported_versions': [3.5, 3.6]}} + ) + build.validate_python() + assert build['python']['version'] == 3.6 + + +def describe_validate_formats(): + + def it_defaults_to_not_being_included(): + build = get_build_config({}) + build.validate_formats() + assert 'formats' not in build + + def it_gets_set_correctly(): + build = get_build_config({'formats': ['pdf']}) + build.validate_formats() + assert build['formats'] == ['pdf'] + + def formats_can_be_empty(): + build = get_build_config({'formats': []}) + build.validate_formats() + assert 'formats' not in build + + def all_valid_formats(): + build = get_build_config({'formats': ['pdf', 'htmlzip', 'epub']}) + build.validate_formats() + assert build['formats'] == ['pdf', 'htmlzip', 'epub'] + + def cant_have_none_as_format(): + build = get_build_config({'formats': ['htmlzip', None]}) + with raises(InvalidConfig) as excinfo: + build.validate_formats() + assert excinfo.value.key == 'format' + assert excinfo.value.code == INVALID_CHOICE + + def formats_have_only_allowed_values(): + build = get_build_config({'formats': ['htmlzip', 'csv']}) + with raises(InvalidConfig) as excinfo: + build.validate_formats() + assert excinfo.value.key == 'format' + assert excinfo.value.code == INVALID_CHOICE + + def only_list_type(): + build = get_build_config({'formats': None}) + with raises(InvalidConfig) as excinfo: + build.validate_formats() + assert excinfo.value.key == 'format' + assert excinfo.value.code == INVALID_LIST + + +def describe_validate_setup_py_path(): + + def it_defaults_to_source_file_directory(tmpdir): + with tmpdir.as_cwd(): + source_file = tmpdir.join('subdir', 'readthedocs.yml') + setup_py = tmpdir.join('subdir', 'setup.py') + build = get_build_config({}, source_file=str(source_file)) + build.validate_python() + assert build['python']['setup_py_path'] == str(setup_py) + + def it_validates_value(tmpdir): + with tmpdir.as_cwd(): + build = get_build_config({'python': {'setup_py_path': 'this-is-string'}}) + with raises(InvalidConfig) as excinfo: + build.validate_python() + assert excinfo.value.key == 'python.setup_py_path' + assert excinfo.value.code == INVALID_PATH + + def it_uses_validate_file(tmpdir): + path = tmpdir.join('setup.py') + path.write('content') + path = str(path) + patcher = patch('readthedocs_build.config.config.validate_file') + with patcher as validate_file: + validate_file.return_value = path + build = get_build_config( + {'python': {'setup_py_path': 'setup.py'}}) + build.validate_python() + args, kwargs = validate_file.call_args + assert args[0] == 'setup.py' + + +def test_valid_build_config(): + build = BuildConfig(env_config, + minimal_config, + source_file='readthedocs.yml', + source_position=0) + build.validate() + assert build['name'] == 'docs' + assert build['type'] == 'sphinx' + assert build['base'] + assert build['python'] + assert 'setup_py_install' in build['python'] + assert 'use_system_site_packages' in build['python'] + assert build['output_base'] + + +def describe_validate_base(): + + def it_validates_to_abspath(tmpdir): + apply_fs(tmpdir, {'configs': minimal_config, 'docs': {}}) + with tmpdir.as_cwd(): + source_file = str(tmpdir.join('configs', 'readthedocs.yml')) + build = BuildConfig( + {}, + {'base': '../docs'}, + source_file=source_file, + source_position=0) + build.validate_base() + assert build['base'] == str(tmpdir.join('docs')) + + @patch('readthedocs_build.config.config.validate_directory') + def it_uses_validate_directory(validate_directory): + validate_directory.return_value = 'path' + build = get_build_config({'base': '../my-path'}) + build.validate_base() + # Test for first argument to validate_directory + args, kwargs = validate_directory.call_args + assert args[0] == '../my-path' + + def it_fails_if_base_is_not_a_string(tmpdir): + apply_fs(tmpdir, minimal_config) + with tmpdir.as_cwd(): + build = BuildConfig( + {}, + {'base': 1}, + source_file=str(tmpdir.join('readthedocs.yml')), + source_position=0) + with raises(InvalidConfig) as excinfo: + build.validate_base() + assert excinfo.value.key == 'base' + assert excinfo.value.code == INVALID_STRING + + def it_fails_if_base_does_not_exist(tmpdir): + apply_fs(tmpdir, minimal_config) + build = BuildConfig( + {}, + {'base': 'docs'}, + source_file=str(tmpdir.join('readthedocs.yml')), + source_position=0) + with raises(InvalidConfig) as excinfo: + build.validate_base() + assert excinfo.value.key == 'base' + assert excinfo.value.code == INVALID_PATH + + +def describe_validate_build(): + + def it_fails_if_build_is_invalid_option(tmpdir): + apply_fs(tmpdir, minimal_config) + build = BuildConfig( + {}, + {'build': {'image': 3.0}}, + source_file=str(tmpdir.join('readthedocs.yml')), + source_position=0) + with raises(InvalidConfig) as excinfo: + build.validate_build() + assert excinfo.value.key == 'build' + assert excinfo.value.code == INVALID_CHOICE + + def it_fails_on_python_validation(tmpdir): + apply_fs(tmpdir, minimal_config) + build = BuildConfig( + {}, + { + 'build': {'image': 1.0}, + 'python': {'version': '3.3'}, + }, + source_file=str(tmpdir.join('readthedocs.yml')), + source_position=0) + build.validate_build() + with raises(InvalidConfig) as excinfo: + build.validate_python() + assert excinfo.value.key == 'python.version' + assert excinfo.value.code == INVALID_CHOICE + + def it_works_on_python_validation(tmpdir): + apply_fs(tmpdir, minimal_config) + build = BuildConfig( + {}, + { + 'build': {'image': 'latest'}, + 'python': {'version': '3.3'}, + }, + source_file=str(tmpdir.join('readthedocs.yml')), + source_position=0) + build.validate_build() + build.validate_python() + + def it_works(tmpdir): + apply_fs(tmpdir, minimal_config) + build = BuildConfig( + {}, + {'build': {'image': 'latest'}}, + source_file=str(tmpdir.join('readthedocs.yml')), + source_position=0) + build.validate_build() + assert build['build']['image'] == 'readthedocs/build:latest' + + def default(tmpdir): + apply_fs(tmpdir, minimal_config) + build = BuildConfig( + {}, + {}, + source_file=str(tmpdir.join('readthedocs.yml')), + source_position=0) + build.validate_build() + assert build['build']['image'] == 'readthedocs/build:2.0' + + +def test_build_validate_calls_all_subvalidators(tmpdir): + apply_fs(tmpdir, minimal_config) + build = BuildConfig( + {}, + {}, + source_file=str(tmpdir.join('readthedocs.yml')), + source_position=0) + with patch.multiple(BuildConfig, + validate_base=DEFAULT, + validate_name=DEFAULT, + validate_type=DEFAULT, + validate_python=DEFAULT, + validate_output_base=DEFAULT): + build.validate() + BuildConfig.validate_base.assert_called_with() + BuildConfig.validate_name.assert_called_with() + BuildConfig.validate_type.assert_called_with() + BuildConfig.validate_python.assert_called_with() + BuildConfig.validate_output_base.assert_called_with() + + +def test_validate_project_config(): + with patch.object(BuildConfig, 'validate') as build_validate: + project = ProjectConfig([ + BuildConfig( + env_config, + minimal_config, + source_file='readthedocs.yml', + source_position=0) + ]) + project.validate() + assert build_validate.call_count == 1 + + +def test_load_calls_validate(tmpdir): + apply_fs(tmpdir, minimal_config_dir) + base = str(tmpdir) + with patch.object(BuildConfig, 'validate') as build_validate: + load(base, env_config) + assert build_validate.call_count == 1 + + +def test_project_set_output_base(): + project = ProjectConfig([ + BuildConfig( + env_config, + minimal_config, + source_file='readthedocs.yml', + source_position=0), + BuildConfig( + env_config, + minimal_config, + source_file='readthedocs.yml', + source_position=1), + ]) + project.set_output_base('random') + for build_config in project: + assert ( + build_config['output_base'] == os.path.join(os.getcwd(), 'random')) + From 91ea31e6e5a6478d3b5a2f16a4d8d08825e350b5 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Jun 2018 13:04:47 -0500 Subject: [PATCH 02/12] Fix imports --- readthedocs/config/__init__.py | 2 + readthedocs/config/config.py | 59 +++++++++++++++++++++++++ readthedocs/config/tests/test_config.py | 32 +++++++------- 3 files changed, 77 insertions(+), 16 deletions(-) create mode 100644 readthedocs/config/config.py diff --git a/readthedocs/config/__init__.py b/readthedocs/config/__init__.py index e69de29bb2d..314f6c394cc 100644 --- a/readthedocs/config/__init__.py +++ b/readthedocs/config/__init__.py @@ -0,0 +1,2 @@ +from .config import * # noqa +from .parser import * # noqa diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py new file mode 100644 index 00000000000..bc862e839d0 --- /dev/null +++ b/readthedocs/config/config.py @@ -0,0 +1,59 @@ +from contextlib import contextmanager +import re +import os + +from .find import find_one +from .parser import ParseError +from .parser import parse +from .validation import (ValidationError, validate_bool, validate_choice, + validate_directory, validate_file, validate_list, + validate_string) + + +__all__ = ( + 'load', 'BuildConfig', 'ConfigError', 'InvalidConfig', 'ProjectConfig') + + +CONFIG_FILENAMES = () + + +BASE_INVALID = '' +BASE_NOT_A_DIR = '' +CONFIG_SYNTAX_INVALID = '' +CONFIG_REQUIRED = '' +NAME_REQUIRED = '' +NAME_INVALID = '' +CONF_FILE_REQUIRED = '' +TYPE_REQUIRED = '' +PYTHON_INVALID = '' + +DOCKER_DEFAULT_IMAGE = '' +DOCKER_DEFAULT_VERSION = '' +# These map to coordisponding settings in the .org, +# so they haven't been renamed. +DOCKER_IMAGE = '{}:{}'.format(DOCKER_DEFAULT_IMAGE, DOCKER_DEFAULT_VERSION) +DOCKER_IMAGE_SETTINGS = {} + + +class ConfigError(Exception): + + def __init__(self, message, code): + self.code = code + super(ConfigError, self).__init__(message) + + +class InvalidConfig(ConfigError): + pass + + +class BuildConfig(dict): + pass + + +class ProjectConfig(list): + pass + + +def load(path, env_config): + pass + diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index d181795defc..4fa3cb7c0dc 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -3,22 +3,22 @@ from pytest import raises import os -from ..utils import apply_fs -from .config import ConfigError -from .config import InvalidConfig -from .config import load -from .config import BuildConfig -from .config import ProjectConfig -from .config import TYPE_REQUIRED -from .config import NAME_REQUIRED -from .config import NAME_INVALID -from .config import PYTHON_INVALID -from .validation import INVALID_BOOL -from .validation import INVALID_CHOICE -from .validation import INVALID_DIRECTORY -from .validation import INVALID_LIST -from .validation import INVALID_PATH -from .validation import INVALID_STRING +from .utils import apply_fs +from readthedocs.config import ConfigError +from readthedocs.config import InvalidConfig +from readthedocs.config import load +from readthedocs.config import BuildConfig +from readthedocs.config import ProjectConfig +from readthedocs.config.config import TYPE_REQUIRED +from readthedocs.config.config import NAME_REQUIRED +from readthedocs.config.config import NAME_INVALID +from readthedocs.config.config import PYTHON_INVALID +from readthedocs.config.validation import INVALID_BOOL +from readthedocs.config.validation import INVALID_CHOICE +from readthedocs.config.validation import INVALID_DIRECTORY +from readthedocs.config.validation import INVALID_LIST +from readthedocs.config.validation import INVALID_PATH +from readthedocs.config.validation import INVALID_STRING env_config = { From 637e4bc585e4b31f5742695ef9efa68e7df70a00 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Jun 2018 13:13:19 -0500 Subject: [PATCH 03/12] Move conf.py file from rtd-build --- readthedocs/config/config.py | 440 +++++++++++++++++++++++++++++++++-- 1 file changed, 423 insertions(+), 17 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index bc862e839d0..c84294e8911 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -14,25 +14,35 @@ 'load', 'BuildConfig', 'ConfigError', 'InvalidConfig', 'ProjectConfig') -CONFIG_FILENAMES = () +CONFIG_FILENAMES = ('readthedocs.yml', '.readthedocs.yml') -BASE_INVALID = '' -BASE_NOT_A_DIR = '' -CONFIG_SYNTAX_INVALID = '' -CONFIG_REQUIRED = '' -NAME_REQUIRED = '' -NAME_INVALID = '' -CONF_FILE_REQUIRED = '' -TYPE_REQUIRED = '' -PYTHON_INVALID = '' +BASE_INVALID = 'base-invalid' +BASE_NOT_A_DIR = 'base-not-a-directory' +CONFIG_SYNTAX_INVALID = 'config-syntax-invalid' +CONFIG_REQUIRED = 'config-required' +NAME_REQUIRED = 'name-required' +NAME_INVALID = 'name-invalid' +CONF_FILE_REQUIRED = 'conf-file-required' +TYPE_REQUIRED = 'type-required' +PYTHON_INVALID = 'python-invalid' -DOCKER_DEFAULT_IMAGE = '' -DOCKER_DEFAULT_VERSION = '' +DOCKER_DEFAULT_IMAGE = 'readthedocs/build' +DOCKER_DEFAULT_VERSION = '2.0' # These map to coordisponding settings in the .org, # so they haven't been renamed. DOCKER_IMAGE = '{}:{}'.format(DOCKER_DEFAULT_IMAGE, DOCKER_DEFAULT_VERSION) -DOCKER_IMAGE_SETTINGS = {} +DOCKER_IMAGE_SETTINGS = { + 'readthedocs/build:1.0': { + 'python': {'supported_versions': [2, 2.7, 3, 3.4]}, + }, + 'readthedocs/build:2.0': { + 'python': {'supported_versions': [2, 2.7, 3, 3.5]}, + }, + 'readthedocs/build:latest': { + 'python': {'supported_versions': [2, 2.7, 3, 3.3, 3.4, 3.5, 3.6]}, + }, +} class ConfigError(Exception): @@ -43,17 +53,413 @@ def __init__(self, message, code): class InvalidConfig(ConfigError): - pass + message_template = 'Invalid "{key}": {error}' + + def __init__(self, key, code, error_message, source_file=None, + source_position=None): + self.key = key + self.code = code + self.source_file = source_file + self.source_position = source_position + message = self.message_template.format( + key=key, + code=code, + error=error_message) + super(InvalidConfig, self).__init__(message, code=code) class BuildConfig(dict): - pass + + """ + Config that handles the build of one particular documentation. Config keys + can be accessed with a dictionary lookup:: + + >>> build_config['type'] + 'sphinx' + + You need to call ``validate`` before the config is ready to use. Also + setting the ``output_base`` is required before using it for a build. + """ + + BASE_INVALID_MESSAGE = 'Invalid value for base: {base}' + BASE_NOT_A_DIR_MESSAGE = '"base" is not a directory: {base}' + NAME_REQUIRED_MESSAGE = 'Missing key "name"' + NAME_INVALID_MESSAGE = ( + 'Invalid name "{name}". Valid values must match {name_re}') + TYPE_REQUIRED_MESSAGE = 'Missing key "type"' + CONF_FILE_REQUIRED_MESSAGE = 'Missing key "conf_file"' + PYTHON_INVALID_MESSAGE = '"python" section must be a mapping.' + PYTHON_EXTRA_REQUIREMENTS_INVALID_MESSAGE = ( + '"python.extra_requirements" section must be a list.') + + PYTHON_SUPPORTED_VERSIONS = [2, 2.7, 3, 3.5] + DOCKER_SUPPORTED_VERSIONS = ['1.0', '2.0', 'latest'] + + def __init__(self, env_config, raw_config, source_file, source_position): + self.env_config = env_config + self.raw_config = raw_config + self.source_file = source_file + self.source_position = source_position + + def error(self, key, message, code): + source = '{file} [{pos}]'.format( + file=self.source_file, + pos=self.source_position) + raise InvalidConfig( + key=key, + code=code, + error_message='{source}: {message}'.format(source=source, + message=message), + source_file=self.source_file, + source_position=self.source_position) + + @contextmanager + def catch_validation_error(self, key): + try: + yield + except ValidationError as error: + raise InvalidConfig( + key=key, + code=error.code, + error_message=str(error), + source_file=self.source_file, + source_position=self.source_position) + + def get_valid_types(self): + return ( + 'sphinx', + ) + + def get_valid_python_versions(self): + try: + return self.env_config['python']['supported_versions'] + except (KeyError, TypeError): + pass + return self.PYTHON_SUPPORTED_VERSIONS + + def get_valid_formats(self): + return ( + 'htmlzip', + 'pdf', + 'epub', + ) + + def validate(self): + """ + Validate and process config into ``config`` attribute that contains the + ready to use build configuration. + + It makes sure that: + + - ``type`` is set and is a valid builder + - ``base`` is a valid directory and defaults to the directory of the + ``readthedocs.yml`` config file if not set + """ + + # Validate env_config. + self.validate_output_base() + + # Validate the build environment first + self.validate_build() # Must happen before `validate_python`! + + # Validate raw_config. Order matters. + self.validate_name() + self.validate_type() + self.validate_base() + self.validate_python() + self.validate_formats() + + self.validate_conda() + self.validate_requirements_file() + self.validate_conf_file() + + def validate_output_base(self): + assert 'output_base' in self.env_config, ( + '"output_base" required in "env_config"') + base_path = os.path.dirname(self.source_file) + self['output_base'] = os.path.abspath( + os.path.join( + self.env_config.get('output_base', base_path), + ) + ) + + def validate_name(self): + name = self.raw_config.get('name', None) + if not name: + name = self.env_config.get('name', None) + if not name: + self.error('name', self.NAME_REQUIRED_MESSAGE, code=NAME_REQUIRED) + name_re = r'^[-_.0-9a-zA-Z]+$' + if not re.match(name_re, name): + self.error( + 'name', + self.NAME_INVALID_MESSAGE.format( + name=name, + name_re=name_re), + code=NAME_INVALID) + + self['name'] = name + + def validate_type(self): + type = self.raw_config.get('type', None) + if not type: + type = self.env_config.get('type', None) + if not type: + self.error('type', self.TYPE_REQUIRED_MESSAGE, code=TYPE_REQUIRED) + + with self.catch_validation_error('type'): + validate_choice(type, self.get_valid_types()) + + self['type'] = type + + def validate_base(self): + if 'base' in self.raw_config: + base = self.raw_config['base'] + else: + base = os.path.dirname(self.source_file) + with self.catch_validation_error('base'): + base_path = os.path.dirname(self.source_file) + base = validate_directory(base, base_path) + self['base'] = base + + def validate_build(self): + """ + Validate the build config settings. + + This is a bit complex, + so here is the logic: + + * We take the default image & version if it's specific in the environment + * Then update the _version_ from the users config + * Then append the default _image_, since users can't change this + * Then update the env_config with the settings for that specific image + - This is currently used for a build image -> python version mapping + + This means we can use custom docker _images_, + but can't change the supported _versions_ that users have defined. + """ + # Defaults + if 'build' in self.env_config: + build = self.env_config['build'] + else: + build = {'image': DOCKER_IMAGE} + + # User specified + if 'build' in self.raw_config: + _build = self.raw_config['build'] + if 'image' in _build: + with self.catch_validation_error('build'): + build['image'] = validate_choice( + str(_build['image']), + self.DOCKER_SUPPORTED_VERSIONS, + ) + if ':' not in build['image']: + # Prepend proper image name to user's image name + build['image'] = '{}:{}'.format( + DOCKER_DEFAULT_IMAGE, + build['image'] + ) + # Update docker default settings from image name + if build['image'] in DOCKER_IMAGE_SETTINGS: + self.env_config.update( + DOCKER_IMAGE_SETTINGS[build['image']] + ) + # Update docker settings from user config + if 'DOCKER_IMAGE_SETTINGS' in self.env_config and \ + build['image'] in self.env_config['DOCKER_IMAGE_SETTINGS']: + self.env_config.update( + self.env_config['DOCKER_IMAGE_SETTINGS'][build['image']] + ) + self['build'] = build + + def validate_python(self): + python = { + 'use_system_site_packages': False, + 'pip_install': False, + 'extra_requirements': [], + 'setup_py_install': False, + 'setup_py_path': os.path.join( + os.path.dirname(self.source_file), + 'setup.py'), + 'version': 2, + } + + if 'python' in self.raw_config: + raw_python = self.raw_config['python'] + if not isinstance(raw_python, dict): + self.error( + 'python', + self.PYTHON_INVALID_MESSAGE, + code=PYTHON_INVALID) + + # Validate use_system_site_packages. + if 'use_system_site_packages' in raw_python: + with self.catch_validation_error( + 'python.use_system_site_packages'): + python['use_system_site_packages'] = validate_bool( + raw_python['use_system_site_packages']) + + # Validate pip_install. + if 'pip_install' in raw_python: + with self.catch_validation_error('python.pip_install'): + python['pip_install'] = validate_bool( + raw_python['pip_install']) + + # Validate extra_requirements. + if 'extra_requirements' in raw_python: + raw_extra_requirements = raw_python['extra_requirements'] + if not isinstance(raw_extra_requirements, list): + self.error( + 'python.extra_requirements', + self.PYTHON_EXTRA_REQUIREMENTS_INVALID_MESSAGE, + code=PYTHON_INVALID) + for extra_name in raw_extra_requirements: + with self.catch_validation_error( + 'python.extra_requirements'): + python['extra_requirements'].append( + validate_string(extra_name)) + + # Validate setup_py_install. + if 'setup_py_install' in raw_python: + with self.catch_validation_error('python.setup_py_install'): + python['setup_py_install'] = validate_bool( + raw_python['setup_py_install']) + + # Validate setup_py_path. + if 'setup_py_path' in raw_python: + with self.catch_validation_error('python.setup_py_path'): + base_path = os.path.dirname(self.source_file) + python['setup_py_path'] = validate_file( + raw_python['setup_py_path'], base_path) + + if 'version' in raw_python: + with self.catch_validation_error('python.version'): + # Try to convert strings to an int first, to catch '2', then + # a float, to catch '2.7' + version = raw_python['version'] + if isinstance(version, str): + try: + version = int(version) + except ValueError: + try: + version = float(version) + except ValueError: + pass + python['version'] = validate_choice( + version, + self.get_valid_python_versions(), + ) + + self['python'] = python + + def validate_conda(self): + conda = {} + + if 'conda' in self.raw_config: + raw_conda = self.raw_config['conda'] + if not isinstance(raw_conda, dict): + self.error( + 'conda', + self.PYTHON_INVALID_MESSAGE, + code=PYTHON_INVALID) + + if 'file' in raw_conda: + with self.catch_validation_error('conda.file'): + base_path = os.path.dirname(self.source_file) + conda['file'] = validate_file( + raw_conda['file'], base_path) + + self['conda'] = conda + + def validate_requirements_file(self): + if 'requirements_file' not in self.raw_config: + return None + + requirements_file = self.raw_config['requirements_file'] + base_path = os.path.dirname(self.source_file) + with self.catch_validation_error('requirements_file'): + validate_file(requirements_file, base_path) + self['requirements_file'] = requirements_file + + return True + + def validate_conf_file(self): + if 'conf_file' not in self.raw_config: + # self.error('conf_file', self.CONF_FILE_REQUIRED_MESSAGE, code=CONF_FILE_REQUIRED) + return None + + conf_file = self.raw_config['conf_file'] + base_path = os.path.dirname(self.source_file) + with self.catch_validation_error('conf_file'): + validate_file(conf_file, base_path) + self['conf_file'] = conf_file + + return True + + def validate_formats(self): + _formats = self.raw_config.get('formats') + if 'formats' not in self.raw_config or _formats == []: + return None + + with self.catch_validation_error('format'): + validate_list(_formats) + for _format in _formats: + validate_choice(_format, self.get_valid_formats()) + self['formats'] = _formats + + return True class ProjectConfig(list): - pass + + """ + Wrapper for multiple build configs. + """ + + def validate(self): + for build in self: + build.validate() + + def set_output_base(self, directory): + for build in self: + build['output_base'] = os.path.abspath(directory) def load(path, env_config): - pass + """ + Load a project configuration and the top-most build config for a + given path. That is usually the root of the project, but will look deeper. + + The config will be validated. + """ + + filename = find_one(path, CONFIG_FILENAMES) + + if not filename: + files = '{}'.format(', '.join(map(repr, CONFIG_FILENAMES[:-1]))) + if files: + files += ' or ' + files += '{!r}'.format(CONFIG_FILENAMES[-1]) + raise ConfigError('No files {} found'.format(files), + code=CONFIG_REQUIRED) + build_configs = [] + with open(filename, 'r') as file: + try: + configs = parse(file.read()) + except ParseError as error: + raise ConfigError( + 'Parse error in {filename}: {message}'.format( + filename=filename, + message=str(error)), + code=CONFIG_SYNTAX_INVALID) + for i, config in enumerate(configs): + build_config = BuildConfig( + env_config, + config, + source_file=filename, + source_position=i) + build_configs.append(build_config) + project_config = ProjectConfig(build_configs) + project_config.validate() + return project_config From e2f64288f766ce6856501bdc0d1b7d97f1e8813b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Jun 2018 13:42:14 -0500 Subject: [PATCH 04/12] Fix patch --- readthedocs/config/tests/test_config.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 4fa3cb7c0dc..c4a50388692 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -208,7 +208,7 @@ def it_validates_is_a_list(): assert excinfo.value.key == 'python.extra_requirements' assert excinfo.value.code == PYTHON_INVALID - @patch('readthedocs_build.config.config.validate_string') + @patch('readthedocs.config.config.validate_string') def it_uses_validate_string(validate_string): validate_string.return_value = True build = get_build_config( @@ -231,7 +231,7 @@ def it_validates_value(): excinfo.value.key = 'python.use_system_site_packages' excinfo.value.code = INVALID_BOOL - @patch('readthedocs_build.config.config.validate_bool') + @patch('readthedocs.config.config.validate_bool') def it_uses_validate_bool(validate_bool): validate_bool.return_value = True build = get_build_config( @@ -254,7 +254,7 @@ def it_validates_value(): assert excinfo.value.key == 'python.setup_py_install' assert excinfo.value.code == INVALID_BOOL - @patch('readthedocs_build.config.config.validate_bool') + @patch('readthedocs.config.config.validate_bool') def it_uses_validate_bool(validate_bool): validate_bool.return_value = True build = get_build_config( @@ -382,7 +382,7 @@ def it_uses_validate_file(tmpdir): path = tmpdir.join('setup.py') path.write('content') path = str(path) - patcher = patch('readthedocs_build.config.config.validate_file') + patcher = patch('readthedocs.config.config.validate_file') with patcher as validate_file: validate_file.return_value = path build = get_build_config( @@ -421,7 +421,7 @@ def it_validates_to_abspath(tmpdir): build.validate_base() assert build['base'] == str(tmpdir.join('docs')) - @patch('readthedocs_build.config.config.validate_directory') + @patch('readthedocs.config.config.validate_directory') def it_uses_validate_directory(validate_directory): validate_directory.return_value = 'path' build = get_build_config({'base': '../my-path'}) From 8e81e1169422f6904e05552617f6738fcca57203 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Jun 2018 13:49:29 -0500 Subject: [PATCH 05/12] Isort --- readthedocs/config/config.py | 16 ++++++------- readthedocs/config/tests/test_config.py | 32 ++++++++++--------------- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index c84294e8911..d23df8e9c81 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -1,14 +1,14 @@ -from contextlib import contextmanager -import re +from __future__ import division, print_function, unicode_literals + import os +import re +from contextlib import contextmanager from .find import find_one -from .parser import ParseError -from .parser import parse -from .validation import (ValidationError, validate_bool, validate_choice, - validate_directory, validate_file, validate_list, - validate_string) - +from .parser import ParseError, parse +from .validation import ( + ValidationError, validate_bool, validate_choice, validate_directory, + validate_file, validate_list, validate_string) __all__ = ( 'load', 'BuildConfig', 'ConfigError', 'InvalidConfig', 'ProjectConfig') diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index c4a50388692..2c88362b615 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -1,25 +1,18 @@ -from mock import patch -from mock import DEFAULT -from pytest import raises +from __future__ import division, print_function, unicode_literals + import os -from .utils import apply_fs -from readthedocs.config import ConfigError -from readthedocs.config import InvalidConfig -from readthedocs.config import load -from readthedocs.config import BuildConfig -from readthedocs.config import ProjectConfig -from readthedocs.config.config import TYPE_REQUIRED -from readthedocs.config.config import NAME_REQUIRED -from readthedocs.config.config import NAME_INVALID -from readthedocs.config.config import PYTHON_INVALID -from readthedocs.config.validation import INVALID_BOOL -from readthedocs.config.validation import INVALID_CHOICE -from readthedocs.config.validation import INVALID_DIRECTORY -from readthedocs.config.validation import INVALID_LIST -from readthedocs.config.validation import INVALID_PATH -from readthedocs.config.validation import INVALID_STRING +from mock import DEFAULT, patch +from pytest import raises +from readthedocs.config import ( + BuildConfig, ConfigError, InvalidConfig, ProjectConfig, load) +from readthedocs.config.config import ( + NAME_INVALID, NAME_REQUIRED, PYTHON_INVALID, TYPE_REQUIRED) +from readthedocs.config.validation import ( + INVALID_BOOL, INVALID_CHOICE, INVALID_LIST, INVALID_PATH, INVALID_STRING) + +from .utils import apply_fs env_config = { 'output_base': '/tmp' @@ -579,4 +572,3 @@ def test_project_set_output_base(): for build_config in project: assert ( build_config['output_base'] == os.path.join(os.getcwd(), 'random')) - From 80ee48489dafcee282495ba11139032f5eff2837 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Jun 2018 13:55:06 -0500 Subject: [PATCH 06/12] Check string with six --- readthedocs/config/config.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index d23df8e9c81..cc3a27a1c01 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -4,6 +4,8 @@ import re from contextlib import contextmanager +import six + from .find import find_one from .parser import ParseError, parse from .validation import ( @@ -337,7 +339,7 @@ def validate_python(self): # Try to convert strings to an int first, to catch '2', then # a float, to catch '2.7' version = raw_python['version'] - if isinstance(version, str): + if isinstance(version, six.string_types): try: version = int(version) except ValueError: From 36edd04524506fa7ed54c124f4e593cff42e2d1b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Jun 2018 15:05:28 -0500 Subject: [PATCH 07/12] Linter --- readthedocs/config/config.py | 61 +++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index cc3a27a1c01..3381646884c 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -1,3 +1,4 @@ +"""Build configuration for rtd.""" from __future__ import division, print_function, unicode_literals import os @@ -49,12 +50,17 @@ class ConfigError(Exception): + """Base error for the rtd configuration file.""" + def __init__(self, message, code): self.code = code super(ConfigError, self).__init__(message) class InvalidConfig(ConfigError): + + """Error for a specific key validation.""" + message_template = 'Invalid "{key}": {error}' def __init__(self, key, code, error_message, source_file=None, @@ -73,8 +79,9 @@ def __init__(self, key, code, error_message, source_file=None, class BuildConfig(dict): """ - Config that handles the build of one particular documentation. Config keys - can be accessed with a dictionary lookup:: + Config that handles the build of one particular documentation. + + Config keys can be accessed with a dictionary lookup:: >>> build_config['type'] 'sphinx' @@ -102,8 +109,10 @@ def __init__(self, env_config, raw_config, source_file, source_position): self.raw_config = raw_config self.source_file = source_file self.source_position = source_position + super(BuildConfig, self).__init__() def error(self, key, message, code): + """Raise an error related to ``key``.""" source = '{file} [{pos}]'.format( file=self.source_file, pos=self.source_position) @@ -117,6 +126,7 @@ def error(self, key, message, code): @contextmanager def catch_validation_error(self, key): + """Catch a ``ValidationError`` and raises an ``InvalidConfig`` error.""" try: yield except ValidationError as error: @@ -127,19 +137,21 @@ def catch_validation_error(self, key): source_file=self.source_file, source_position=self.source_position) - def get_valid_types(self): + def get_valid_types(self): # noqa return ( 'sphinx', ) def get_valid_python_versions(self): + """Get all valid python versions.""" try: return self.env_config['python']['supported_versions'] except (KeyError, TypeError): pass return self.PYTHON_SUPPORTED_VERSIONS - def get_valid_formats(self): + def get_valid_formats(self): # noqa + """Get all valid documentation formats.""" return ( 'htmlzip', 'pdf', @@ -148,8 +160,7 @@ def get_valid_formats(self): def validate(self): """ - Validate and process config into ``config`` attribute that contains the - ready to use build configuration. + Validate and process ``raw_config`` and ``env_config`` attributes. It makes sure that: @@ -157,7 +168,6 @@ def validate(self): - ``base`` is a valid directory and defaults to the directory of the ``readthedocs.yml`` config file if not set """ - # Validate env_config. self.validate_output_base() @@ -176,6 +186,7 @@ def validate(self): self.validate_conf_file() def validate_output_base(self): + """Validates that ``output_base`` exists and set its absolute path.""" assert 'output_base' in self.env_config, ( '"output_base" required in "env_config"') base_path = os.path.dirname(self.source_file) @@ -186,6 +197,7 @@ def validate_output_base(self): ) def validate_name(self): + """Validates that name exists.""" name = self.raw_config.get('name', None) if not name: name = self.env_config.get('name', None) @@ -203,18 +215,20 @@ def validate_name(self): self['name'] = name def validate_type(self): - type = self.raw_config.get('type', None) - if not type: - type = self.env_config.get('type', None) - if not type: + """Validates that type is a valid choice.""" + type_ = self.raw_config.get('type', None) + if not type_: + type_ = self.env_config.get('type', None) + if not type_: self.error('type', self.TYPE_REQUIRED_MESSAGE, code=TYPE_REQUIRED) with self.catch_validation_error('type'): - validate_choice(type, self.get_valid_types()) + validate_choice(type_, self.get_valid_types()) - self['type'] = type + self['type'] = type_ def validate_base(self): + """Validates that path is a valid directory.""" if 'base' in self.raw_config: base = self.raw_config['base'] else: @@ -275,6 +289,7 @@ def validate_build(self): self['build'] = build def validate_python(self): + """Validates the ``python`` key, set default values it's necessary.""" python = { 'use_system_site_packages': False, 'pip_install': False, @@ -355,6 +370,7 @@ def validate_python(self): self['python'] = python def validate_conda(self): + """Validates the ``conda`` key.""" conda = {} if 'conda' in self.raw_config: @@ -374,6 +390,7 @@ def validate_conda(self): self['conda'] = conda def validate_requirements_file(self): + """Validates that the requirements file exists.""" if 'requirements_file' not in self.raw_config: return None @@ -386,8 +403,8 @@ def validate_requirements_file(self): return True def validate_conf_file(self): + """Validates the conf.py file for sphinx.""" if 'conf_file' not in self.raw_config: - # self.error('conf_file', self.CONF_FILE_REQUIRED_MESSAGE, code=CONF_FILE_REQUIRED) return None conf_file = self.raw_config['conf_file'] @@ -399,6 +416,7 @@ def validate_conf_file(self): return True def validate_formats(self): + """Validates that formats contains only valid formats.""" _formats = self.raw_config.get('formats') if 'formats' not in self.raw_config or _formats == []: return None @@ -414,27 +432,26 @@ def validate_formats(self): class ProjectConfig(list): - """ - Wrapper for multiple build configs. - """ + """Wrapper for multiple build configs.""" def validate(self): + """Validates each configuration build.""" for build in self: build.validate() def set_output_base(self, directory): + """Set a common ``output_base`` for each configuration build.""" for build in self: build['output_base'] = os.path.abspath(directory) def load(path, env_config): """ - Load a project configuration and the top-most build config for a - given path. That is usually the root of the project, but will look deeper. + Load a project configuration and the top-most build config for a given path. + That is usually the root of the project, but will look deeper. The config will be validated. """ - filename = find_one(path, CONFIG_FILENAMES) if not filename: @@ -445,9 +462,9 @@ def load(path, env_config): raise ConfigError('No files {} found'.format(files), code=CONFIG_REQUIRED) build_configs = [] - with open(filename, 'r') as file: + with open(filename, 'r') as configuration_file: try: - configs = parse(file.read()) + configs = parse(configuration_file.read()) except ParseError as error: raise ConfigError( 'Parse error in {filename}: {message}'.format( From 9759e70336d7fe1c5dbbef2f4b6b70d25f40174d Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Jun 2018 18:22:18 -0500 Subject: [PATCH 08/12] Remove readthedocs-build from dependencies --- requirements/pip.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements/pip.txt b/requirements/pip.txt index bcc03d1dcae..a9b9eef04b7 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -13,7 +13,6 @@ mkdocs==0.17.3 django==1.9.13 six==1.11.0 future==0.16.0 -readthedocs-build<2.1 # django-tastypie 0.13.x and 0.14.0 are not compatible with our code django-tastypie==0.13.0 From 7dd15c4bff9f7bb03a58f05a81e0d7b4e1d592d1 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 19 Jun 2018 18:23:33 -0500 Subject: [PATCH 09/12] Change imports --- readthedocs/doc_builder/config.py | 7 ++++--- readthedocs/projects/tasks.py | 2 +- readthedocs/rtd_tests/tests/test_config_wrapper.py | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/readthedocs/doc_builder/config.py b/readthedocs/doc_builder/config.py index 05cc58397fc..14f8e128703 100644 --- a/readthedocs/doc_builder/config.py +++ b/readthedocs/doc_builder/config.py @@ -5,10 +5,11 @@ absolute_import, division, print_function, unicode_literals) from builtins import filter, object -from readthedocs_build.config import load as load_config -from readthedocs_build.config import BuildConfig, ConfigError, InvalidConfig -from .constants import DOCKER_IMAGE_SETTINGS, DOCKER_IMAGE +from readthedocs.config import BuildConfig, ConfigError, InvalidConfig +from readthedocs.config import load as load_config + +from .constants import DOCKER_IMAGE, DOCKER_IMAGE_SETTINGS class ConfigWrapper(object): diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 01f674265bc..796fe565d3e 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -25,7 +25,7 @@ from django.core.urlresolvers import reverse from django.db.models import Q from django.utils.translation import ugettext_lazy as _ -from readthedocs_build.config import ConfigError +from readthedocs.config import ConfigError from slumber.exceptions import HttpClientError from .constants import LOG_TEMPLATE diff --git a/readthedocs/rtd_tests/tests/test_config_wrapper.py b/readthedocs/rtd_tests/tests/test_config_wrapper.py index cae7bec62f5..c9848b4d36c 100644 --- a/readthedocs/rtd_tests/tests/test_config_wrapper.py +++ b/readthedocs/rtd_tests/tests/test_config_wrapper.py @@ -3,7 +3,7 @@ from django.test import TestCase from django_dynamic_fixture import get -from readthedocs_build.config import BuildConfig, ProjectConfig, InvalidConfig +from readthedocs.config import BuildConfig, ProjectConfig, InvalidConfig from readthedocs.builds.models import Version from readthedocs.projects.models import Project from readthedocs.doc_builder.config import load_yaml_config From aebf1fbf4557a9ba1e33ed4e7e3567e353df24ab Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 20 Jun 2018 21:37:28 -0500 Subject: [PATCH 10/12] Add test for keep previous behavior --- readthedocs/config/tests/test_config.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 2c88362b615..8dd01b7e8b7 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -321,10 +321,20 @@ def it_gets_set_correctly(): build.validate_formats() assert build['formats'] == ['pdf'] + def formats_can_be_null(): + build = get_build_config({'formats': None}) + build.validate_formats() + assert 'formats' not in build + + def formats_with_previous_none(): + build = get_build_config({'formats': ['none']}) + build.validate_formats() + assert build['formats'] == [] + def formats_can_be_empty(): build = get_build_config({'formats': []}) build.validate_formats() - assert 'formats' not in build + assert build['formats'] == [] def all_valid_formats(): build = get_build_config({'formats': ['pdf', 'htmlzip', 'epub']}) From 931d45c231b03fab4456e162cd1423408dd0ae6f Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 20 Jun 2018 21:56:23 -0500 Subject: [PATCH 11/12] Fix tests --- readthedocs/config/tests/test_config.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 8dd01b7e8b7..5b1ba7ae653 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -106,11 +106,11 @@ def test_build_config_has_source_position(tmpdir): assert second.source_position == 1 -def test_build_config_has_list_with_single_null_value(tmpdir): +def test_build_config_has_list_with_single_empty_value(tmpdir): base = str(apply_fs(tmpdir, config_with_explicit_empty_list)) build = load(base, env_config)[0] assert isinstance(build, BuildConfig) - assert 'formats' not in build + assert build['formats'] == [] def test_config_requires_name(): @@ -356,7 +356,7 @@ def formats_have_only_allowed_values(): assert excinfo.value.code == INVALID_CHOICE def only_list_type(): - build = get_build_config({'formats': None}) + build = get_build_config({'formats': 'no-list'}) with raises(InvalidConfig) as excinfo: build.validate_formats() assert excinfo.value.key == 'format' From cd0547e494e10c1ed046d2354c253eb20f32a2b8 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 20 Jun 2018 21:57:15 -0500 Subject: [PATCH 12/12] Keep previous behavior --- readthedocs/config/config.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 3381646884c..c382cd68f2f 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -417,15 +417,18 @@ def validate_conf_file(self): def validate_formats(self): """Validates that formats contains only valid formats.""" - _formats = self.raw_config.get('formats') - if 'formats' not in self.raw_config or _formats == []: + formats = self.raw_config.get('formats') + if formats is None: return None + if formats == ['none']: + self['formats'] = [] + return True with self.catch_validation_error('format'): - validate_list(_formats) - for _format in _formats: - validate_choice(_format, self.get_valid_formats()) - self['formats'] = _formats + validate_list(formats) + for format_ in formats: + validate_choice(format_, self.get_valid_formats()) + self['formats'] = formats return True