From ba652de6eb37be4cf25f3a70c18e403cba43c409 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 5 Jun 2023 11:37:57 -0500 Subject: [PATCH 1/2] Build: pass environment explicitly With https://github.com/readthedocs/readthedocs.org/pull/10378/ we now need to always pass an environment, we can't just create a default one. --- readthedocs/projects/models.py | 4 +- readthedocs/rtd_tests/tests/test_backend.py | 47 ++++++++++--------- .../rtd_tests/tests/test_backend_svn.py | 4 +- readthedocs/rtd_tests/tests/test_version.py | 2 +- readthedocs/vcs_support/base.py | 10 +--- 5 files changed, 33 insertions(+), 34 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index afc8f7e0fc5..0b622aba89e 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -947,10 +947,8 @@ def has_htmlzip(self, version_slug=LATEST, version_type=None): version_type=version_type ) - # NOTE: if `environment=None` everything fails, because it cannot execute - # any command. def vcs_repo( - self, version=LATEST, environment=None, + self, environment, version=LATEST, verbose_name=None, version_type=None ): """ diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index 5d17600b392..6b75faa3a46 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -12,6 +12,7 @@ from readthedocs.builds.constants import EXTERNAL from readthedocs.builds.models import Version from readthedocs.config import ALL +from readthedocs.doc_builder.environments import LocalBuildEnvironment from readthedocs.projects.exceptions import RepositoryError from readthedocs.projects.models import Feature, Project from readthedocs.rtd_tests.utils import ( @@ -45,6 +46,7 @@ def setUp(self): # These are the default values from v1 self.dummy_conf.submodules.include = ALL self.dummy_conf.submodules.exclude = [] + self.build_environment = LocalBuildEnvironment() def test_git_lsremote(self): repo_path = self.project.repo @@ -68,7 +70,7 @@ def test_git_lsremote(self): create_git_tag(repo_path, 'v02', annotated=True) create_git_tag(repo_path, 'release-ünîø∂é') - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) # create the working dir if it not exists. It's required to ``cwd`` to # execute the command repo.check_working_dir() @@ -91,7 +93,7 @@ def test_git_lsremote_tags_only(self): create_git_tag(repo_path, "v02", annotated=True) create_git_tag(repo_path, "release-ünîø∂é") - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) # create the working dir if it not exists. It's required to ``cwd`` to # execute the command repo.check_working_dir() @@ -123,7 +125,7 @@ def test_git_lsremote_branches_only(self): for branch in branches: create_git_branch(repo_path, branch) - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) # create the working dir if it not exists. It's required to ``cwd`` to # execute the command repo.check_working_dir() @@ -160,7 +162,7 @@ def test_git_branches(self, checkout_path): os.mkdir(local_repo) checkout_path.return_value = local_repo - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.clone() self.assertEqual( @@ -188,7 +190,7 @@ def test_git_branches_unicode(self, checkout_path): os.mkdir(local_repo) checkout_path.return_value = local_repo - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.clone() self.assertEqual( @@ -197,7 +199,7 @@ def test_git_branches_unicode(self, checkout_path): ) def test_git_update_and_checkout(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) code, _, _ = repo.update() self.assertEqual(code, 0) @@ -217,7 +219,8 @@ def test_git_update_with_external_version(self, fetch): ) repo = self.project.vcs_repo( verbose_name=version.verbose_name, - version_type=version.type + version_type=version.type, + environment=self.build_environment, ) repo.update() fetch.assert_called_once() @@ -231,14 +234,15 @@ def test_git_fetch_with_external_version(self): ) repo = self.project.vcs_repo( verbose_name=version.verbose_name, - version_type=version.type + version_type=version.type, + environment=self.build_environment, ) repo.update() code, _, _ = repo.fetch() self.assertEqual(code, 0) def test_git_checkout_invalid_revision(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.update() version = 'invalid-revision' with self.assertRaises(RepositoryError) as e: @@ -253,7 +257,7 @@ def test_git_tags(self): create_git_tag(repo_path, 'v01') create_git_tag(repo_path, 'v02', annotated=True) create_git_tag(repo_path, 'release-ünîø∂é') - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) # We aren't cloning the repo, # so we need to hack the repo path repo.working_dir = repo_path @@ -264,7 +268,7 @@ def test_git_tags(self): ) def test_check_for_submodules(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.update() self.assertFalse(repo.are_submodules_available(self.dummy_conf)) @@ -274,13 +278,13 @@ def test_check_for_submodules(self): self.assertTrue(repo.are_submodules_available(self.dummy_conf)) def test_skip_submodule_checkout(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.update() repo.checkout('submodule') self.assertTrue(repo.are_submodules_available(self.dummy_conf)) def test_use_shallow_clone(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.update() repo.checkout('submodule') self.assertTrue(repo.use_shallow_clone()) @@ -293,14 +297,14 @@ def test_use_shallow_clone(self): self.assertFalse(repo.use_shallow_clone()) def test_check_submodule_urls(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.update() repo.checkout('submodule') valid, _ = repo.validate_submodules(self.dummy_conf) self.assertTrue(valid) def test_check_invalid_submodule_urls(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.update() repo.checkout('invalidsubmodule') with self.assertRaises(RepositoryError) as e: @@ -313,7 +317,7 @@ def test_check_invalid_submodule_urls(self): ) def test_invalid_submodule_is_ignored(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.update() repo.checkout('submodule') gitmodules_path = os.path.join(repo.working_dir, '.gitmodules') @@ -341,7 +345,7 @@ def test_fetch_clean_tags_and_branches(self, checkout_path): os.mkdir(local_repo) checkout_path.return_value = local_repo - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.clone() delete_git_tag(upstream_repo, 'v02') @@ -391,6 +395,7 @@ def setUp(self): repo=hg_repo, ) self.project.users.add(self.eric) + self.build_environment = LocalBuildEnvironment() def test_parse_branches(self): data = """\ @@ -400,11 +405,11 @@ def test_parse_branches(self): expected_ids = ['stable', 'default'] given_ids = [x.identifier for x in - self.project.vcs_repo().parse_branches(data)] + self.project.vcs_repo(environment=self.build_environment).parse_branches(data)] self.assertEqual(expected_ids, given_ids) def test_update_and_checkout(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.make_clean_working_dir() code, _, _ = repo.update() self.assertEqual(code, 0) @@ -413,7 +418,7 @@ def test_update_and_checkout(self): self.assertTrue(exists(repo.working_dir)) def test_checkout_invalid_revision(self): - repo = self.project.vcs_repo() + repo = self.project.vcs_repo(environment=self.build_environment) repo.update() version = 'invalid-revision' with self.assertRaises(RepositoryError) as e: @@ -437,5 +442,5 @@ def test_parse_tags(self): ] given_ids = [(x.identifier, x.verbose_name) for x in - self.project.vcs_repo().parse_tags(data)] + self.project.vcs_repo(environment=self.build_environment).parse_tags(data)] self.assertEqual(expected_tags, given_ids) diff --git a/readthedocs/rtd_tests/tests/test_backend_svn.py b/readthedocs/rtd_tests/tests/test_backend_svn.py index 9be4505f3c0..859f89f89eb 100644 --- a/readthedocs/rtd_tests/tests/test_backend_svn.py +++ b/readthedocs/rtd_tests/tests/test_backend_svn.py @@ -4,6 +4,7 @@ from django_dynamic_fixture import get from readthedocs.builds.models import Version +from readthedocs.doc_builder.environments import LocalBuildEnvironment from readthedocs.projects.models import Project from readthedocs.vcs_support.backends.svn import Backend as SvnBackend @@ -13,7 +14,8 @@ class TestSvnBackend(TestCase): def test_get_url(self): project = get(Project) version = get(Version, project=project) - backend_obj = SvnBackend(project, version.slug) + environment = LocalBuildEnvironment() + backend_obj = SvnBackend(project, version.slug, environment=environment) base = 'http://example.com/' tag = 'xyz/' diff --git a/readthedocs/rtd_tests/tests/test_version.py b/readthedocs/rtd_tests/tests/test_version.py index 7656ab106fb..67b27557772 100644 --- a/readthedocs/rtd_tests/tests/test_version.py +++ b/readthedocs/rtd_tests/tests/test_version.py @@ -70,7 +70,7 @@ def test_vcs_url_for_external_version_gitlab(self): self.assertEqual(self.external_version.vcs_url, expected_url) def test_vcs_url_for_latest_version(self): - slug = self.pip.default_branch or self.pip.vcs_repo().fallback_branch + slug = self.pip.default_branch or self.pip.vcs_class().fallback_branch expected_url = f'https://github.com/pypa/pip/tree/{slug}/' self.assertEqual(self.tag_version.vcs_url, expected_url) diff --git a/readthedocs/vcs_support/base.py b/readthedocs/vcs_support/base.py index 32be8b19108..bf328761cf4 100644 --- a/readthedocs/vcs_support/base.py +++ b/readthedocs/vcs_support/base.py @@ -55,7 +55,7 @@ class BaseVCS: # Defining a base API, so we'll have unused args # pylint: disable=unused-argument def __init__( - self, project, version_slug, environment=None, + self, project, version_slug, environment, verbose_name=None, version_type=None, **kwargs ): self.default_branch = project.default_branch @@ -67,13 +67,7 @@ def __init__( self.verbose_name = verbose_name self.version_type = version_type - # TODO: always pass an explicit environment - # This is only used in tests #6546 - # - # TODO: we should not allow ``environment=None`` and always use the - # environment defined by the settings - from readthedocs.doc_builder.environments import LocalBuildEnvironment - self.environment = environment or LocalBuildEnvironment() + self.environment = environment def check_working_dir(self): if not os.path.exists(self.working_dir): From 976933b87842eb6f687efd88d7789fd7996f27fb Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 5 Jun 2023 11:44:12 -0500 Subject: [PATCH 2/2] Black --- readthedocs/projects/models.py | 3 +-- readthedocs/rtd_tests/tests/test_backend.py | 18 +++++++++++++----- readthedocs/vcs_support/base.py | 9 +++++++-- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 0b622aba89e..186c31c49cc 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -948,8 +948,7 @@ def has_htmlzip(self, version_slug=LATEST, version_type=None): ) def vcs_repo( - self, environment, version=LATEST, - verbose_name=None, version_type=None + self, environment, version=LATEST, verbose_name=None, version_type=None ): """ Return a Backend object for this project able to handle VCS commands. diff --git a/readthedocs/rtd_tests/tests/test_backend.py b/readthedocs/rtd_tests/tests/test_backend.py index 6b75faa3a46..c9ba4c7ef6f 100644 --- a/readthedocs/rtd_tests/tests/test_backend.py +++ b/readthedocs/rtd_tests/tests/test_backend.py @@ -403,9 +403,13 @@ def test_parse_branches(self): default """ - expected_ids = ['stable', 'default'] - given_ids = [x.identifier for x in - self.project.vcs_repo(environment=self.build_environment).parse_branches(data)] + expected_ids = ["stable", "default"] + given_ids = [ + x.identifier + for x in self.project.vcs_repo( + environment=self.build_environment + ).parse_branches(data) + ] self.assertEqual(expected_ids, given_ids) def test_update_and_checkout(self): @@ -441,6 +445,10 @@ def test_parse_tags(self): ('2b2155623ee2', '1.7.5'), ] - given_ids = [(x.identifier, x.verbose_name) for x in - self.project.vcs_repo(environment=self.build_environment).parse_tags(data)] + given_ids = [ + (x.identifier, x.verbose_name) + for x in self.project.vcs_repo( + environment=self.build_environment + ).parse_tags(data) + ] self.assertEqual(expected_tags, given_ids) diff --git a/readthedocs/vcs_support/base.py b/readthedocs/vcs_support/base.py index bf328761cf4..475c544be70 100644 --- a/readthedocs/vcs_support/base.py +++ b/readthedocs/vcs_support/base.py @@ -55,8 +55,13 @@ class BaseVCS: # Defining a base API, so we'll have unused args # pylint: disable=unused-argument def __init__( - self, project, version_slug, environment, - verbose_name=None, version_type=None, **kwargs + self, + project, + version_slug, + environment, + verbose_name=None, + version_type=None, + **kwargs ): self.default_branch = project.default_branch self.project = project