From 9d787aa9cc3d0c961fa1296bbc0aa611ac01d9f7 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 8 Nov 2018 22:53:31 -0500 Subject: [PATCH 1/4] Remove unused validations from v1 config --- readthedocs/config/config.py | 73 -------------------- readthedocs/config/tests/test_config.py | 91 +------------------------ 2 files changed, 1 insertion(+), 163 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 4b7c48f5649..c91a4546209 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -6,7 +6,6 @@ from __future__ import division, print_function, unicode_literals import os -import re from contextlib import contextmanager import six @@ -22,7 +21,6 @@ validate_bool, validate_choice, validate_dict, - validate_directory, validate_file, validate_list, validate_string, @@ -43,12 +41,8 @@ CONFIG_NOT_SUPPORTED = 'config-not-supported' VERSION_INVALID = 'version-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' PYTHON_INVALID = 'python-invalid' SUBMODULES_INVALID = 'submodules-invalid' @@ -263,12 +257,6 @@ class BuildConfigV1(BuildConfigBase): """Version 1 of the configuration file.""" - 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}' - ) CONF_FILE_REQUIRED_MESSAGE = 'Missing key "conf_file"' PYTHON_INVALID_MESSAGE = '"python" section must be a mapping.' PYTHON_EXTRA_REQUIREMENTS_INVALID_MESSAGE = ( @@ -306,63 +294,17 @@ def validate(self): ``readthedocs.yml`` config file if not set """ # Validate env_config. - # TODO: this isn't used - self._config['output_base'] = self.validate_output_base() - # Validate the build environment first # Must happen before `validate_python`! self._config['build'] = self.validate_build() # Validate raw_config. Order matters. - # TODO: this isn't used - self._config['name'] = self.validate_name() - # TODO: this isn't used - self._config['base'] = self.validate_base() self._config['python'] = self.validate_python() self._config['formats'] = self.validate_formats() self._config['conda'] = self.validate_conda() self._config['requirements_file'] = self.validate_requirements_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"') - output_base = os.path.abspath( - os.path.join( - self.env_config.get('output_base', self.base_path), - ) - ) - return output_base - - 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) - 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) - - return name - - def validate_base(self): - """Validates that path is a valid directory.""" - if 'base' in self.raw_config: - base = self.raw_config['base'] - else: - base = self.base_path - with self.catch_validation_error('base'): - base = validate_directory(base, self.base_path) - return base - def validate_build(self): """ Validate the build config settings. @@ -542,21 +484,6 @@ def validate_formats(self): return formats - @property - def name(self): - """The project name.""" - return self._config['name'] - - @property - def base(self): - """The base directory.""" - return self._config['base'] - - @property - def output_base(self): - """The output base.""" - return self._config['output_base'] - @property def formats(self): """The documentation formats to be built.""" diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index f623881a615..667e1d220e7 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -24,8 +24,6 @@ CONFIG_NOT_SUPPORTED, CONFIG_REQUIRED, INVALID_KEY, - NAME_INVALID, - NAME_REQUIRED, PYTHON_INVALID, VERSION_INVALID, ) @@ -34,8 +32,6 @@ INVALID_BOOL, INVALID_CHOICE, INVALID_LIST, - INVALID_PATH, - INVALID_STRING, VALUE_NOT_FOUND, ValidationError, ) @@ -90,10 +86,7 @@ def get_build_config(config, env_config=None, source_file='readthedocs.yml'): def get_env_config(extra=None): """Get the minimal env_config for the configuration object.""" - defaults = { - 'output_base': '', - 'name': 'name', - } + defaults = {} if extra is None: extra = {} defaults.update(extra) @@ -186,30 +179,6 @@ def test_build_config_has_list_with_single_empty_value(tmpdir): assert build.formats == [] -def test_config_requires_name(): - build = BuildConfigV1( - {'output_base': ''}, - {}, - source_file='readthedocs.yml', - ) - with raises(InvalidConfig) as excinfo: - build.validate() - assert excinfo.value.key == 'name' - assert excinfo.value.code == NAME_REQUIRED - - -def test_build_requires_valid_name(): - build = BuildConfigV1( - {'output_base': ''}, - {'name': 'with/slashes'}, - source_file='readthedocs.yml', - ) - with raises(InvalidConfig) as excinfo: - build.validate() - assert excinfo.value.key == 'name' - assert excinfo.value.code == NAME_INVALID - - def test_version(): build = get_build_config({}, get_env_config()) assert build.version == '1' @@ -528,62 +497,10 @@ def test_valid_build_config(): source_file='readthedocs.yml', ) build.validate() - assert build.name == 'docs' - assert build.base assert build.python assert build.python.install_with_setup is False assert build.python.install_with_pip is False assert build.python.use_system_site_packages is False - assert build.output_base - - -class TestValidateBase(object): - - def test_it_validates_to_abspath(self, tmpdir): - apply_fs(tmpdir, {'configs': minimal_config, 'docs': {}}) - with tmpdir.as_cwd(): - source_file = str(tmpdir.join('configs', 'readthedocs.yml')) - build = BuildConfigV1( - get_env_config(), - {'base': '../docs'}, - source_file=source_file, - ) - build.validate() - assert build.base == str(tmpdir.join('docs')) - - @patch('readthedocs.config.config.validate_directory') - def test_it_uses_validate_directory(self, validate_directory): - validate_directory.return_value = 'path' - build = get_build_config({'base': '../my-path'}, get_env_config()) - build.validate() - # Test for first argument to validate_directory - args, kwargs = validate_directory.call_args - assert args[0] == '../my-path' - - def test_it_fails_if_base_is_not_a_string(self, tmpdir): - apply_fs(tmpdir, minimal_config) - with tmpdir.as_cwd(): - build = BuildConfigV1( - get_env_config(), - {'base': 1}, - source_file=str(tmpdir.join('readthedocs.yml')), - ) - with raises(InvalidConfig) as excinfo: - build.validate() - assert excinfo.value.key == 'base' - assert excinfo.value.code == INVALID_STRING - - def test_it_fails_if_base_does_not_exist(self, tmpdir): - apply_fs(tmpdir, minimal_config) - build = BuildConfigV1( - get_env_config(), - {'base': 'docs'}, - source_file=str(tmpdir.join('readthedocs.yml')), - ) - with raises(InvalidConfig) as excinfo: - build.validate() - assert excinfo.value.key == 'base' - assert excinfo.value.code == INVALID_PATH class TestValidateBuild(object): @@ -752,16 +669,10 @@ def test_build_validate_calls_all_subvalidators(tmpdir): ) with patch.multiple( BuildConfigV1, - validate_base=DEFAULT, - validate_name=DEFAULT, validate_python=DEFAULT, - validate_output_base=DEFAULT, ): build.validate() - BuildConfigV1.validate_base.assert_called_with() - BuildConfigV1.validate_name.assert_called_with() BuildConfigV1.validate_python.assert_called_with() - BuildConfigV1.validate_output_base.assert_called_with() def test_load_calls_validate(tmpdir): From 51e693eae05c37d68a71008188e347049705b5cc Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Fri, 9 Nov 2018 11:34:23 -0500 Subject: [PATCH 2/4] Clean up tests --- readthedocs/config/tests/test_config.py | 236 +++++++++++------------- 1 file changed, 103 insertions(+), 133 deletions(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 667e1d220e7..e015c22badf 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -38,41 +38,13 @@ from .utils import apply_fs -env_config = { - 'output_base': '/tmp', -} - -minimal_config = { - 'name': 'docs', -} - -config_with_explicit_empty_list = { - 'readthedocs.yml': ''' -name: docs -formats: [] -''', -} - -minimal_config_dir = { - 'readthedocs.yml': '''\ -name: docs -''', -} - -multiple_config_dir = { - 'readthedocs.yml': ''' -name: first ---- -name: second - ''', - 'nested': minimal_config_dir, -} - -yaml_extension_config_dir = { - 'readthedocs.yaml': '''\ -name: docs -type: sphinx -''' +yaml_config_dir = { + 'readthedocs.yml': textwrap.dedent( + ''' + formats: + - pdf + ''' + ), } @@ -84,15 +56,6 @@ def get_build_config(config, env_config=None, source_file='readthedocs.yml'): ) -def get_env_config(extra=None): - """Get the minimal env_config for the configuration object.""" - defaults = {} - if extra is None: - extra = {} - defaults.update(extra) - return defaults - - @pytest.mark.parametrize('files', [ {'readthedocs.ymlmore': ''}, {'first': {'readthedocs.yml': ''}}, {'startreadthedocs.yml': ''}, {'second': {'confuser.txt': 'content'}}, @@ -104,7 +67,7 @@ def test_load_no_config_file(tmpdir, files): apply_fs(tmpdir, files) base = str(tmpdir) with raises(ConfigError) as e: - load(base, env_config) + load(base, {}) assert e.value.code == CONFIG_REQUIRED @@ -114,13 +77,13 @@ def test_load_empty_config_file(tmpdir): }) base = str(tmpdir) with raises(ConfigError): - load(base, env_config) + load(base, {}) def test_minimal_config(tmpdir): - apply_fs(tmpdir, minimal_config_dir) + apply_fs(tmpdir, yaml_config_dir) base = str(tmpdir) - build = load(base, env_config) + build = load(base, {'formats': []}) assert isinstance(build, BuildConfigV1) @@ -131,7 +94,7 @@ def test_load_version1(tmpdir): ''') }) base = str(tmpdir) - build = load(base, get_env_config({'allow_v2': True})) + build = load(base, {'allow_v2': True}) assert isinstance(build, BuildConfigV1) @@ -142,7 +105,7 @@ def test_load_version2(tmpdir): ''') }) base = str(tmpdir) - build = load(base, get_env_config({'allow_v2': True})) + build = load(base, {'allow_v2': True}) assert isinstance(build, BuildConfigV2) @@ -154,59 +117,70 @@ def test_load_unknow_version(tmpdir): }) base = str(tmpdir) with raises(ConfigError) as excinfo: - load(base, get_env_config({'allow_v2': True})) + load(base, {'allow_v2': True}) assert excinfo.value.code == VERSION_INVALID def test_yaml_extension(tmpdir): """Make sure it's capable of loading the 'readthedocs' file with a 'yaml' extension.""" - apply_fs(tmpdir, yaml_extension_config_dir) + apply_fs(tmpdir, { + 'readthedocs.yaml': textwrap.dedent( + ''' + python: + version: 3 + ''' + ), + }) base = str(tmpdir) - config = load(base, env_config) + config = load(base, {}) assert isinstance(config, BuildConfigV1) def test_build_config_has_source_file(tmpdir): - base = str(apply_fs(tmpdir, minimal_config_dir)) - build = load(base, env_config) + base = str(apply_fs(tmpdir, yaml_config_dir)) + build = load(base, {}) assert build.source_file == os.path.join(base, 'readthedocs.yml') 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) + base = str(apply_fs(tmpdir, { + 'readthedocs.yml': textwrap.dedent( + ''' + formats: [] + ''' + ) + })) + build = load(base, {}) assert isinstance(build, BuildConfigV1) assert build.formats == [] def test_version(): - build = get_build_config({}, get_env_config()) + build = get_build_config({}, {}) assert build.version == '1' def test_doc_type(): build = get_build_config( {}, - get_env_config( - { - 'defaults': { - 'doctype': 'sphinx', - }, - } - ) + { + 'defaults': { + 'doctype': 'sphinx', + }, + } ) build.validate() assert build.doctype == 'sphinx' def test_empty_python_section_is_valid(): - build = get_build_config({'python': {}}, get_env_config()) + build = get_build_config({'python': {}}, {}) build.validate() assert build.python def test_python_section_must_be_dict(): - build = get_build_config({'python': 123}, get_env_config()) + build = get_build_config({'python': 123}, {}) with raises(InvalidConfig) as excinfo: build.validate() assert excinfo.value.key == 'python' @@ -214,7 +188,7 @@ def test_python_section_must_be_dict(): def test_use_system_site_packages_defaults_to_false(): - build = get_build_config({'python': {}}, get_env_config()) + build = get_build_config({'python': {}}, {}) build.validate() # Default is False. assert not build.python.use_system_site_packages @@ -225,13 +199,13 @@ def test_use_system_site_packages_repects_default_value(value): defaults = { 'use_system_packages': value, } - build = get_build_config({}, get_env_config({'defaults': defaults})) + build = get_build_config({}, {'defaults': defaults}) build.validate() assert build.python.use_system_site_packages is value def test_python_pip_install_default(): - build = get_build_config({'python': {}}, get_env_config()) + build = get_build_config({'python': {}}, {}) build.validate() # Default is False. assert build.python.install_with_pip is False @@ -240,7 +214,7 @@ def test_python_pip_install_default(): class TestValidatePythonExtraRequirements(object): def test_it_defaults_to_list(self): - build = get_build_config({'python': {}}, get_env_config()) + build = get_build_config({'python': {}}, {}) build.validate() # Default is an empty list. assert build.python.extra_requirements == [] @@ -248,7 +222,7 @@ def test_it_defaults_to_list(self): def test_it_validates_is_a_list(self): build = get_build_config( {'python': {'extra_requirements': 'invalid'}}, - get_env_config(), + {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -265,7 +239,7 @@ def test_it_uses_validate_string(self, validate_string): 'extra_requirements': ['tests'], }, }, - get_env_config(), + {}, ) build.validate() validate_string.assert_any_call('tests') @@ -274,14 +248,14 @@ def test_it_uses_validate_string(self, validate_string): class TestValidateUseSystemSitePackages(object): def test_it_defaults_to_false(self): - build = get_build_config({'python': {}}, get_env_config()) + build = get_build_config({'python': {}}, {}) build.validate() assert build.python.use_system_site_packages is False def test_it_validates_value(self): build = get_build_config( {'python': {'use_system_site_packages': 'invalid'}}, - get_env_config(), + {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -293,7 +267,7 @@ def test_it_uses_validate_bool(self, validate_bool): validate_bool.return_value = True build = get_build_config( {'python': {'use_system_site_packages': 'to-validate'}}, - get_env_config(), + {}, ) build.validate() validate_bool.assert_any_call('to-validate') @@ -302,14 +276,14 @@ def test_it_uses_validate_bool(self, validate_bool): class TestValidateSetupPyInstall(object): def test_it_defaults_to_false(self): - build = get_build_config({'python': {}}, get_env_config()) + build = get_build_config({'python': {}}, {}) build.validate() assert build.python.install_with_setup is False def test_it_validates_value(self): build = get_build_config( {'python': {'setup_py_install': 'this-is-string'}}, - get_env_config(), + {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -321,7 +295,7 @@ def test_it_uses_validate_bool(self, validate_bool): validate_bool.return_value = True build = get_build_config( {'python': {'setup_py_install': 'to-validate'}}, - get_env_config(), + {}, ) build.validate() validate_bool.assert_any_call('to-validate') @@ -330,7 +304,7 @@ def test_it_uses_validate_bool(self, validate_bool): class TestValidatePythonVersion(object): def test_it_defaults_to_a_valid_version(self): - build = get_build_config({'python': {}}, get_env_config()) + build = get_build_config({'python': {}}, {}) build.validate() assert build.python.version == 2 assert build.python_interpreter == 'python2.7' @@ -339,7 +313,7 @@ def test_it_defaults_to_a_valid_version(self): def test_it_supports_other_versions(self): build = get_build_config( {'python': {'version': 3.5}}, - get_env_config(), + {}, ) build.validate() assert build.python.version == 3.5 @@ -349,7 +323,7 @@ def test_it_supports_other_versions(self): def test_it_validates_versions_out_of_range(self): build = get_build_config( {'python': {'version': 1.0}}, - get_env_config(), + {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -359,7 +333,7 @@ def test_it_validates_versions_out_of_range(self): def test_it_validates_wrong_type(self): build = get_build_config( {'python': {'version': 'this-is-string'}}, - get_env_config(), + {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -369,7 +343,7 @@ def test_it_validates_wrong_type(self): def test_it_validates_wrong_type_right_value(self): build = get_build_config( {'python': {'version': '3.5'}}, - get_env_config(), + {}, ) build.validate() assert build.python.version == 3.5 @@ -378,7 +352,7 @@ def test_it_validates_wrong_type_right_value(self): build = get_build_config( {'python': {'version': '3'}}, - get_env_config(), + {}, ) build.validate() assert build.python.version == 3 @@ -388,12 +362,10 @@ def test_it_validates_wrong_type_right_value(self): def test_it_validates_env_supported_versions(self): build = get_build_config( {'python': {'version': 3.6}}, - env_config=get_env_config( - { - 'python': {'supported_versions': [3.5]}, - 'build': {'image': 'custom'}, - } - ) + env_config={ + 'python': {'supported_versions': [3.5]}, + 'build': {'image': 'custom'}, + }, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -402,12 +374,10 @@ def test_it_validates_env_supported_versions(self): build = get_build_config( {'python': {'version': 3.6}}, - env_config=get_env_config( - { - 'python': {'supported_versions': [3.5, 3.6]}, - 'build': {'image': 'custom'}, - } - ) + env_config={ + 'python': {'supported_versions': [3.5, 3.6]}, + 'build': {'image': 'custom'}, + }, ) build.validate() assert build.python.version == 3.6 @@ -421,7 +391,7 @@ def test_it_respects_default_value(self, value): } build = get_build_config( {}, - get_env_config({'defaults': defaults}), + {'defaults': defaults}, ) build.validate() assert build.python.version == value @@ -430,34 +400,34 @@ def test_it_respects_default_value(self, value): class TestValidateFormats(object): def test_it_defaults_to_empty(self): - build = get_build_config({}, get_env_config()) + build = get_build_config({}, {}) build.validate() assert build.formats == [] def test_it_gets_set_correctly(self): - build = get_build_config({'formats': ['pdf']}, get_env_config()) + build = get_build_config({'formats': ['pdf']}, {}) build.validate() assert build.formats == ['pdf'] def test_formats_can_be_null(self): - build = get_build_config({'formats': None}, get_env_config()) + build = get_build_config({'formats': None}, {}) build.validate() assert build.formats == [] def test_formats_with_previous_none(self): - build = get_build_config({'formats': ['none']}, get_env_config()) + build = get_build_config({'formats': ['none']}, {}) build.validate() assert build.formats == [] def test_formats_can_be_empty(self): - build = get_build_config({'formats': []}, get_env_config()) + build = get_build_config({'formats': []}, {}) build.validate() assert build.formats == [] def test_all_valid_formats(self): build = get_build_config( {'formats': ['pdf', 'htmlzip', 'epub']}, - get_env_config() + {}, ) build.validate() assert build.formats == ['pdf', 'htmlzip', 'epub'] @@ -465,7 +435,7 @@ def test_all_valid_formats(self): def test_cant_have_none_as_format(self): build = get_build_config( {'formats': ['htmlzip', None]}, - get_env_config() + {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -475,7 +445,7 @@ def test_cant_have_none_as_format(self): def test_formats_have_only_allowed_values(self): build = get_build_config( {'formats': ['htmlzip', 'csv']}, - get_env_config() + {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -483,7 +453,7 @@ def test_formats_have_only_allowed_values(self): assert excinfo.value.code == INVALID_CHOICE def test_only_list_type(self): - build = get_build_config({'formats': 'no-list'}, get_env_config()) + build = get_build_config({'formats': 'no-list'}, {}) with raises(InvalidConfig) as excinfo: build.validate() assert excinfo.value.key == 'format' @@ -492,8 +462,8 @@ def test_only_list_type(self): def test_valid_build_config(): build = BuildConfigV1( - env_config, - minimal_config, + {}, + {}, source_file='readthedocs.yml', ) build.validate() @@ -506,9 +476,9 @@ def test_valid_build_config(): class TestValidateBuild(object): def test_it_fails_if_build_is_invalid_option(self, tmpdir): - apply_fs(tmpdir, minimal_config) + apply_fs(tmpdir, yaml_config_dir) build = BuildConfigV1( - get_env_config(), + {}, {'build': {'image': 3.0}}, source_file=str(tmpdir.join('readthedocs.yml')), ) @@ -518,7 +488,7 @@ def test_it_fails_if_build_is_invalid_option(self, tmpdir): assert excinfo.value.code == INVALID_CHOICE def test_it_fails_on_python_validation(self, tmpdir): - apply_fs(tmpdir, minimal_config) + apply_fs(tmpdir, yaml_config_dir) build = BuildConfigV1( {}, { @@ -534,7 +504,7 @@ def test_it_fails_on_python_validation(self, tmpdir): assert excinfo.value.code == INVALID_CHOICE def test_it_works_on_python_validation(self, tmpdir): - apply_fs(tmpdir, minimal_config) + apply_fs(tmpdir, yaml_config_dir) build = BuildConfigV1( {}, { @@ -547,9 +517,9 @@ def test_it_works_on_python_validation(self, tmpdir): build.validate_python() def test_it_works(self, tmpdir): - apply_fs(tmpdir, minimal_config) + apply_fs(tmpdir, yaml_config_dir) build = BuildConfigV1( - get_env_config(), + {}, {'build': {'image': 'latest'}}, source_file=str(tmpdir.join('readthedocs.yml')), ) @@ -557,9 +527,9 @@ def test_it_works(self, tmpdir): assert build.build.image == 'readthedocs/build:latest' def test_default(self, tmpdir): - apply_fs(tmpdir, minimal_config) + apply_fs(tmpdir, yaml_config_dir) build = BuildConfigV1( - get_env_config(), + {}, {}, source_file=str(tmpdir.join('readthedocs.yml')), ) @@ -569,12 +539,12 @@ def test_default(self, tmpdir): @pytest.mark.parametrize( 'image', ['latest', 'readthedocs/build:3.0', 'rtd/build:latest']) def test_it_priorities_image_from_env_config(self, tmpdir, image): - apply_fs(tmpdir, minimal_config) + apply_fs(tmpdir, yaml_config_dir) defaults = { 'build_image': image, } build = BuildConfigV1( - get_env_config({'defaults': defaults}), + {'defaults': defaults}, {'build': {'image': 'latest'}}, source_file=str(tmpdir.join('readthedocs.yml')), ) @@ -583,7 +553,7 @@ def test_it_priorities_image_from_env_config(self, tmpdir, image): def test_use_conda_default_false(): - build = get_build_config({}, get_env_config()) + build = get_build_config({}, {}) build.validate() assert build.conda is None @@ -591,7 +561,7 @@ def test_use_conda_default_false(): def test_use_conda_respects_config(): build = get_build_config( {'conda': {}}, - get_env_config(), + {}, ) build.validate() assert isinstance(build.conda, Conda) @@ -601,7 +571,7 @@ def test_validates_conda_file(tmpdir): apply_fs(tmpdir, {'environment.yml': ''}) build = get_build_config( {'conda': {'file': 'environment.yml'}}, - get_env_config(), + {}, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() @@ -610,7 +580,7 @@ def test_validates_conda_file(tmpdir): def test_requirements_file_empty(): - build = get_build_config({}, get_env_config()) + build = get_build_config({}, {}) build.validate() assert build.python.requirements is None @@ -622,7 +592,7 @@ def test_requirements_file_repects_default_value(tmpdir): } build = get_build_config( {}, - get_env_config({'defaults': defaults}), + {'defaults': defaults}, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() @@ -633,7 +603,7 @@ def test_requirements_file_respects_configuration(tmpdir): apply_fs(tmpdir, {'requirements.txt': ''}) build = get_build_config( {'requirements_file': 'requirements.txt'}, - get_env_config(), + {}, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() @@ -643,7 +613,7 @@ def test_requirements_file_respects_configuration(tmpdir): def test_requirements_file_is_null(tmpdir): build = get_build_config( {'requirements_file': None}, - get_env_config(), + {}, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() @@ -653,7 +623,7 @@ def test_requirements_file_is_null(tmpdir): def test_requirements_file_is_blank(tmpdir): build = get_build_config( {'requirements_file': ''}, - get_env_config(), + {}, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() @@ -661,7 +631,7 @@ def test_requirements_file_is_blank(tmpdir): def test_build_validate_calls_all_subvalidators(tmpdir): - apply_fs(tmpdir, minimal_config) + apply_fs(tmpdir, {}) build = BuildConfigV1( {}, {}, @@ -676,15 +646,15 @@ def test_build_validate_calls_all_subvalidators(tmpdir): def test_load_calls_validate(tmpdir): - apply_fs(tmpdir, minimal_config_dir) + apply_fs(tmpdir, yaml_config_dir) base = str(tmpdir) with patch.object(BuildConfigV1, 'validate') as build_validate: - load(base, env_config) + load(base, {}) assert build_validate.call_count == 1 def test_raise_config_not_supported(): - build = get_build_config({}, get_env_config()) + build = get_build_config({}, {}) build.validate() with raises(ConfigOptionNotSupportedError) as excinfo: build.redirects @@ -710,12 +680,12 @@ def test_as_dict(tmpdir): }, 'requirements_file': 'requirements.txt', }, - get_env_config({ + { 'defaults': { 'doctype': 'sphinx', 'sphinx_configuration': None, }, - }), + }, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() From e4d2e5b3bf729001f0c8834c1cecf60976d726d1 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Fri, 9 Nov 2018 11:36:10 -0500 Subject: [PATCH 3/4] Remove unnecessary values --- readthedocs/doc_builder/config.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/readthedocs/doc_builder/config.py b/readthedocs/doc_builder/config.py index 00a0095bcc6..c9ee021a2f8 100644 --- a/readthedocs/doc_builder/config.py +++ b/readthedocs/doc_builder/config.py @@ -43,8 +43,6 @@ def load_yaml_config(version): 'build': { 'image': img_name, }, - 'output_base': '', - 'name': version.slug, 'defaults': { 'install_project': project.install_project, 'formats': get_default_formats(project), From 421e1f80a7e697ad085908e6a38eede1ecf1260d Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Fri, 9 Nov 2018 11:56:38 -0500 Subject: [PATCH 4/4] More clean up --- readthedocs/config/tests/test_config.py | 57 +++++++------------ .../tests/test_config_integration.py | 2 - 2 files changed, 19 insertions(+), 40 deletions(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index e015c22badf..593453d8cc4 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -83,7 +83,7 @@ def test_load_empty_config_file(tmpdir): def test_minimal_config(tmpdir): apply_fs(tmpdir, yaml_config_dir) base = str(tmpdir) - build = load(base, {'formats': []}) + build = load(base, {}) assert isinstance(build, BuildConfigV1) @@ -156,7 +156,7 @@ def test_build_config_has_list_with_single_empty_value(tmpdir): def test_version(): - build = get_build_config({}, {}) + build = get_build_config({}) assert build.version == '1' @@ -174,13 +174,13 @@ def test_doc_type(): def test_empty_python_section_is_valid(): - build = get_build_config({'python': {}}, {}) + build = get_build_config({'python': {}}) build.validate() assert build.python def test_python_section_must_be_dict(): - build = get_build_config({'python': 123}, {}) + build = get_build_config({'python': 123}) with raises(InvalidConfig) as excinfo: build.validate() assert excinfo.value.key == 'python' @@ -188,7 +188,7 @@ def test_python_section_must_be_dict(): def test_use_system_site_packages_defaults_to_false(): - build = get_build_config({'python': {}}, {}) + build = get_build_config({'python': {}}) build.validate() # Default is False. assert not build.python.use_system_site_packages @@ -205,7 +205,7 @@ def test_use_system_site_packages_repects_default_value(value): def test_python_pip_install_default(): - build = get_build_config({'python': {}}, {}) + build = get_build_config({'python': {}}) build.validate() # Default is False. assert build.python.install_with_pip is False @@ -214,7 +214,7 @@ def test_python_pip_install_default(): class TestValidatePythonExtraRequirements(object): def test_it_defaults_to_list(self): - build = get_build_config({'python': {}}, {}) + build = get_build_config({'python': {}}) build.validate() # Default is an empty list. assert build.python.extra_requirements == [] @@ -222,7 +222,6 @@ def test_it_defaults_to_list(self): def test_it_validates_is_a_list(self): build = get_build_config( {'python': {'extra_requirements': 'invalid'}}, - {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -239,7 +238,6 @@ def test_it_uses_validate_string(self, validate_string): 'extra_requirements': ['tests'], }, }, - {}, ) build.validate() validate_string.assert_any_call('tests') @@ -248,14 +246,13 @@ def test_it_uses_validate_string(self, validate_string): class TestValidateUseSystemSitePackages(object): def test_it_defaults_to_false(self): - build = get_build_config({'python': {}}, {}) + build = get_build_config({'python': {}}) build.validate() assert build.python.use_system_site_packages is False def test_it_validates_value(self): build = get_build_config( {'python': {'use_system_site_packages': 'invalid'}}, - {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -267,7 +264,6 @@ def test_it_uses_validate_bool(self, validate_bool): validate_bool.return_value = True build = get_build_config( {'python': {'use_system_site_packages': 'to-validate'}}, - {}, ) build.validate() validate_bool.assert_any_call('to-validate') @@ -276,14 +272,13 @@ def test_it_uses_validate_bool(self, validate_bool): class TestValidateSetupPyInstall(object): def test_it_defaults_to_false(self): - build = get_build_config({'python': {}}, {}) + build = get_build_config({'python': {}}) build.validate() assert build.python.install_with_setup is False def test_it_validates_value(self): build = get_build_config( {'python': {'setup_py_install': 'this-is-string'}}, - {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -295,7 +290,6 @@ def test_it_uses_validate_bool(self, validate_bool): validate_bool.return_value = True build = get_build_config( {'python': {'setup_py_install': 'to-validate'}}, - {}, ) build.validate() validate_bool.assert_any_call('to-validate') @@ -304,7 +298,7 @@ def test_it_uses_validate_bool(self, validate_bool): class TestValidatePythonVersion(object): def test_it_defaults_to_a_valid_version(self): - build = get_build_config({'python': {}}, {}) + build = get_build_config({'python': {}}) build.validate() assert build.python.version == 2 assert build.python_interpreter == 'python2.7' @@ -313,7 +307,6 @@ def test_it_defaults_to_a_valid_version(self): def test_it_supports_other_versions(self): build = get_build_config( {'python': {'version': 3.5}}, - {}, ) build.validate() assert build.python.version == 3.5 @@ -323,7 +316,6 @@ def test_it_supports_other_versions(self): def test_it_validates_versions_out_of_range(self): build = get_build_config( {'python': {'version': 1.0}}, - {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -333,7 +325,6 @@ def test_it_validates_versions_out_of_range(self): def test_it_validates_wrong_type(self): build = get_build_config( {'python': {'version': 'this-is-string'}}, - {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -343,7 +334,6 @@ def test_it_validates_wrong_type(self): def test_it_validates_wrong_type_right_value(self): build = get_build_config( {'python': {'version': '3.5'}}, - {}, ) build.validate() assert build.python.version == 3.5 @@ -352,7 +342,6 @@ def test_it_validates_wrong_type_right_value(self): build = get_build_config( {'python': {'version': '3'}}, - {}, ) build.validate() assert build.python.version == 3 @@ -400,34 +389,33 @@ def test_it_respects_default_value(self, value): class TestValidateFormats(object): def test_it_defaults_to_empty(self): - build = get_build_config({}, {}) + build = get_build_config({}) build.validate() assert build.formats == [] def test_it_gets_set_correctly(self): - build = get_build_config({'formats': ['pdf']}, {}) + build = get_build_config({'formats': ['pdf']}) build.validate() assert build.formats == ['pdf'] def test_formats_can_be_null(self): - build = get_build_config({'formats': None}, {}) + build = get_build_config({'formats': None}) build.validate() assert build.formats == [] def test_formats_with_previous_none(self): - build = get_build_config({'formats': ['none']}, {}) + build = get_build_config({'formats': ['none']}) build.validate() assert build.formats == [] def test_formats_can_be_empty(self): - build = get_build_config({'formats': []}, {}) + build = get_build_config({'formats': []}) build.validate() assert build.formats == [] def test_all_valid_formats(self): build = get_build_config( {'formats': ['pdf', 'htmlzip', 'epub']}, - {}, ) build.validate() assert build.formats == ['pdf', 'htmlzip', 'epub'] @@ -435,7 +423,6 @@ def test_all_valid_formats(self): def test_cant_have_none_as_format(self): build = get_build_config( {'formats': ['htmlzip', None]}, - {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -445,7 +432,6 @@ def test_cant_have_none_as_format(self): def test_formats_have_only_allowed_values(self): build = get_build_config( {'formats': ['htmlzip', 'csv']}, - {}, ) with raises(InvalidConfig) as excinfo: build.validate() @@ -453,7 +439,7 @@ def test_formats_have_only_allowed_values(self): assert excinfo.value.code == INVALID_CHOICE def test_only_list_type(self): - build = get_build_config({'formats': 'no-list'}, {}) + build = get_build_config({'formats': 'no-list'}) with raises(InvalidConfig) as excinfo: build.validate() assert excinfo.value.key == 'format' @@ -553,7 +539,7 @@ def test_it_priorities_image_from_env_config(self, tmpdir, image): def test_use_conda_default_false(): - build = get_build_config({}, {}) + build = get_build_config({}) build.validate() assert build.conda is None @@ -561,7 +547,6 @@ def test_use_conda_default_false(): def test_use_conda_respects_config(): build = get_build_config( {'conda': {}}, - {}, ) build.validate() assert isinstance(build.conda, Conda) @@ -571,7 +556,6 @@ def test_validates_conda_file(tmpdir): apply_fs(tmpdir, {'environment.yml': ''}) build = get_build_config( {'conda': {'file': 'environment.yml'}}, - {}, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() @@ -580,7 +564,7 @@ def test_validates_conda_file(tmpdir): def test_requirements_file_empty(): - build = get_build_config({}, {}) + build = get_build_config({}) build.validate() assert build.python.requirements is None @@ -603,7 +587,6 @@ def test_requirements_file_respects_configuration(tmpdir): apply_fs(tmpdir, {'requirements.txt': ''}) build = get_build_config( {'requirements_file': 'requirements.txt'}, - {}, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() @@ -613,7 +596,6 @@ def test_requirements_file_respects_configuration(tmpdir): def test_requirements_file_is_null(tmpdir): build = get_build_config( {'requirements_file': None}, - {}, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() @@ -623,7 +605,6 @@ def test_requirements_file_is_null(tmpdir): def test_requirements_file_is_blank(tmpdir): build = get_build_config( {'requirements_file': ''}, - {}, source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() @@ -654,7 +635,7 @@ def test_load_calls_validate(tmpdir): def test_raise_config_not_supported(): - build = get_build_config({}, {}) + build = get_build_config({}) build.validate() with raises(ConfigOptionNotSupportedError) as excinfo: build.redirects diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 2b2c6ceb489..95cdefb54d4 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -88,8 +88,6 @@ def test_python_supported_versions_default_image_1_0(self, load_config): env_config={ 'allow_v2': mock.ANY, 'build': {'image': 'readthedocs/build:1.0'}, - 'output_base': '', - 'name': mock.ANY, 'defaults': { 'install_project': self.project.install_project, 'formats': [