From 5a707e97a9510bbf02483c2afb2f6cb609895222 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 31 Jul 2018 18:58:48 -0500 Subject: [PATCH 01/28] Add integratio tests for config file v2 --- .../tests/test_config_integration.py | 269 +++++++++++++++++- 1 file changed, 268 insertions(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index a1fa306c28f..97d713fb731 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -2,14 +2,23 @@ from __future__ import ( absolute_import, division, print_function, unicode_literals) +from os import path + import mock +import pytest +import yaml from django.test import TestCase from django_dynamic_fixture import get +from mock import MagicMock, PropertyMock, patch from readthedocs.builds.models import Version from readthedocs.config import BuildConfigV1, InvalidConfig, ProjectConfig +from readthedocs.config.tests.utils import apply_fs from readthedocs.doc_builder.config import load_yaml_config -from readthedocs.projects.models import Project +from readthedocs.doc_builder.environments import LocalBuildEnvironment +from readthedocs.doc_builder.python_environments import Conda, Virtualenv +from readthedocs.projects import tasks +from readthedocs.projects.models import Feature, Project def create_load(config=None): @@ -219,3 +228,261 @@ def test_requirements_file(self, load_config): load_config.side_effect = create_load() config = load_yaml_config(self.version) self.assertEqual(config.requirements_file, '__init__.py') + + +@pytest.mark.django_db +@mock.patch('readthedocs.projects.models.Project.checkout_path') +class TestLoadConfigV2(object): + + @pytest.fixture(autouse=True) + def create_project(self): + self.project = get( + Project, + main_language_project=None, + install_project=False, + container_image=None, + ) + self.version = get(Version, project=self.project) + # TODO: Remove later + get( + Feature, + projects=[self.project], + feature_id=Feature.ALLOW_V2_CONFIG_FILE, + ) + + def create_config_file(self, tmpdir, config): + base_path = apply_fs(tmpdir, { + 'readthedocs.yml': '', + }) + config.setdefault('version', 2) + config_file = path.join(str(base_path), 'readthedocs.yml') + yaml.safe_dump(config, open(config_file, 'w')) + return base_path + + def get_update_docs_task(self): + build_env = LocalBuildEnvironment( + self.project, self.version, record=False + ) + + update_docs = tasks.UpdateDocsTaskStep( + build_env=build_env, + config=load_yaml_config(self.version), + project=self.project, + version=self.version, + ) + return update_docs + + def test_using_v2(self, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file(tmpdir, {}) + update_docs = self.get_update_docs_task() + assert update_docs.config.version == '2' + + @pytest.mark.parametrize('config', [{}, {'formats': []}]) + @patch('readthedocs.projects.models.Project.repo_nonblockinglock', new=MagicMock()) + @patch('readthedocs.doc_builder.backends.sphinx.SearchBuilder.build') + @patch('readthedocs.doc_builder.backends.sphinx.HtmlBuilder.build') + @patch('readthedocs.doc_builder.backends.sphinx.HtmlBuilder.append_conf') + def test_build_formats_default_empty( + self, append_conf, html_build, search_build, + checkout_path, config, tmpdir): + """ + The default value for formats is [], which means no extra + formats are build. + """ + checkout_path.return_value = str(tmpdir) + self.create_config_file(tmpdir, config) + + update_docs = self.get_update_docs_task() + outcomes = update_docs.build_docs() + + # No extra formats were triggered + assert outcomes['html'] + assert outcomes['search'] + assert not outcomes['localmedia'] + assert not outcomes['pdf'] + assert not outcomes['epub'] + + @patch('readthedocs.projects.models.Project.repo_nonblockinglock', new=MagicMock()) + @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.build_docs_class') + @patch('readthedocs.doc_builder.backends.sphinx.SearchBuilder.build') + @patch('readthedocs.doc_builder.backends.sphinx.HtmlBuilder.build') + @patch('readthedocs.doc_builder.backends.sphinx.HtmlBuilder.append_conf') + def test_build_formats_only_pdf( + self, append_conf, html_build, search_build, build_docs_class, + checkout_path, tmpdir): + """ + Only the pdf format is build. + """ + checkout_path.return_value = str(tmpdir) + self.create_config_file(tmpdir, {'formats': ['pdf']}) + + update_docs = self.get_update_docs_task() + outcomes = update_docs.build_docs() + + # Only pdf extra format was triggered + assert outcomes['html'] + assert outcomes['search'] + build_docs_class.assert_called_with('sphinx_pdf') + assert outcomes['pdf'] + assert not outcomes['localmedia'] + assert not outcomes['epub'] + + @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.update_documentation_type', new=MagicMock()) + @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.setup_python_environment', new=MagicMock()) + @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.build_docs', new=MagicMock()) + @patch('readthedocs.doc_builder.environments.BuildEnvironment.failed', new_callable=PropertyMock) + def test_conda_environment(self, build_failed, checkout_path, tmpdir): + build_failed.return_value = False + checkout_path.return_value = str(tmpdir) + conda_file = 'environmemt.yml' + apply_fs(tmpdir, {conda_file: ''}) + base_path = self.create_config_file( + tmpdir, + { + 'conda': {'environment': conda_file} + } + ) + + update_docs = self.get_update_docs_task() + update_docs.run_build(docker=False, record=False) + + conda_file = path.join(str(base_path), conda_file) + assert update_docs.config.conda.environment == conda_file + assert isinstance(update_docs.python_env, Conda) + + def test_default_build_image(self, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + build_image = 'readthedocs/build:latest' + self.create_config_file(tmpdir, {}) + update_docs = self.get_update_docs_task() + assert update_docs.config.build.image == build_image + + def test_build_image(self, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + build_image = 'readthedocs/build:stable' + self.create_config_file( + tmpdir, + {'build': {'image': 'stable'}}, + ) + update_docs = self.get_update_docs_task() + assert update_docs.config.build.image == build_image + + def test_custom_build_image(self, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + + build_image = 'readthedocs/build:3.0' + self.project.container_image = build_image + self.project.save() + + self.create_config_file(tmpdir, {}) + update_docs = self.get_update_docs_task() + assert update_docs.config.build.image == build_image + + def test_python_version(self, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file(tmpdir, {}) + # The default version is always 3 + self.project.python_interpreter = 'python2' + self.project.save() + + config = self.get_update_docs_task().config + assert config.python.version == 3 + assert config.python_full_version == 3.6 + + @patch('readthedocs.doc_builder.environments.BuildEnvironment.run') + def test_python_requirements(self, run, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + requirements_file = 'requirements.txt' + apply_fs(tmpdir, {requirements_file: ''}) + base_path = self.create_config_file( + tmpdir, + { + 'python': {'requirements': requirements_file} + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + update_docs.python_env.install_user_requirements() + + args, kwargs = run.call_args + requirements_file = path.join(str(base_path), requirements_file) + + assert config.python.requirements == requirements_file + assert requirements_file in args + + @patch('readthedocs.doc_builder.environments.BuildEnvironment.run') + def test_python_install_setup(self, run, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file( + tmpdir, + { + 'python': {'install': 'setup.py'} + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + update_docs.python_env.install_package() + + args, kwargs = run.call_args + + assert 'setup.py' in args + assert 'install' in args + assert config.python.install_with_setup + assert not config.python.install_with_pip + + @patch('readthedocs.doc_builder.environments.BuildEnvironment.run') + def test_python_install_pip(self, run, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file( + tmpdir, + { + 'python': {'install': 'pip'} + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + update_docs.python_env.install_package() + + args, kwargs = run.call_args + + assert 'setup.py' not in args + assert 'install' in args + assert config.python.install_with_pip + assert not config.python.install_with_setup + + def test_python_install_project(self, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file(tmpdir, {}) + + self.project.install_project = True + self.project.save() + + config = self.get_update_docs_task().config + + assert config.python.install_with_setup + assert not config.python.install_with_pip From 8b564b69533bacee36d15dcd7c74cc16d6a82800 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 31 Jul 2018 19:27:08 -0500 Subject: [PATCH 02/28] Complete tests for python --- .../tests/test_config_integration.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 97d713fb731..4e79aa64d0d 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -486,3 +486,63 @@ def test_python_install_project(self, checkout_path, tmpdir): assert config.python.install_with_setup assert not config.python.install_with_pip + + @patch('readthedocs.doc_builder.environments.BuildEnvironment.run') + def test_python_extra_requirements(self, run, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file( + tmpdir, + { + 'python': { + 'install': 'pip', + 'extra_requirements': ['docs'], + } + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + update_docs.python_env.install_package() + + args, kwargs = run.call_args + + assert 'setup.py' not in args + assert 'install' in args + assert '.[docs]' in args + assert config.python.install_with_pip + assert not config.python.install_with_setup + + @patch('readthedocs.doc_builder.environments.BuildEnvironment.run') + def test_system_packages(self, run, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file( + tmpdir, + { + 'python': { + 'system_packages': True, + } + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + update_docs.python_env.setup_base() + + args, kwargs = run.call_args + + assert '--system-site-packages' in args + assert config.python.use_system_site_packages From c15c5398651bec958feafdf124f53724d944e964 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 11:50:29 -0500 Subject: [PATCH 03/28] Change external keys to python scope v1 --- readthedocs/config/config.py | 68 +++------------------ readthedocs/config/models.py | 15 +++++ readthedocs/config/tests/test_config.py | 80 ++++++------------------- 3 files changed, 40 insertions(+), 123 deletions(-) create mode 100644 readthedocs/config/models.py diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 10a32f5b945..dd831022462 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -18,6 +18,7 @@ ValidationError, validate_bool, validate_choice, validate_dict, validate_directory, validate_file, validate_list, validate_string, validate_value_exists) +from .models import Python __all__ = ( 'ALL', @@ -177,7 +178,7 @@ def python_interpreter(self): @property def python_full_version(self): - ver = self.python_version + ver = self.python.version if ver in [2, 3]: # Get the highest version of the major series version if user only # gave us a version of '2', or '3' @@ -361,12 +362,9 @@ def validate_python(self): version = self.defaults.get('python_version', 2) python = { 'use_system_site_packages': use_system_packages, - 'pip_install': False, + 'install_with_pip': False, 'extra_requirements': [], - 'setup_py_install': install_project, - 'setup_py_path': os.path.join( - os.path.dirname(self.source_file), - 'setup.py'), + 'install_with_setup': install_project, 'version': version, } @@ -388,7 +386,7 @@ def validate_python(self): # Validate pip_install. if 'pip_install' in raw_python: with self.catch_validation_error('python.pip_install'): - python['pip_install'] = validate_bool( + python['install_with_pip'] = validate_bool( raw_python['pip_install']) # Validate extra_requirements. @@ -412,13 +410,6 @@ def validate_python(self): 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 @@ -511,36 +502,9 @@ def formats(self): @property def python(self): """Python related configuration.""" - return self._config.get('python', {}) - - @property - def python_version(self): - """Python version.""" - return self._config['python']['version'] - - @property - def pip_install(self): - """True if the project should be installed using pip.""" - return self._config['python']['pip_install'] - - @property - def install_project(self): - """True if the project should be installed.""" - if self.pip_install: - return True - return self._config['python']['setup_py_install'] - - @property - def extra_requirements(self): - """Extra requirements to be installed with pip.""" - if self.pip_install: - return self._config['python']['extra_requirements'] - return [] - - @property - def use_system_site_packages(self): - """True if the project should have access to the system packages.""" - return self._config['python']['use_system_site_packages'] + requirements = self._config['requirements_file'] + self._config['python']['requirements'] = requirements + return Python(**self._config['python']) @property def use_conda(self): @@ -554,11 +518,6 @@ def conda_file(self): return self._config['conda'].get('file') return None - @property - def requirements_file(self): - """The project requirements file.""" - return self._config['requirements_file'] - @property def build_image(self): """The docker image used by the builders.""" @@ -900,17 +859,6 @@ def build(self): @property def python(self): - Python = namedtuple( # noqa - 'Python', - [ - 'version', - 'requirements', - 'install_with_pip', - 'install_with_setup', - 'extra_requirements', - 'use_system_site_packages', - ], - ) return Python(**self._config['python']) @property diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py new file mode 100644 index 00000000000..d904ae4fd56 --- /dev/null +++ b/readthedocs/config/models.py @@ -0,0 +1,15 @@ +from __future__ import division, print_function, unicode_literals + +from collections import namedtuple + +Python = namedtuple( # noqa + 'Python', + [ + 'version', + 'requirements', + 'install_with_pip', + 'install_with_setup', + 'extra_requirements', + 'use_system_site_packages', + ], +) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index ddbf59c2eac..633eba1f59d 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -212,7 +212,7 @@ def test_use_system_site_packages_defaults_to_false(): build = get_build_config({'python': {}}, get_env_config()) build.validate() # Default is False. - assert not build.use_system_site_packages + assert not build.python.use_system_site_packages @pytest.mark.parametrize('value', [True, False]) @@ -222,14 +222,14 @@ def test_use_system_site_packages_repects_default_value(value): } build = get_build_config({}, get_env_config({'defaults': defaults})) build.validate() - assert build.use_system_site_packages is value + assert build.python.use_system_site_packages is value def test_python_pip_install_default(): build = get_build_config({'python': {}}, get_env_config()) build.validate() # Default is False. - assert build.pip_install is False + assert build.python.install_with_pip is False def describe_validate_python_extra_requirements(): @@ -238,7 +238,7 @@ def it_defaults_to_list(): build = get_build_config({'python': {}}, get_env_config()) build.validate() # Default is an empty list. - assert build.extra_requirements == [] + assert build.python.extra_requirements == [] def it_validates_is_a_list(): build = get_build_config( @@ -266,7 +266,7 @@ def describe_validate_use_system_site_packages(): def it_defaults_to_false(): build = get_build_config({'python': {}}, get_env_config()) build.validate() - assert build.use_system_site_packages is False + assert build.python.use_system_site_packages is False def it_validates_value(): build = get_build_config( @@ -294,7 +294,7 @@ def describe_validate_setup_py_install(): def it_defaults_to_false(): build = get_build_config({'python': {}}, get_env_config()) build.validate() - assert build.python['setup_py_install'] is False + assert build.python.install_with_setup is False def it_validates_value(): build = get_build_config( @@ -322,7 +322,7 @@ def describe_validate_python_version(): def it_defaults_to_a_valid_version(): build = get_build_config({'python': {}}, get_env_config()) build.validate() - assert build.python_version == 2 + assert build.python.version == 2 assert build.python_interpreter == 'python2.7' assert build.python_full_version == 2.7 @@ -332,7 +332,7 @@ def it_supports_other_versions(): get_env_config(), ) build.validate() - assert build.python_version == 3.5 + assert build.python.version == 3.5 assert build.python_interpreter == 'python3.5' assert build.python_full_version == 3.5 @@ -362,7 +362,7 @@ def it_validates_wrong_type_right_value(): get_env_config(), ) build.validate() - assert build.python_version == 3.5 + assert build.python.version == 3.5 assert build.python_interpreter == 'python3.5' assert build.python_full_version == 3.5 @@ -371,7 +371,7 @@ def it_validates_wrong_type_right_value(): get_env_config(), ) build.validate() - assert build.python_version == 3 + assert build.python.version == 3 assert build.python_interpreter == 'python3.5' assert build.python_full_version == 3.5 @@ -400,7 +400,7 @@ def it_validates_env_supported_versions(): ) ) build.validate() - assert build.python_version == 3.6 + assert build.python.version == 3.6 assert build.python_interpreter == 'python3.6' assert build.python_full_version == 3.6 @@ -414,7 +414,7 @@ def it_respects_default_value(value): get_env_config({'defaults': defaults}), ) build.validate() - assert build.python_version == value + assert build.python.version == value def describe_validate_formats(): @@ -480,52 +480,6 @@ def only_list_type(): assert excinfo.value.code == INVALID_LIST -def describe_validate_setup_py_path(): - - def it_defaults_to_source_file_directory(tmpdir): - apply_fs( - tmpdir, - { - 'subdir': { - 'readthedocs.yml': '', - 'setup.py': '', - }, - }, - ) - with tmpdir.as_cwd(): - source_file = tmpdir.join('subdir', 'readthedocs.yml') - setup_py = tmpdir.join('subdir', 'setup.py') - build = get_build_config( - {}, - env_config=get_env_config(), - source_file=str(source_file), - ) - build.validate() - 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.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 = BuildConfigV1( env_config, @@ -537,8 +491,8 @@ def test_valid_build_config(): assert build.name == 'docs' assert build.base assert build.python - assert 'setup_py_install' in build.python - assert 'use_system_site_packages' in build.python + assert build.python.install_with_setup is False + assert build.python.install_with_pip is False assert build.output_base @@ -709,7 +663,7 @@ def test_validates_conda_file(tmpdir): def test_requirements_file_empty(): build = get_build_config({}, get_env_config()) build.validate() - assert build.requirements_file is None + assert build.python.requirements is None def test_requirements_file_repects_default_value(tmpdir): @@ -723,7 +677,7 @@ def test_requirements_file_repects_default_value(tmpdir): source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() - assert build.requirements_file == 'myrequirements.txt' + assert build.python.requirements == 'myrequirements.txt' def test_requirements_file_respects_configuration(tmpdir): @@ -734,7 +688,7 @@ def test_requirements_file_respects_configuration(tmpdir): source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() - assert build.requirements_file == 'requirements.txt' + assert build.python.requirements == 'requirements.txt' def test_build_validate_calls_all_subvalidators(tmpdir): From 229daf743ef02f610aa320e87b2e5b558f3069a3 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 12:13:44 -0500 Subject: [PATCH 04/28] Fix tests --- readthedocs/config/config.py | 17 ++++++++++------- readthedocs/config/tests/test_config.py | 7 ++++++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index dd831022462..a0627455a32 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -397,17 +397,20 @@ def validate_python(self): '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) - ) + if not python['install_with_pip']: + python['extra_requirements'] = [] + else: + 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( + python['install_with_setup'] = validate_bool( raw_python['setup_py_install']) if 'version' in raw_python: diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 633eba1f59d..fb9b282d0df 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -254,7 +254,12 @@ def it_validates_is_a_list(): def it_uses_validate_string(validate_string): validate_string.return_value = True build = get_build_config( - {'python': {'extra_requirements': ['tests']}}, + { + 'python': { + 'pip_install': True, + 'extra_requirements': ['tests'] + } + }, get_env_config(), ) build.validate() From 8edadf0bccfeb7dd6bef86989793ebdebcc46984 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 12:14:37 -0500 Subject: [PATCH 05/28] Adapt tests from v1 --- .../tests/test_config_integration.py | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 4e79aa64d0d..94d1065bc74 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -98,7 +98,7 @@ def test_python_supported_versions_default_image_1_0(self, load_config): }, ), ]) - self.assertEqual(config.python_version, 2) + self.assertEqual(config.python.version, 2) def test_python_supported_versions_image_1_0(self, load_config): load_config.side_effect = create_load() @@ -127,7 +127,7 @@ def test_python_supported_versions_image_latest(self, load_config): def test_python_default_version(self, load_config): load_config.side_effect = create_load() config = load_yaml_config(self.version) - self.assertEqual(config.python_version, 2) + self.assertEqual(config.python.version, 2) self.assertEqual(config.python_interpreter, 'python2.7') def test_python_set_python_version_on_project(self, load_config): @@ -136,7 +136,7 @@ def test_python_set_python_version_on_project(self, load_config): self.project.python_interpreter = 'python3' self.project.save() config = load_yaml_config(self.version) - self.assertEqual(config.python_version, 3) + self.assertEqual(config.python.version, 3) self.assertEqual(config.python_interpreter, 'python3.5') def test_python_set_python_version_in_config(self, load_config): @@ -146,7 +146,7 @@ def test_python_set_python_version_in_config(self, load_config): self.project.container_image = 'readthedocs/build:2.0' self.project.save() config = load_yaml_config(self.version) - self.assertEqual(config.python_version, 3.5) + self.assertEqual(config.python.version, 3.5) self.assertEqual(config.python_interpreter, 'python3.5') def test_python_invalid_version_in_config(self, load_config): @@ -161,13 +161,16 @@ def test_python_invalid_version_in_config(self, load_config): def test_install_project(self, load_config): load_config.side_effect = create_load() config = load_yaml_config(self.version) - self.assertEqual(config.install_project, False) + self.assertEqual( + config.python.install_with_pip and config.python.install_with_setup, + False + ) load_config.side_effect = create_load({ 'python': {'setup_py_install': True} }) config = load_yaml_config(self.version) - self.assertEqual(config.install_project, True) + self.assertEqual(config.python.install_with_setup, True) def test_extra_requirements(self, load_config): load_config.side_effect = create_load({ @@ -177,7 +180,7 @@ def test_extra_requirements(self, load_config): } }) config = load_yaml_config(self.version) - self.assertEqual(config.extra_requirements, ['tests', 'docs']) + self.assertEqual(config.python.extra_requirements, ['tests', 'docs']) load_config.side_effect = create_load({ 'python': { @@ -185,11 +188,11 @@ def test_extra_requirements(self, load_config): } }) config = load_yaml_config(self.version) - self.assertEqual(config.extra_requirements, []) + self.assertEqual(config.python.extra_requirements, []) load_config.side_effect = create_load() config = load_yaml_config(self.version) - self.assertEqual(config.extra_requirements, []) + self.assertEqual(config.python.extra_requirements, []) load_config.side_effect = create_load({ 'python': { @@ -198,7 +201,7 @@ def test_extra_requirements(self, load_config): } }) config = load_yaml_config(self.version) - self.assertEqual(config.extra_requirements, []) + self.assertEqual(config.python.extra_requirements, []) def test_conda(self, load_config): to_find = '__init__.py' @@ -222,12 +225,12 @@ def test_requirements_file(self, load_config): 'requirements_file': requirements_file }) config = load_yaml_config(self.version) - self.assertEqual(config.requirements_file, requirements_file) + self.assertEqual(config.python.requirements, requirements_file) # Respects the requirements file from the project settings load_config.side_effect = create_load() config = load_yaml_config(self.version) - self.assertEqual(config.requirements_file, '__init__.py') + self.assertEqual(config.python.requirements, '__init__.py') @pytest.mark.django_db From 73731d817d88afef694e8007520450fad7f12203 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 12:22:22 -0500 Subject: [PATCH 06/28] Skip new tests --- readthedocs/rtd_tests/tests/test_config_integration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 94d1065bc74..67b30af7a00 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -233,6 +233,7 @@ def test_requirements_file(self, load_config): self.assertEqual(config.python.requirements, '__init__.py') +@pytest.mark.skip('No implemented yet') @pytest.mark.django_db @mock.patch('readthedocs.projects.models.Project.checkout_path') class TestLoadConfigV2(object): From 6f6a4cfcd891229558c603b46dc2546699745e0c Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 12:37:52 -0500 Subject: [PATCH 07/28] Refactor code to use new python options --- readthedocs/doc_builder/python_environments.py | 9 +++++---- readthedocs/rtd_tests/tests/test_doc_building.py | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index faaee70e3d1..13d1dba539f 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -203,7 +203,7 @@ def venv_path(self): def setup_base(self): site_packages = '--no-site-packages' - if self.config.use_system_site_packages: + if self.config.python.use_system_site_packages: site_packages = '--system-site-packages' env_path = self.venv_path() self.build_env.run( @@ -257,7 +257,7 @@ def install_core_requirements(self): '--cache-dir', self.project.pip_cache_path, ] - if self.config.use_system_site_packages: + if self.config.python.use_system_site_packages: # Other code expects sphinx-build to be installed inside the # virtualenv. Using the -I option makes sure it gets installed # even if it is already installed system-wide (and @@ -270,7 +270,7 @@ def install_core_requirements(self): ) def install_user_requirements(self): - requirements_file_path = self.config.requirements_file + requirements_file_path = self.config.python.requirements if not requirements_file_path: builder_class = get_builder_class(self.project.documentation_type) docs_dir = (builder_class(build_env=self.build_env, python_env=self) @@ -295,7 +295,8 @@ def install_user_requirements(self): '--exists-action=w', '--cache-dir', self.project.pip_cache_path, - '-r{0}'.format(requirements_file_path), + '-r', + requirements_file_path, ] self.build_env.run( *args, diff --git a/readthedocs/rtd_tests/tests/test_doc_building.py b/readthedocs/rtd_tests/tests/test_doc_building.py index 8d809e94778..563b9bc9c5b 100644 --- a/readthedocs/rtd_tests/tests/test_doc_building.py +++ b/readthedocs/rtd_tests/tests/test_doc_building.py @@ -1231,6 +1231,7 @@ def test_install_user_requirements(self, checkout_path): '--exists-action=w', '--cache-dir', mock.ANY, # cache path + '-r', 'requirements_file' ] @@ -1240,7 +1241,7 @@ def test_install_user_requirements(self, checkout_path): paths[root_requirements] = False with fake_paths_lookup(paths): python_env.install_user_requirements() - args[-1] = '-r{}'.format(docs_requirements) + args[-1] = docs_requirements self.build_env_mock.run.assert_called_with( *args, cwd=mock.ANY, bin_path=mock.ANY ) @@ -1251,7 +1252,7 @@ def test_install_user_requirements(self, checkout_path): paths[root_requirements] = True with fake_paths_lookup(paths): python_env.install_user_requirements() - args[-1] = '-r{}'.format(root_requirements) + args[-1] = root_requirements self.build_env_mock.run.assert_called_with( *args, cwd=mock.ANY, bin_path=mock.ANY ) @@ -1262,7 +1263,7 @@ def test_install_user_requirements(self, checkout_path): paths[root_requirements] = True with fake_paths_lookup(paths): python_env.install_user_requirements() - args[-1] = '-r{}'.format(docs_requirements) + args[-1] = docs_requirements self.build_env_mock.run.assert_called_with( *args, cwd=mock.ANY, bin_path=mock.ANY ) From 41e5fb7dce6de5a105d603bb69dca80a3f32d6eb Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 12:54:31 -0500 Subject: [PATCH 08/28] Refactor conda option from v1 --- readthedocs/config/config.py | 22 +++++++++------------- readthedocs/config/models.py | 2 ++ readthedocs/config/tests/test_config.py | 8 ++++---- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index a0627455a32..c37461104f5 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -18,7 +18,7 @@ ValidationError, validate_bool, validate_choice, validate_dict, validate_directory, validate_file, validate_list, validate_string, validate_value_exists) -from .models import Python +from .models import Conda, Python __all__ = ( 'ALL', @@ -445,11 +445,14 @@ def validate_conda(self): self.PYTHON_INVALID_MESSAGE, code=PYTHON_INVALID) + conda_environment = None 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) + conda_environment = validate_file( + raw_conda['file'], base_path + ) + conda['environment'] = conda_environment return conda return None @@ -510,15 +513,9 @@ def python(self): return Python(**self._config['python']) @property - def use_conda(self): - """True if the project use Conda.""" - return self._config.get('conda') is not None - - @property - def conda_file(self): - """The Conda environment file.""" - if self.use_conda: - return self._config['conda'].get('file') + def conda(self): + if self._config['conda'] is not None: + return Conda(**self._config['conda']) return None @property @@ -850,7 +847,6 @@ def formats(self): @property def conda(self): - Conda = namedtuple('Conda', ['environment']) # noqa if self._config['conda']: return Conda(**self._config['conda']) return None diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py index d904ae4fd56..72c7e475b0d 100644 --- a/readthedocs/config/models.py +++ b/readthedocs/config/models.py @@ -13,3 +13,5 @@ 'use_system_site_packages', ], ) + +Conda = namedtuple('Conda', ['environment']) # noqa diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index fb9b282d0df..3480070b5d4 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -641,7 +641,7 @@ def it_priorities_image_from_env_config(tmpdir, image): def test_use_conda_default_false(): build = get_build_config({}, get_env_config()) build.validate() - assert build.use_conda is False + assert build.conda is None def test_use_conda_respects_config(): @@ -650,7 +650,7 @@ def test_use_conda_respects_config(): get_env_config(), ) build.validate() - assert build.use_conda is True + assert build.conda is not None def test_validates_conda_file(tmpdir): @@ -661,8 +661,8 @@ def test_validates_conda_file(tmpdir): source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() - assert build.use_conda is True - assert build.conda_file == str(tmpdir.join('environment.yml')) + assert build.conda is not None + assert build.conda.environment == str(tmpdir.join('environment.yml')) def test_requirements_file_empty(): From 14e931a5b886622b1698baf18101d773200f7bef Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 13:03:31 -0500 Subject: [PATCH 09/28] Use new conda option in tests --- readthedocs/rtd_tests/tests/test_config_integration.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 67b30af7a00..71818bf3d35 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -211,13 +211,12 @@ def test_conda(self, load_config): } }) config = load_yaml_config(self.version) - self.assertEqual(config.use_conda, True) - self.assertTrue(config.conda_file[-len(to_find):] == to_find) + self.assertTrue(config.conda is not None) + self.assertTrue(config.conda.environment[-len(to_find):] == to_find) load_config.side_effect = create_load() config = load_yaml_config(self.version) - self.assertEqual(config.use_conda, False) - self.assertEqual(config.conda_file, None) + self.assertIsNone(config.conda) def test_requirements_file(self, load_config): requirements_file = '__init__.py' From 64df5ed4867a57ad7f0447fab8226c57d2bb9fb3 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 13:07:22 -0500 Subject: [PATCH 10/28] Linter --- readthedocs/config/models.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py index 72c7e475b0d..a6494c544c1 100644 --- a/readthedocs/config/models.py +++ b/readthedocs/config/models.py @@ -1,3 +1,5 @@ +"""Models for the response of the configuration object.""" + from __future__ import division, print_function, unicode_literals from collections import namedtuple From 45c2cff3dcb82c603dad64d338c9e35b4924782d Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 13:20:39 -0500 Subject: [PATCH 11/28] Use new configurations --- .../doc_builder/python_environments.py | 51 ++++++++++--------- readthedocs/projects/tasks.py | 22 +++++--- 2 files changed, 42 insertions(+), 31 deletions(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 13d1dba539f..960f0e24387 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -62,32 +62,33 @@ def delete_existing_venv_dir(self): shutil.rmtree(venv_dir) def install_package(self): - if self.config.install_project: - if self.config.pip_install or getattr(settings, 'USE_PIP_INSTALL', False): - extra_req_param = '' - if self.config.extra_requirements: - extra_req_param = '[{0}]'.format( - ','.join(self.config.extra_requirements)) - self.build_env.run( - 'python', - self.venv_bin(filename='pip'), - 'install', - '--ignore-installed', - '--cache-dir', - self.project.pip_cache_path, - '.{0}'.format(extra_req_param), - cwd=self.checkout_path, - bin_path=self.venv_bin() - ) - else: - self.build_env.run( - 'python', - 'setup.py', - 'install', - '--force', - cwd=self.checkout_path, - bin_path=self.venv_bin() + if (self.config.python.install_with_pip or + getattr(settings, 'USE_PIP_INSTALL', False)): + extra_req_param = '' + if self.config.python.extra_requirements: + extra_req_param = '[{0}]'.format( + ','.join(self.config.python.extra_requirements) ) + self.build_env.run( + 'python', + self.venv_bin(filename='pip'), + 'install', + '--ignore-installed', + '--cache-dir', + self.project.pip_cache_path, + '.{0}'.format(extra_req_param), + cwd=self.checkout_path, + bin_path=self.venv_bin() + ) + elif self.config.python.install_with_setup: + self.build_env.run( + 'python', + 'setup.py', + 'install', + '--force', + cwd=self.checkout_path, + bin_path=self.venv_bin() + ) def venv_bin(self, filename=None): """ diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index d781311d9eb..2bdf1668113 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -466,7 +466,7 @@ def run_build(self, docker, record): self.update_documentation_type() python_env_cls = Virtualenv - if self.config.use_conda: + if self.config.conda: self._log('Using conda') python_env_cls = Conda self.python_env = python_env_cls(version=self.version, @@ -476,8 +476,8 @@ def run_build(self, docker, record): try: self.setup_python_environment() - # TODO the build object should have an idea of these states, extend - # the model to include an idea of these outcomes + # TODO the build object should have an idea of these states, + # extend the model to include an idea of these outcomes outcomes = self.build_docs() build_id = self.build.get('id') except vcs_support_utils.LockTimeout as e: @@ -550,15 +550,25 @@ def get_env_vars(self): 'READTHEDOCS_PROJECT': self.project.slug } - if self.config.use_conda: + if self.config.conda: env.update({ 'CONDA_ENVS_PATH': os.path.join(self.project.doc_path, 'conda'), 'CONDA_DEFAULT_ENV': self.version.slug, - 'BIN_PATH': os.path.join(self.project.doc_path, 'conda', self.version.slug, 'bin') + 'BIN_PATH': os.path.join( + self.project.doc_path, + 'conda', + self.version.slug, + 'bin' + ), }) else: env.update({ - 'BIN_PATH': os.path.join(self.project.doc_path, 'envs', self.version.slug, 'bin') + 'BIN_PATH': os.path.join( + self.project.doc_path, + 'envs', + self.version.slug, + 'bin' + ), }) return env From 359c3ffe8548c3af6cc366adfb18713ad4b1b1b2 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 13:39:00 -0500 Subject: [PATCH 12/28] Add doctype to v1 & move models --- readthedocs/config/config.py | 18 +++++------------- readthedocs/config/models.py | 15 +++++++++++++++ readthedocs/config/tests/test_config.py | 9 +++++++++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index c37461104f5..93355b9dc7b 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -13,12 +13,12 @@ import six from .find import find_one +from .models import Conda, Mkdocs, Python, Sphinx, Submodules from .parser import ParseError, parse from .validation import ( ValidationError, validate_bool, validate_choice, validate_dict, validate_directory, validate_file, validate_list, validate_string, validate_value_exists) -from .models import Conda, Python __all__ = ( 'ALL', @@ -523,6 +523,10 @@ def build_image(self): """The docker image used by the builders.""" return self._config['build']['image'] + @property + def doctype(self): + return self.defaults['doctype'] + class BuildConfigV2(BuildConfigBase): @@ -862,20 +866,12 @@ def python(self): @property def sphinx(self): - Sphinx = namedtuple( # noqa - 'Sphinx', - ['builder', 'configuration', 'fail_on_warning'], - ) if self._config['sphinx']: return Sphinx(**self._config['sphinx']) return None @property def mkdocs(self): - Mkdocs = namedtuple( # noqa - 'Mkdocs', - ['configuration', 'fail_on_warning'], - ) if self._config['mkdocs']: return Mkdocs(**self._config['mkdocs']) return None @@ -888,10 +884,6 @@ def doctype(self): @property def submodules(self): - Submodules = namedtuple( # noqa - 'Submodules', - ['include', 'exclude', 'recursive'], - ) return Submodules(**self._config['submodules']) diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py index a6494c544c1..1af35014c52 100644 --- a/readthedocs/config/models.py +++ b/readthedocs/config/models.py @@ -17,3 +17,18 @@ ) Conda = namedtuple('Conda', ['environment']) # noqa + +Sphinx = namedtuple( # noqa + 'Sphinx', + ['builder', 'configuration', 'fail_on_warning'], +) + +Mkdocs = namedtuple( # noqa + 'Mkdocs', + ['configuration', 'fail_on_warning'], +) + +Submodules = namedtuple( # noqa + 'Submodules', + ['include', 'exclude', 'recursive'], +) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 3480070b5d4..7640799a14d 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -194,6 +194,15 @@ def test_version(): assert build.version == '1' +def test_doc_type(): + build = get_build_config( + {}, + get_env_config({'doctype': 'sphinx'}) + ) + build.validate() + assert build.doctype == 'sphinx' + + def test_empty_python_section_is_valid(): build = get_build_config({'python': {}}, get_env_config()) build.validate() From 691f13c52bf55993cf250928ad2f74a9c2412ef5 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 13:50:20 -0500 Subject: [PATCH 13/28] Use build model in v1 --- readthedocs/config/config.py | 7 +++---- readthedocs/config/models.py | 3 +++ readthedocs/config/tests/test_config.py | 14 ++++++++++---- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 93355b9dc7b..451a1f405a0 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -13,7 +13,7 @@ import six from .find import find_one -from .models import Conda, Mkdocs, Python, Sphinx, Submodules +from .models import Build, Conda, Mkdocs, Python, Sphinx, Submodules from .parser import ParseError, parse from .validation import ( ValidationError, validate_bool, validate_choice, validate_dict, @@ -519,9 +519,9 @@ def conda(self): return None @property - def build_image(self): + def build(self): """The docker image used by the builders.""" - return self._config['build']['image'] + return Build(**self._config['build']) @property def doctype(self): @@ -857,7 +857,6 @@ def conda(self): @property def build(self): - Build = namedtuple('Build', ['image']) # noqa return Build(**self._config['build']) @property diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py index 1af35014c52..bf12ddfa6d4 100644 --- a/readthedocs/config/models.py +++ b/readthedocs/config/models.py @@ -4,6 +4,9 @@ from collections import namedtuple + +Build = namedtuple('Build', ['image']) # noqa + Python = namedtuple( # noqa 'Python', [ diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 7640799a14d..f9aa0e2d822 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -197,7 +197,13 @@ def test_version(): def test_doc_type(): build = get_build_config( {}, - get_env_config({'doctype': 'sphinx'}) + get_env_config( + { + 'defaults': { + 'doctype': 'sphinx' + } + } + ) ) build.validate() assert build.doctype == 'sphinx' @@ -617,7 +623,7 @@ def it_works(tmpdir): source_position=0, ) build.validate() - assert build.build_image == 'readthedocs/build:latest' + assert build.build.image == 'readthedocs/build:latest' def default(tmpdir): apply_fs(tmpdir, minimal_config) @@ -628,7 +634,7 @@ def default(tmpdir): source_position=0, ) build.validate() - assert build.build_image == 'readthedocs/build:2.0' + assert build.build.image == 'readthedocs/build:2.0' @pytest.mark.parametrize( 'image', ['latest', 'readthedocs/build:3.0', 'rtd/build:latest']) @@ -644,7 +650,7 @@ def it_priorities_image_from_env_config(tmpdir, image): source_position=0, ) build.validate() - assert build.build_image == image + assert build.build.image == image def test_use_conda_default_false(): From 8361603ef9179182bb17a5c499c9b61913b53b7d Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 13:53:51 -0500 Subject: [PATCH 14/28] Use new key for build.image --- readthedocs/doc_builder/environments.py | 4 ++-- readthedocs/doc_builder/python_environments.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index eb5002a5bd5..148da228163 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -634,8 +634,8 @@ def __init__(self, *args, **kwargs): project_name=self.project.slug, )[:DOCKER_HOSTNAME_MAX_LEN] ) - if self.config and self.config.build_image: - self.container_image = self.config.build_image + if self.config and self.config.build.image: + self.container_image = self.config.build.image if self.project.container_image: self.container_image = self.project.container_image if self.project.container_mem_limit: diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 960f0e24387..8b572da3962 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -151,7 +151,7 @@ def is_obsolete(self): env_build_hash = env_build.get('hash', None) if isinstance(self.build_env, DockerBuildEnvironment): - build_image = self.config.build_image or DOCKER_IMAGE + build_image = self.config.build.image or DOCKER_IMAGE image_hash = self.build_env.image_hash else: # e.g. LocalBuildEnvironment @@ -177,7 +177,7 @@ def save_environment_json(self): } if isinstance(self.build_env, DockerBuildEnvironment): - build_image = self.config.build_image or DOCKER_IMAGE + build_image = self.config.build.image or DOCKER_IMAGE data.update({ 'build': { 'image': build_image, From 7e72b91ea63cf3575353ecea5d6f35f92278d17b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 13:56:54 -0500 Subject: [PATCH 15/28] Stop skipping tests --- readthedocs/rtd_tests/tests/test_config_integration.py | 1 - 1 file changed, 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 71818bf3d35..f1292c36a45 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -232,7 +232,6 @@ def test_requirements_file(self, load_config): self.assertEqual(config.python.requirements, '__init__.py') -@pytest.mark.skip('No implemented yet') @pytest.mark.django_db @mock.patch('readthedocs.projects.models.Project.checkout_path') class TestLoadConfigV2(object): From 30806326c51c90c229a1ff51ec09602388483017 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 14:19:55 -0500 Subject: [PATCH 16/28] Test empty requirements --- .../tests/test_config_integration.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index f1292c36a45..1f8d79dc72c 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -421,6 +421,30 @@ def test_python_requirements(self, run, checkout_path, tmpdir): assert config.python.requirements == requirements_file assert requirements_file in args + @patch('readthedocs.doc_builder.environments.BuildEnvironment.run') + def test_python_requirements_empty(self, run, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file( + tmpdir, + { + 'python': {'requirements': ''} + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + update_docs.python_env.install_user_requirements() + + assert config.python.requirements == '' + assert not run.called + @patch('readthedocs.doc_builder.environments.BuildEnvironment.run') def test_python_install_setup(self, run, checkout_path, tmpdir): checkout_path.return_value = str(tmpdir) From 4b301b8f156ed032ed31f300173da36e085745dc Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 14:24:23 -0500 Subject: [PATCH 17/28] Linter --- readthedocs/config/config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 451a1f405a0..0607fd3eea1 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -7,7 +7,6 @@ import os import re -from collections import namedtuple from contextlib import contextmanager import six From c4661d78c5c35d63e5952375037c3849583ce663 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 14:25:21 -0500 Subject: [PATCH 18/28] Use new settings --- readthedocs/doc_builder/python_environments.py | 4 ++-- readthedocs/projects/tasks.py | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 8b572da3962..7ee1e2f676c 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -272,7 +272,7 @@ def install_core_requirements(self): def install_user_requirements(self): requirements_file_path = self.config.python.requirements - if not requirements_file_path: + if not requirements_file_path and requirements_file_path != '': builder_class = get_builder_class(self.project.documentation_type) docs_dir = (builder_class(build_env=self.build_env, python_env=self) .docs_dir()) @@ -332,7 +332,7 @@ def setup_base(self): '--name', self.version.slug, '--file', - self.config.conda_file, + self.config.conda.environment, bin_path=None, # Don't use conda bin that doesn't exist yet ) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 2bdf1668113..2018137c264 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -456,8 +456,14 @@ def run_build(self, docker, record): env_cls = DockerBuildEnvironment else: env_cls = LocalBuildEnvironment - self.build_env = env_cls(project=self.project, version=self.version, config=self.config, - build=self.build, record=record, environment=env_vars) + self.build_env = env_cls( + project=self.project, + version=self.version, + config=self.config, + build=self.build, + record=record, + environment=env_vars + ) # Environment used for building code, usually with Docker with self.build_env: @@ -469,9 +475,11 @@ def run_build(self, docker, record): if self.config.conda: self._log('Using conda') python_env_cls = Conda - self.python_env = python_env_cls(version=self.version, - build_env=self.build_env, - config=self.config) + self.python_env = python_env_cls( + version=self.version, + build_env=self.build_env, + config=self.config + ) try: self.setup_python_environment() From ec1e0775e34ca984e324c0d1e8b76678c1867f8e Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 16:02:48 -0500 Subject: [PATCH 19/28] Add tests for sphinx --- .../tests/test_config_integration.py | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 1f8d79dc72c..671065b0de9 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -572,3 +572,109 @@ def test_system_packages(self, run, checkout_path, tmpdir): assert '--system-site-packages' in args assert config.python.use_system_site_packages + + @pytest.mark.parametrize('value,result', + [('html', 'sphinx'), + ('htmldir', 'sphinx_htmldir'), + ('singlehtml', 'sphinx_singlehtml')]) + @patch('readthedocs.projects.tasks.get_builder_class') + def test_sphinx_builder( + self, get_builder_class, checkout_path, value, result, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file(tmpdir, {'sphinx': {'builder': value}}) + + self.project.documentation_type = 'mkdocs' + self.project.save() + + update_docs = self.get_update_docs_task() + update_docs.build_docs_html() + + get_builder_class.assert_called_with(result) + + @patch('readthedocs.projects.tasks.get_builder_class') + def test_sphinx_builder_default( + self, get_builder_class, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file(tmpdir, {}) + + self.project.documentation_type = 'mkdocs' + self.project.save() + + update_docs = self.get_update_docs_task() + update_docs.build_docs_html() + + get_builder_class.assert_called_with('sphinx') + + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.move') + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.append_conf') + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.run') + def test_sphinx_configuration( + self, run, append_conf, move, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + apply_fs(tmpdir, { + 'conf.py': '', + 'docx': { + 'conf.py': '', + }, + }) + self.create_config_file( + tmpdir, + { + 'sphinx': { + 'configuration': 'docx/conf.py', + }, + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + + update_docs.build_docs_html() + + args, kwargs = run.call_args + assert kwargs['cwd'] == path.join(str(tmpdir), 'docx') + append_conf.assert_called_once() + move.assert_called_once() + + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.move') + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.append_conf') + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.run') + def test_sphinx_fail_on_warning( + self, run, append_conf, move, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + apply_fs(tmpdir, { + 'docx': { + 'conf.py': '', + }, + }) + self.create_config_file( + tmpdir, + { + 'sphinx': { + 'configuration': 'docx/conf.py', + 'fail_on_warning': True, + }, + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + + update_docs.build_docs_html() + + args, kwargs = run.call_args + assert '-W' in args + append_conf.assert_called_once() + move.assert_called_once() From 32c9aeb627bda8d219dc39d17e944fc7d67c0276 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 16:13:26 -0500 Subject: [PATCH 20/28] Add tests for mkdocs --- .../tests/test_config_integration.py | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 671065b0de9..a554a8304c0 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -678,3 +678,78 @@ def test_sphinx_fail_on_warning( assert '-W' in args append_conf.assert_called_once() move.assert_called_once() + + @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.move') + @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.append_conf') + @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.run') + def test_mkdocs_configuration( + self, run, append_conf, move, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + apply_fs(tmpdir, { + 'mkdocs.yml': '', + 'docx': { + 'mkdocs.yml': '', + }, + }) + self.create_config_file( + tmpdir, + { + 'mkdocs': { + 'configuration': 'docx/mkdocs.yml', + }, + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + + update_docs.build_docs_html() + + args, kwargs = run.call_args + assert '--config-file' in args + assert path.join(str(tmpdir), 'docx/mkdocs.yml') in args + append_conf.assert_called_once() + move.assert_called_once() + + @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.move') + @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.append_conf') + @patch('readthedocs.doc_builder.backends.mkdocs.BaseMkdocs.run') + def test_mkdocs_fail_on_warning( + self, run, append_conf, move, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + apply_fs(tmpdir, { + 'docx': { + 'mkdocs.yml': '', + }, + }) + self.create_config_file( + tmpdir, + { + 'mkdocs': { + 'configuration': 'docx/mkdocs.yml', + 'fail_on_warning': True, + }, + } + ) + + update_docs = self.get_update_docs_task() + config = update_docs.config + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + + update_docs.build_docs_html() + + args, kwargs = run.call_args + assert '--strict' in args + append_conf.assert_called_once() + move.assert_called_once() From 5b37760eccbf1a2e0327c706f42fba584e680209 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 1 Aug 2018 16:51:10 -0500 Subject: [PATCH 21/28] Pass config object to builders --- readthedocs/doc_builder/base.py | 3 +- .../doc_builder/python_environments.py | 8 +++- readthedocs/projects/tasks.py | 9 ++++- readthedocs/rtd_tests/tests/test_builds.py | 2 - .../rtd_tests/tests/test_doc_builder.py | 37 +++++++++++++++---- 5 files changed, 44 insertions(+), 15 deletions(-) diff --git a/readthedocs/doc_builder/base.py b/readthedocs/doc_builder/base.py index fcb70964709..05f09275ac8 100644 --- a/readthedocs/doc_builder/base.py +++ b/readthedocs/doc_builder/base.py @@ -41,11 +41,12 @@ class BaseBuilder(object): # old_artifact_path = .. - def __init__(self, build_env, python_env, force=False): + def __init__(self, build_env, python_env, config, force=False): self.build_env = build_env self.python_env = python_env self.version = build_env.version self.project = build_env.project + self.config = config self._force = force self.target = self.project.artifact_path( version=self.version.slug, type_=self.type) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index 7ee1e2f676c..c17b2902b68 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -274,8 +274,12 @@ def install_user_requirements(self): requirements_file_path = self.config.python.requirements if not requirements_file_path and requirements_file_path != '': builder_class = get_builder_class(self.project.documentation_type) - docs_dir = (builder_class(build_env=self.build_env, python_env=self) - .docs_dir()) + builder = builder_class( + build_env=self.build_env, + python_env=self, + config=self.config + ) + docs_dir = builder.docs_dir() paths = [docs_dir, ''] req_files = ['pip_requirements.txt', 'requirements.txt'] for path, req_file in itertools.product(paths, req_files): diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 2018137c264..04d8cc272da 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -702,9 +702,10 @@ def build_docs(self): def build_docs_html(self): """Build HTML docs.""" - html_builder = get_builder_class(self.project.documentation_type)( + html_builder = get_builder_class(self.config.doctype)( build_env=self.build_env, python_env=self.python_env, + config=self.config, ) if self.build_force: html_builder.force() @@ -775,7 +776,11 @@ def build_docs_class(self, builder_class): only raise a warning exception here. A hard error will halt the build process. """ - builder = get_builder_class(builder_class)(self.build_env, python_env=self.python_env) + builder = get_builder_class(builder_class)( + self.build_env, + python_env=self.python_env, + config=self.config, + ) success = builder.build() builder.move() return success diff --git a/readthedocs/rtd_tests/tests/test_builds.py b/readthedocs/rtd_tests/tests/test_builds.py index 31b4d348fd7..0cff27772a6 100644 --- a/readthedocs/rtd_tests/tests/test_builds.py +++ b/readthedocs/rtd_tests/tests/test_builds.py @@ -1,7 +1,6 @@ from __future__ import absolute_import import mock -import six from django.test import TestCase from django_dynamic_fixture import get @@ -11,7 +10,6 @@ from readthedocs.doc_builder.config import load_yaml_config from readthedocs.doc_builder.environments import LocalBuildEnvironment from readthedocs.doc_builder.python_environments import Virtualenv -from readthedocs.doc_builder.loader import get_builder_class from readthedocs.projects.tasks import UpdateDocsTaskStep from readthedocs.rtd_tests.tests.test_config_integration import create_load diff --git a/readthedocs/rtd_tests/tests/test_doc_builder.py b/readthedocs/rtd_tests/tests/test_doc_builder.py index 06a48b8ea50..2012b82f4e6 100644 --- a/readthedocs/rtd_tests/tests/test_doc_builder.py +++ b/readthedocs/rtd_tests/tests/test_doc_builder.py @@ -6,9 +6,9 @@ import tempfile from collections import namedtuple +import mock import pytest import yaml -import mock from django.test import TestCase from django.test.utils import override_settings from django_dynamic_fixture import get @@ -17,6 +17,7 @@ from readthedocs.builds.models import Version from readthedocs.doc_builder.backends.mkdocs import BaseMkdocs, MkdocsHTML from readthedocs.doc_builder.backends.sphinx import BaseSphinx, SearchBuilder +from readthedocs.doc_builder.config import load_yaml_config from readthedocs.projects.exceptions import ProjectConfigurationError from readthedocs.projects.models import Project @@ -35,7 +36,12 @@ def setUp(self): BaseSphinx.type = 'base' BaseSphinx.sphinx_build_dir = tempfile.mkdtemp() - self.base_sphinx = BaseSphinx(build_env=build_env, python_env=None) + config = load_yaml_config(self.version) + self.base_sphinx = BaseSphinx( + build_env=build_env, + python_env=None, + config=config, + ) @patch( 'readthedocs.doc_builder.backends.sphinx.SPHINX_TEMPLATE_DIR', @@ -89,7 +95,12 @@ def setUp(self): build_env.project = self.project build_env.version = self.version - self.searchbuilder = SearchBuilder(build_env=build_env, python_env=None) + config = load_yaml_config(self.version) + self.searchbuilder = SearchBuilder( + build_env=build_env, + python_env=None, + config=config, + ) def test_ignore_patterns(self): src = tempfile.mkdtemp() @@ -134,9 +145,11 @@ def test_append_conf_create_yaml(self, checkout_path, run): os.mkdir(os.path.join(tmpdir, 'docs')) checkout_path.return_value = tmpdir + config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None + python_env=None, + config=config, ) self.searchbuilder.append_conf() @@ -189,9 +202,11 @@ def test_append_conf_existing_yaml_on_root(self, checkout_path, run): ) checkout_path.return_value = tmpdir + config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None + python_env=None, + config=config, ) self.searchbuilder.append_conf() @@ -243,9 +258,11 @@ def test_override_theme_new_style(self, checkout_path, run): ) checkout_path.return_value = tmpdir + config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None + python_env=None, + config=config, ) self.searchbuilder.append_conf() @@ -276,9 +293,11 @@ def test_override_theme_old_style(self, checkout_path, run): ) checkout_path.return_value = tmpdir + config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None + python_env=None, + config=config, ) self.searchbuilder.append_conf() @@ -307,9 +326,11 @@ def test_dont_override_theme(self, checkout_path, run): ) checkout_path.return_value = tmpdir + config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None + python_env=None, + config=config, ) self.searchbuilder.append_conf() From f1c99dc85d7b57c2a4e4ea57012585f8270e7ba4 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 2 Aug 2018 12:24:31 -0500 Subject: [PATCH 22/28] Revert "Pass config object to builders" This reverts commit 5b37760eccbf1a2e0327c706f42fba584e680209. --- readthedocs/doc_builder/base.py | 3 +- .../doc_builder/python_environments.py | 8 +--- readthedocs/projects/tasks.py | 9 +---- readthedocs/rtd_tests/tests/test_builds.py | 2 + .../rtd_tests/tests/test_doc_builder.py | 37 ++++--------------- 5 files changed, 15 insertions(+), 44 deletions(-) diff --git a/readthedocs/doc_builder/base.py b/readthedocs/doc_builder/base.py index 05f09275ac8..fcb70964709 100644 --- a/readthedocs/doc_builder/base.py +++ b/readthedocs/doc_builder/base.py @@ -41,12 +41,11 @@ class BaseBuilder(object): # old_artifact_path = .. - def __init__(self, build_env, python_env, config, force=False): + def __init__(self, build_env, python_env, force=False): self.build_env = build_env self.python_env = python_env self.version = build_env.version self.project = build_env.project - self.config = config self._force = force self.target = self.project.artifact_path( version=self.version.slug, type_=self.type) diff --git a/readthedocs/doc_builder/python_environments.py b/readthedocs/doc_builder/python_environments.py index c17b2902b68..7ee1e2f676c 100644 --- a/readthedocs/doc_builder/python_environments.py +++ b/readthedocs/doc_builder/python_environments.py @@ -274,12 +274,8 @@ def install_user_requirements(self): requirements_file_path = self.config.python.requirements if not requirements_file_path and requirements_file_path != '': builder_class = get_builder_class(self.project.documentation_type) - builder = builder_class( - build_env=self.build_env, - python_env=self, - config=self.config - ) - docs_dir = builder.docs_dir() + docs_dir = (builder_class(build_env=self.build_env, python_env=self) + .docs_dir()) paths = [docs_dir, ''] req_files = ['pip_requirements.txt', 'requirements.txt'] for path, req_file in itertools.product(paths, req_files): diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 04d8cc272da..2018137c264 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -702,10 +702,9 @@ def build_docs(self): def build_docs_html(self): """Build HTML docs.""" - html_builder = get_builder_class(self.config.doctype)( + html_builder = get_builder_class(self.project.documentation_type)( build_env=self.build_env, python_env=self.python_env, - config=self.config, ) if self.build_force: html_builder.force() @@ -776,11 +775,7 @@ def build_docs_class(self, builder_class): only raise a warning exception here. A hard error will halt the build process. """ - builder = get_builder_class(builder_class)( - self.build_env, - python_env=self.python_env, - config=self.config, - ) + builder = get_builder_class(builder_class)(self.build_env, python_env=self.python_env) success = builder.build() builder.move() return success diff --git a/readthedocs/rtd_tests/tests/test_builds.py b/readthedocs/rtd_tests/tests/test_builds.py index 0cff27772a6..31b4d348fd7 100644 --- a/readthedocs/rtd_tests/tests/test_builds.py +++ b/readthedocs/rtd_tests/tests/test_builds.py @@ -1,6 +1,7 @@ from __future__ import absolute_import import mock +import six from django.test import TestCase from django_dynamic_fixture import get @@ -10,6 +11,7 @@ from readthedocs.doc_builder.config import load_yaml_config from readthedocs.doc_builder.environments import LocalBuildEnvironment from readthedocs.doc_builder.python_environments import Virtualenv +from readthedocs.doc_builder.loader import get_builder_class from readthedocs.projects.tasks import UpdateDocsTaskStep from readthedocs.rtd_tests.tests.test_config_integration import create_load diff --git a/readthedocs/rtd_tests/tests/test_doc_builder.py b/readthedocs/rtd_tests/tests/test_doc_builder.py index 2012b82f4e6..06a48b8ea50 100644 --- a/readthedocs/rtd_tests/tests/test_doc_builder.py +++ b/readthedocs/rtd_tests/tests/test_doc_builder.py @@ -6,9 +6,9 @@ import tempfile from collections import namedtuple -import mock import pytest import yaml +import mock from django.test import TestCase from django.test.utils import override_settings from django_dynamic_fixture import get @@ -17,7 +17,6 @@ from readthedocs.builds.models import Version from readthedocs.doc_builder.backends.mkdocs import BaseMkdocs, MkdocsHTML from readthedocs.doc_builder.backends.sphinx import BaseSphinx, SearchBuilder -from readthedocs.doc_builder.config import load_yaml_config from readthedocs.projects.exceptions import ProjectConfigurationError from readthedocs.projects.models import Project @@ -36,12 +35,7 @@ def setUp(self): BaseSphinx.type = 'base' BaseSphinx.sphinx_build_dir = tempfile.mkdtemp() - config = load_yaml_config(self.version) - self.base_sphinx = BaseSphinx( - build_env=build_env, - python_env=None, - config=config, - ) + self.base_sphinx = BaseSphinx(build_env=build_env, python_env=None) @patch( 'readthedocs.doc_builder.backends.sphinx.SPHINX_TEMPLATE_DIR', @@ -95,12 +89,7 @@ def setUp(self): build_env.project = self.project build_env.version = self.version - config = load_yaml_config(self.version) - self.searchbuilder = SearchBuilder( - build_env=build_env, - python_env=None, - config=config, - ) + self.searchbuilder = SearchBuilder(build_env=build_env, python_env=None) def test_ignore_patterns(self): src = tempfile.mkdtemp() @@ -145,11 +134,9 @@ def test_append_conf_create_yaml(self, checkout_path, run): os.mkdir(os.path.join(tmpdir, 'docs')) checkout_path.return_value = tmpdir - config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None, - config=config, + python_env=None ) self.searchbuilder.append_conf() @@ -202,11 +189,9 @@ def test_append_conf_existing_yaml_on_root(self, checkout_path, run): ) checkout_path.return_value = tmpdir - config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None, - config=config, + python_env=None ) self.searchbuilder.append_conf() @@ -258,11 +243,9 @@ def test_override_theme_new_style(self, checkout_path, run): ) checkout_path.return_value = tmpdir - config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None, - config=config, + python_env=None ) self.searchbuilder.append_conf() @@ -293,11 +276,9 @@ def test_override_theme_old_style(self, checkout_path, run): ) checkout_path.return_value = tmpdir - config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None, - config=config, + python_env=None ) self.searchbuilder.append_conf() @@ -326,11 +307,9 @@ def test_dont_override_theme(self, checkout_path, run): ) checkout_path.return_value = tmpdir - config = load_yaml_config(self.version) self.searchbuilder = MkdocsHTML( build_env=self.build_env, - python_env=None, - config=config, + python_env=None ) self.searchbuilder.append_conf() From fb748291ad40f7fa152107144d7a49784a3965d5 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 2 Aug 2018 12:37:24 -0500 Subject: [PATCH 23/28] Skip new tests --- readthedocs/rtd_tests/tests/test_config_integration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index a554a8304c0..0e5bd232983 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -232,6 +232,7 @@ def test_requirements_file(self, load_config): self.assertEqual(config.python.requirements, '__init__.py') +@pytest.mark.skip @pytest.mark.django_db @mock.patch('readthedocs.projects.models.Project.checkout_path') class TestLoadConfigV2(object): From 0141139ca1ed0404f2b98fe3e406ea0c0d5ca063 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 2 Aug 2018 13:02:46 -0500 Subject: [PATCH 24/28] Style --- readthedocs/config/tests/test_config.py | 8 ++++---- readthedocs/projects/tasks.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index f9aa0e2d822..c4cc99ba451 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -200,8 +200,8 @@ def test_doc_type(): get_env_config( { 'defaults': { - 'doctype': 'sphinx' - } + 'doctype': 'sphinx', + }, } ) ) @@ -272,8 +272,8 @@ def it_uses_validate_string(validate_string): { 'python': { 'pip_install': True, - 'extra_requirements': ['tests'] - } + 'extra_requirements': ['tests'], + }, }, get_env_config(), ) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 2018137c264..ab476f8b93c 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -462,7 +462,7 @@ def run_build(self, docker, record): config=self.config, build=self.build, record=record, - environment=env_vars + environment=env_vars, ) # Environment used for building code, usually with Docker @@ -478,7 +478,7 @@ def run_build(self, docker, record): self.python_env = python_env_cls( version=self.version, build_env=self.build_env, - config=self.config + config=self.config, ) try: From 5ae10e84fcb6b9b5d6b044e3e0d92b10127e0b68 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 2 Aug 2018 13:02:57 -0500 Subject: [PATCH 25/28] Fix test --- readthedocs/rtd_tests/tests/test_config_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 0e5bd232983..00afbf24365 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -162,7 +162,7 @@ def test_install_project(self, load_config): load_config.side_effect = create_load() config = load_yaml_config(self.version) self.assertEqual( - config.python.install_with_pip and config.python.install_with_setup, + config.python.install_with_pip or config.python.install_with_setup, False ) From e73480fac10ff672fda83152e805ea427f9b8e48 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Fri, 3 Aug 2018 17:47:08 -0500 Subject: [PATCH 26/28] One more test for v2 --- readthedocs/rtd_tests/tests/test_config_integration.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index 00afbf24365..a1d34ce6f43 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -281,6 +281,13 @@ def test_using_v2(self, checkout_path, tmpdir): update_docs = self.get_update_docs_task() assert update_docs.config.version == '2' + def test_report_using_invalid_version(self, checkout_path, tmpdir): + checkout_path.return_value = str(tmpdir) + self.create_config_file(tmpdir, {'version': 12}) + with pytest.raises(InvalidConfig) as exinfo: + self.get_update_docs_task() + assert exinfo.value.key == 'version' + @pytest.mark.parametrize('config', [{}, {'formats': []}]) @patch('readthedocs.projects.models.Project.repo_nonblockinglock', new=MagicMock()) @patch('readthedocs.doc_builder.backends.sphinx.SearchBuilder.build') From 27759fe913d840dc3c2f88b3ee4182fc57452797 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Fri, 3 Aug 2018 18:30:14 -0500 Subject: [PATCH 27/28] Test conf.py default value --- readthedocs/config/tests/test_config.py | 6 ++-- .../tests/test_config_integration.py | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index c4cc99ba451..7c2755d94a4 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -14,6 +14,7 @@ from readthedocs.config.config import ( CONFIG_NOT_SUPPORTED, NAME_INVALID, NAME_REQUIRED, PYTHON_INVALID, VERSION_INVALID) +from readthedocs.config.models import Conda from readthedocs.config.validation import ( INVALID_BOOL, INVALID_CHOICE, INVALID_LIST, INVALID_PATH, INVALID_STRING) @@ -513,6 +514,7 @@ def test_valid_build_config(): 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 @@ -665,7 +667,7 @@ def test_use_conda_respects_config(): get_env_config(), ) build.validate() - assert build.conda is not None + assert isinstance(build.conda, Conda) def test_validates_conda_file(tmpdir): @@ -676,7 +678,7 @@ def test_validates_conda_file(tmpdir): source_file=str(tmpdir.join('readthedocs.yml')), ) build.validate() - assert build.conda is not None + assert isinstance(build.conda, Conda) assert build.conda.environment == str(tmpdir.join('environment.yml')) diff --git a/readthedocs/rtd_tests/tests/test_config_integration.py b/readthedocs/rtd_tests/tests/test_config_integration.py index a1d34ce6f43..1673cacf2b6 100644 --- a/readthedocs/rtd_tests/tests/test_config_integration.py +++ b/readthedocs/rtd_tests/tests/test_config_integration.py @@ -613,6 +613,35 @@ def test_sphinx_builder_default( get_builder_class.assert_called_with('sphinx') + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.move') + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.append_conf') + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.run') + def test_sphinx_configuration_default( + self, run, append_conf, move, checkout_path, tmpdir): + """Should be default to find a conf.py file.""" + checkout_path.return_value = str(tmpdir) + + apply_fs(tmpdir, {'conf.py': ''}) + self.create_config_file(tmpdir, {}) + self.project.conf_py_file = '' + self.project.save() + + update_docs = self.get_update_docs_task() + config = update_docs.config + python_env = Virtualenv( + version=self.version, + build_env=update_docs.build_env, + config=config + ) + update_docs.python_env = python_env + + update_docs.build_docs_html() + + args, kwargs = run.call_args + assert kwargs['cwd'] == str(tmpdir) + append_conf.assert_called_once() + move.assert_called_once() + @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.move') @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.append_conf') @patch('readthedocs.doc_builder.backends.sphinx.BaseSphinx.run') From 4baf899fa0fc4fe1879c73a970dc6e042ac186c7 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Wed, 15 Aug 2018 14:38:22 -0500 Subject: [PATCH 28/28] Better check for conda --- readthedocs/projects/tasks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index ab476f8b93c..5918e25cf34 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -472,7 +472,7 @@ def run_build(self, docker, record): self.update_documentation_type() python_env_cls = Virtualenv - if self.config.conda: + if self.config.conda is not None: self._log('Using conda') python_env_cls = Conda self.python_env = python_env_cls( @@ -558,7 +558,7 @@ def get_env_vars(self): 'READTHEDOCS_PROJECT': self.project.slug } - if self.config.conda: + if self.config.conda is not None: env.update({ 'CONDA_ENVS_PATH': os.path.join(self.project.doc_path, 'conda'), 'CONDA_DEFAULT_ENV': self.version.slug,