Skip to content

Build: pass environment explicitly #10388

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -947,11 +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,
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.
Expand Down
61 changes: 37 additions & 24 deletions readthedocs/rtd_tests/tests/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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)

Expand All @@ -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()
Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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))
Expand All @@ -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())
Expand All @@ -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:
Expand All @@ -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')
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -391,20 +395,25 @@ def setUp(self):
repo=hg_repo,
)
self.project.users.add(self.eric)
self.build_environment = LocalBuildEnvironment()

def test_parse_branches(self):
data = """\
stable
default
"""

expected_ids = ['stable', 'default']
given_ids = [x.identifier for x in
self.project.vcs_repo().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)
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe, since it's test code... readability is acceptable 😄

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)
Expand All @@ -413,7 +422,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:
Expand All @@ -436,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().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)
4 changes: 3 additions & 1 deletion readthedocs/rtd_tests/tests/test_backend_svn.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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/'
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/rtd_tests/tests/test_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
17 changes: 8 additions & 9 deletions readthedocs/vcs_support/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=None,
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
Expand All @@ -67,13 +72,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):
Expand Down