launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29429
[Merge] ~cjwatson/launchpad:factory-proxy-git into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:factory-proxy-git into launchpad:master.
Commit message:
Return proxied objects from makeGitRefs
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/433619
`LaunchpadObjectFactory` issues `UnproxiedFactoryMethodWarning` when its methods return objects not wrapped in a security proxy, since that tends to result in tests that are less accurate simulations of production.
This required tightening up a number of tests. Also, `GitRefMixin.__eq__` and `GitRefMixin.__hash__` need to be more careful to cope with the `commit_sha1` attribute of `GitRefDefault` objects raising `NotFoundError`.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:factory-proxy-git into launchpad:master.
diff --git a/lib/lp/bugs/model/tests/test_bug.py b/lib/lp/bugs/model/tests/test_bug.py
index 75921da..4f708c4 100644
--- a/lib/lp/bugs/model/tests/test_bug.py
+++ b/lib/lp/bugs/model/tests/test_bug.py
@@ -536,10 +536,10 @@ class TestBug(TestCaseWithFactory):
[private_git_ref] = self.factory.makeGitRefs(
owner=private_owner, information_type=InformationType.USERDATA
)
- private_bmp = self.factory.makeBranchMergeProposalForGit(
- source_ref=private_git_ref
- )
with person_logged_in(private_owner):
+ private_bmp = self.factory.makeBranchMergeProposalForGit(
+ source_ref=private_git_ref
+ )
bug.linkMergeProposal(private_bmp, private_bmp.registrant)
public_owner = self.factory.makePerson()
public_git_refs = [self.factory.makeGitRefs()[0] for i in range(4)]
diff --git a/lib/lp/charms/browser/tests/test_charmrecipe.py b/lib/lp/charms/browser/tests/test_charmrecipe.py
index f3ca37e..836fb56 100644
--- a/lib/lp/charms/browser/tests/test_charmrecipe.py
+++ b/lib/lp/charms/browser/tests/test_charmrecipe.py
@@ -212,6 +212,8 @@ class TestCharmRecipeAddView(BaseTestCharmRecipeView):
def test_create_new_recipe_project(self):
project = self.factory.makeProduct(displayname="Test Project")
[git_ref] = self.factory.makeGitRefs()
+ git_ref_shortened_path = git_ref.repository.shortened_path
+ git_ref_path = git_ref.path
source_display = git_ref.display_name
browser = self.getViewBrowser(
project, view_name="+new-charm-recipe", user=self.person
@@ -219,8 +221,8 @@ class TestCharmRecipeAddView(BaseTestCharmRecipeView):
browser.getControl(name="field.name").value = "charm-name"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.shortened_path
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_shortened_path
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
browser.getControl("Create charm recipe").click()
content = find_main_content(browser.contents)
@@ -514,7 +516,6 @@ class TestCharmRecipeAddView(BaseTestCharmRecipeView):
self.assertIn("You must select a risk.", browser.contents)
# Entering only the track is not enough
- view_url = canonical_url(git_ref, view_name="+new-charm-recipe")
browser = self.getNonRedirectingBrowser(url=view_url, user=self.person)
browser.getControl(name="field.project").value = "test-project"
browser.getControl("Automatically upload to store").selected = True
@@ -529,7 +530,6 @@ class TestCharmRecipeAddView(BaseTestCharmRecipeView):
self.assertIn("You must select a risk.", browser.contents)
# Entering only the track and branch will error
- view_url = canonical_url(git_ref, view_name="+new-charm-recipe")
browser = self.getNonRedirectingBrowser(url=view_url, user=self.person)
browser.getControl(name="field.project").value = "test-project"
browser.getControl("Automatically upload to store").selected = True
@@ -615,6 +615,9 @@ class TestCharmRecipeEditView(BaseTestCharmRecipeView):
name="new-team", displayname="New Team", members=[self.person]
)
[new_git_ref] = self.factory.makeGitRefs()
+ new_git_ref_display_name = new_git_ref.display_name
+ new_git_ref_identity = new_git_ref.repository.identity
+ new_git_ref_path = new_git_ref.path
browser = self.getViewBrowser(recipe, user=self.person)
browser.getLink("Edit charm recipe").click()
@@ -622,8 +625,8 @@ class TestCharmRecipeEditView(BaseTestCharmRecipeView):
browser.getControl(name="field.name").value = "new-name"
browser.getControl(
name="field.git_ref.repository"
- ).value = new_git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = new_git_ref.path
+ ).value = new_git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = new_git_ref_path
browser.getControl(
"Automatically build when branch changes"
).selected = True
@@ -636,7 +639,7 @@ class TestCharmRecipeEditView(BaseTestCharmRecipeView):
self.assertEqual("new-name", extract_text(content.h1))
self.assertThat("New Team", MatchesPickerText(content, "edit-owner"))
self.assertThat(
- "Source:\n%s\nEdit charm recipe" % new_git_ref.display_name,
+ "Source:\n%s\nEdit charm recipe" % new_git_ref_display_name,
MatchesTagText(content, "source"),
)
self.assertThat(
diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
index e624054..4ff034a 100644
--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
@@ -1839,6 +1839,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
{"uid": bmp.source_git_repository.owner.id},
)
)
+ login_person(self.user)
self.assertTrue(view.pending_diff)
def test_description_is_meta_description(self):
@@ -1857,10 +1858,11 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
def test_revisionStatusReports_display(self):
bmp = self.factory.makeBranchMergeProposalForGit()
+ owner = bmp.source_git_repository.owner
sha1 = hashlib.sha1(b"0").hexdigest()
commit_date = datetime(2015, 1, 1, tzinfo=pytz.UTC)
report1 = self.factory.makeRevisionStatusReport(
- user=bmp.source_git_repository.owner,
+ user=owner,
git_repository=bmp.source_git_repository,
title="CI",
commit_sha1=sha1,
@@ -1869,7 +1871,7 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
result=RevisionStatusResult.SUCCEEDED,
)
pending_report = self.factory.makeRevisionStatusReport(
- user=bmp.source_git_repository.owner,
+ user=owner,
git_repository=bmp.source_git_repository,
title="Build",
commit_sha1=sha1,
@@ -1893,13 +1895,13 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
browser = self.getUserBrowser(
canonical_url(bmp, rootsite="code"),
- user=bmp.source_git_repository.owner,
+ user=owner,
)
reports_section = find_tags_by_class(
browser.contents, "status-reports-table"
)
- with person_logged_in(bmp.source_git_repository.owner):
+ with person_logged_in(owner):
self.assertThat(
reports_section[0],
Within(
@@ -2164,7 +2166,9 @@ class TestBranchMergeProposalBrowserView(BrowserTestCase):
# information.
hosting_fixture = self.useFixture(GitHostingFixture())
bmp = self.factory.makeBranchMergeProposalForGit()
+ owner = bmp.source_git_repository.owner
self.getMainText(bmp, "+index")
+ login_person(owner)
target_path = bmp.target_git_repository.getInternalPath()
source_path = bmp.source_git_repository.getInternalPath()
self.assertThat(
diff --git a/lib/lp/code/browser/tests/test_gitref.py b/lib/lp/code/browser/tests/test_gitref.py
index a465a96..ffaa0d0 100644
--- a/lib/lp/code/browser/tests/test_gitref.py
+++ b/lib/lp/code/browser/tests/test_gitref.py
@@ -1064,7 +1064,7 @@ class TestGitRefView(BrowserTestCase):
merged_tip = dict(log[-1])
merged_tip["sha1"] = hashlib.sha1(b"merged").hexdigest()
self.scanRef(mp.merge_source, merged_tip)
- mp.markAsMerged(merged_revision_id=log[0]["sha1"])
+ removeSecurityProxy(mp).markAsMerged(merged_revision_id=log[0]["sha1"])
view = create_initialized_view(ref, "+index")
contents = view()
soup = BeautifulSoup(contents)
@@ -1118,7 +1118,7 @@ class TestGitRefView(BrowserTestCase):
merged_tip = dict(log[-1])
merged_tip["sha1"] = hashlib.sha1(b"merged").hexdigest()
self.scanRef(mp.merge_source, merged_tip)
- mp.markAsMerged(merged_revision_id=log[0]["sha1"])
+ removeSecurityProxy(mp).markAsMerged(merged_revision_id=log[0]["sha1"])
mp.source_git_repository.removeRefs([mp.source_git_path])
view = create_initialized_view(ref, "+index")
contents = view()
@@ -1282,7 +1282,7 @@ class TestGitRefView(BrowserTestCase):
view = create_view(ref, "+index")
with StormStatementRecorder() as recorder:
view.landing_candidates
- self.assertThat(recorder, HasQueryCount(Equals(12)))
+ self.assertThat(recorder, HasQueryCount(Equals(13)))
def test_query_count_landing_targets(self):
project = self.factory.makeProduct()
@@ -1299,7 +1299,7 @@ class TestGitRefView(BrowserTestCase):
view = create_view(ref, "+index")
with StormStatementRecorder() as recorder:
view.landing_targets
- self.assertThat(recorder, HasQueryCount(Equals(12)))
+ self.assertThat(recorder, HasQueryCount(Equals(13)))
def test_timeout(self):
# The page renders even if fetching commits times out.
diff --git a/lib/lp/code/browser/widgets/tests/test_gitgrantee.py b/lib/lp/code/browser/widgets/tests/test_gitgrantee.py
index c807d40..452bfa5 100644
--- a/lib/lp/code/browser/widgets/tests/test_gitgrantee.py
+++ b/lib/lp/code/browser/widgets/tests/test_gitgrantee.py
@@ -22,7 +22,7 @@ from lp.services.beautifulsoup import BeautifulSoup
from lp.services.webapp.escaping import html_escape
from lp.services.webapp.publisher import canonical_url
from lp.services.webapp.servers import LaunchpadTestRequest
-from lp.testing import TestCaseWithFactory, verifyObject
+from lp.testing import TestCaseWithFactory, login_person, verifyObject
from lp.testing.layers import DatabaseFunctionalLayer
@@ -32,7 +32,9 @@ class TestGitGranteeWidgetBase:
def setUp(self):
super().setUp()
- [self.ref] = self.factory.makeGitRefs()
+ owner = self.factory.makePerson()
+ login_person(owner)
+ [self.ref] = self.factory.makeGitRefs(owner=owner)
self.rule = self.factory.makeGitRule(
repository=self.ref.repository, ref_pattern=self.ref.path
)
diff --git a/lib/lp/code/model/gitref.py b/lib/lp/code/model/gitref.py
index 8e86dda..6740db6 100644
--- a/lib/lp/code/model/gitref.py
+++ b/lib/lp/code/model/gitref.py
@@ -82,21 +82,37 @@ class GitRefMixin:
repository_url = None
def __eq__(self, other):
- return (
+ if not (
IGitRef.providedBy(other)
and not IGitRefRemote.providedBy(other)
and self.repository == other.repository
and self.repository_url == other.repository_url
and self.path == other.path
- and self.commit_sha1 == other.commit_sha1
- )
+ ):
+ return False
+ try:
+ self_commit_sha1 = self.commit_sha1
+ except NotFoundError:
+ # Possible for GitRefDefault objects.
+ self_commit_sha1 = None
+ try:
+ other_commit_sha1 = other.commit_sha1
+ except NotFoundError:
+ # Possible for GitRefDefault objects.
+ other_commit_sha1 = None
+ return self_commit_sha1 == other_commit_sha1
def __ne__(self, other):
return not self == other
def __hash__(self):
+ try:
+ commit_sha1 = self.commit_sha1
+ except NotFoundError:
+ # Possible for GitRefDefault objects.
+ commit_sha1 = None
return hash(
- (self.repository, self.repository_url, self.path, self.commit_sha1)
+ (self.repository, self.repository_url, self.path, commit_sha1)
)
@property
diff --git a/lib/lp/code/model/tests/test_branchmergeproposal.py b/lib/lp/code/model/tests/test_branchmergeproposal.py
index f71aabe..a404d5e 100644
--- a/lib/lp/code/model/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/model/tests/test_branchmergeproposal.py
@@ -1842,7 +1842,7 @@ class TestBranchMergeProposalBugs(WithVCSScenarios, TestCaseWithFactory):
"message": "Non-existent bug; LP: #%d" % (bugs[2].id + 100),
},
]
- related_bugs = bmp._fetchRelatedBugIDsFromSource()
+ related_bugs = removeSecurityProxy(bmp)._fetchRelatedBugIDsFromSource()
path = "%s:%s" % (
bmp.target_git_repository.getInternalPath(),
bmp.source_git_repository.getInternalPath(),
diff --git a/lib/lp/code/model/tests/test_gitref.py b/lib/lp/code/model/tests/test_gitref.py
index 51d9f42..2afc0ad 100644
--- a/lib/lp/code/model/tests/test_gitref.py
+++ b/lib/lp/code/model/tests/test_gitref.py
@@ -24,6 +24,7 @@ from testtools.matchers import (
MatchesStructure,
)
from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
from lp.app.enums import InformationType
from lp.app.interfaces.informationtype import IInformationType
@@ -153,11 +154,13 @@ class TestGitRef(TestCaseWithFactory):
verifyObject(IPrivacy, ref)
def test_refs_in_private_repositories_are_private(self):
+ owner = self.factory.makePerson()
[ref] = self.factory.makeGitRefs(
- information_type=InformationType.USERDATA
+ owner=owner, information_type=InformationType.USERDATA
)
- self.assertTrue(ref.private)
- self.assertEqual(InformationType.USERDATA, ref.information_type)
+ with person_logged_in(owner):
+ self.assertTrue(ref.private)
+ self.assertEqual(InformationType.USERDATA, ref.information_type)
class TestGitRefGetCommits(TestCaseWithFactory):
@@ -385,7 +388,7 @@ class TestGitRefGetCommits(TestCaseWithFactory):
def test_extended_details_with_merge(self):
mp = self.factory.makeBranchMergeProposalForGit(target_ref=self.ref)
- mp.markAsMerged(merged_revision_id=self.sha1_tip)
+ removeSecurityProxy(mp).markAsMerged(merged_revision_id=self.sha1_tip)
revisions = self.ref.getLatestCommits(
self.sha1_tip, extended_details=True, user=self.ref.owner
)
@@ -707,9 +710,10 @@ class TestGitRefCreateMergeProposal(TestCaseWithFactory):
"""Personal repositories cannot be used as a source for MPs to
project repositories.
"""
- self.source.repository.setTarget(
- target=self.source.owner, user=self.source.owner
- )
+ with person_logged_in(self.source.owner):
+ self.source.repository.setTarget(
+ target=self.source.owner, user=self.source.owner
+ )
self.assertRaises(
InvalidBranchMergeProposal,
self.source.addLandingTarget,
@@ -721,17 +725,19 @@ class TestGitRefCreateMergeProposal(TestCaseWithFactory):
"""A branch in a personal repository can be used as a source for MPs
to another branch in the same personal repository.
"""
- self.target.repository.setTarget(
- target=self.target.owner, user=self.target.owner
- )
+ with person_logged_in(self.target.owner):
+ self.target.repository.setTarget(
+ target=self.target.owner, user=self.target.owner
+ )
[source] = self.factory.makeGitRefs(repository=self.target.repository)
source.addLandingTarget(self.user, self.target)
def test_target_repository_same_target(self):
"""The target repository's target must match that of the source."""
- self.target.repository.setTarget(
- target=self.target.owner, user=self.target.owner
- )
+ with person_logged_in(self.target.owner):
+ self.target.repository.setTarget(
+ target=self.target.owner, user=self.target.owner
+ )
self.assertRaises(
InvalidBranchMergeProposal,
self.source.addLandingTarget,
@@ -740,9 +746,10 @@ class TestGitRefCreateMergeProposal(TestCaseWithFactory):
)
project = self.factory.makeProduct()
- self.target.repository.setTarget(
- target=project, user=self.target.owner
- )
+ with person_logged_in(self.target.owner):
+ self.target.repository.setTarget(
+ target=project, user=self.target.owner
+ )
self.assertRaises(
InvalidBranchMergeProposal,
self.source.addLandingTarget,
@@ -761,9 +768,10 @@ class TestGitRefCreateMergeProposal(TestCaseWithFactory):
def test_prerequisite_repository_same_target(self):
"""The prerequisite repository, if any, must be for the same target."""
- self.prerequisite.repository.setTarget(
- target=self.prerequisite.owner, user=self.prerequisite.owner
- )
+ with person_logged_in(self.prerequisite.owner):
+ self.prerequisite.repository.setTarget(
+ target=self.prerequisite.owner, user=self.prerequisite.owner
+ )
self.assertRaises(
InvalidBranchMergeProposal,
self.source.addLandingTarget,
@@ -773,9 +781,10 @@ class TestGitRefCreateMergeProposal(TestCaseWithFactory):
)
project = self.factory.makeProduct()
- self.prerequisite.repository.setTarget(
- target=project, user=self.prerequisite.owner
- )
+ with person_logged_in(self.prerequisite.owner):
+ self.prerequisite.repository.setTarget(
+ target=project, user=self.prerequisite.owner
+ )
self.assertRaises(
InvalidBranchMergeProposal,
self.source.addLandingTarget,
@@ -826,7 +835,8 @@ class TestGitRefCreateMergeProposal(TestCaseWithFactory):
proposal = self.source.addLandingTarget(
self.user, self.target, self.prerequisite
)
- proposal.rejectBranch(self.user, "some_revision")
+ with person_logged_in(self.user):
+ proposal.rejectBranch(self.user, "some_revision")
self.source.addLandingTarget(self.user, self.target, self.prerequisite)
def test_default_reviewer(self):
@@ -927,8 +937,10 @@ class TestGitRefGrants(TestCaseWithFactory):
repository=repository, ref_pattern="refs/heads/*"
)
self.factory.makeGitRuleGrant(rule=wildcard_rule)
+ with person_logged_in(repository.owner):
+ observed_grants = ref.getGrants()
self.assertThat(
- ref.getGrants(),
+ observed_grants,
MatchesSetwise(
MatchesStructure(
rule=Equals(rule),
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index 670b4d6..0363da5 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -707,7 +707,7 @@ class TestGitRepository(TestCaseWithFactory):
grantee=GitGranteeType.REPOSITORY_OWNER,
)
- results = ref.repository.findRuleGrantsByGrantee(
+ results = repository.findRuleGrantsByGrantee(
GitGranteeType.REPOSITORY_OWNER, ref_pattern=ref.path
)
self.assertEqual([exact_grant], list(results))
@@ -730,11 +730,11 @@ class TestGitRepository(TestCaseWithFactory):
grantee=GitGranteeType.REPOSITORY_OWNER,
)
- results = ref.repository.findRuleGrantsByGrantee(
+ results = repository.findRuleGrantsByGrantee(
GitGranteeType.REPOSITORY_OWNER, include_transitive=False
)
self.assertItemsEqual([exact_grant, wildcard_grant], list(results))
- results = ref.repository.findRuleGrantsByGrantee(
+ results = repository.findRuleGrantsByGrantee(
GitGranteeType.REPOSITORY_OWNER,
ref_pattern=ref.path,
include_transitive=False,
@@ -3879,9 +3879,9 @@ class TestGitRepositoryDetectMerges(TestCaseWithFactory):
self.assertNotEqual(
BranchMergeProposalStatus.MERGED, proposal.queue_status
)
- proposal.target_git_repository._markProposalMerged(
- proposal, proposal.target_git_commit_sha1
- )
+ removeSecurityProxy(
+ proposal.target_git_repository
+ )._markProposalMerged(proposal, proposal.target_git_commit_sha1)
self.assertEqual(
BranchMergeProposalStatus.MERGED, proposal.queue_status
)
diff --git a/lib/lp/code/model/tests/test_sourcepackagerecipe.py b/lib/lp/code/model/tests/test_sourcepackagerecipe.py
index b4352e5..1227ece 100644
--- a/lib/lp/code/model/tests/test_sourcepackagerecipe.py
+++ b/lib/lp/code/model/tests/test_sourcepackagerecipe.py
@@ -125,8 +125,8 @@ class GitMixin:
@staticmethod
def setInformationType(branch, information_type):
removeSecurityProxy(
- branch.repository
- ).information_type = information_type
+ branch
+ ).repository.information_type = information_type
def makeRecipeText(self):
branch = self.makeBranch()
diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
index 96610c3..05ccc58 100644
--- a/lib/lp/code/xmlrpc/tests/test_git.py
+++ b/lib/lp/code/xmlrpc/tests/test_git.py
@@ -2330,6 +2330,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
for build in builds
]
repository = refs[0].repository
+ registrant = repository.registrant
path = "/%s" % repository.unique_name
self.assertUnauthorized(
LAUNCHPAD_SERVICES,
@@ -2370,7 +2371,7 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
macaroon_raw="nonsense",
)
self.assertUnauthorized(
- repository.registrant,
+ registrant,
path,
permission="read",
macaroon_raw=macaroons[0].serialize(),
@@ -3505,10 +3506,11 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
issuer = getUtility(IMacaroonIssuer, "snap-build")
macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
build.updateStatus(BuildStatus.BUILDING)
+ repository = ref.repository
path = ref.path.encode("UTF-8")
self.assertHasRefPermissions(
LAUNCHPAD_SERVICES,
- ref.repository,
+ repository,
[path],
{path: []},
macaroon_raw=macaroon.serialize(),
@@ -3528,10 +3530,11 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
issuer = getUtility(IMacaroonIssuer, "ci-build")
macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
build.updateStatus(BuildStatus.BUILDING)
+ repository = ref.repository
path = ref.path.encode("UTF-8")
self.assertHasRefPermissions(
LAUNCHPAD_SERVICES,
- ref.repository,
+ repository,
[path],
{path: []},
macaroon_raw=macaroon.serialize(),
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index f214eac..56d61f5 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -207,6 +207,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
oci_project = self.factory.makeOCIProject()
oci_project_display = oci_project.display_name
[git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v2.0-20.04"])
+ git_ref_identity = git_ref.repository.identity
+ git_ref_path = git_ref.path
source_display = git_ref.display_name
browser = self.getViewBrowser(
oci_project, view_name="+new-recipe", user=self.person
@@ -215,8 +217,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl("Description").value = "Recipe description"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
browser.getControl("Create OCI recipe").click()
content = find_main_content(browser.contents)
@@ -285,6 +287,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
def test_create_new_recipe_invalid_format(self):
oci_project = self.factory.makeOCIProject()
[git_ref] = self.factory.makeGitRefs(paths=["refs/heads/invalid"])
+ git_ref_identity = git_ref.repository.identity
+ git_ref_path = git_ref.path
browser = self.getViewBrowser(
oci_project, view_name="+new-recipe", user=self.person
)
@@ -292,14 +296,16 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl("Description").value = "Recipe description"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
browser.getControl("Create OCI recipe").click()
self.assertIn("Branch does not match format", browser.contents)
def test_create_new_recipe_with_build_args(self):
oci_project = self.factory.makeOCIProject()
[git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v2.0-20.04"])
+ git_ref_identity = git_ref.repository.identity
+ git_ref_path = git_ref.path
browser = self.getViewBrowser(
oci_project, view_name="+new-recipe", user=self.person
)
@@ -307,8 +313,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl("Description").value = "Recipe description"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
browser.getControl(
"Build-time ARG variables"
).value = "VAR1=10\nVAR2=20"
@@ -327,6 +333,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
with person_logged_in(oci_project.distribution.owner):
oci_project.distribution.oci_registry_credentials = credentials
[git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v2.0-20.04"])
+ git_ref_identity = git_ref.repository.identity
+ git_ref_path = git_ref.path
browser = self.getViewBrowser(
oci_project, view_name="+new-recipe", user=self.person
)
@@ -334,8 +342,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl("Description").value = "Recipe description"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
image_name = self.factory.getUniqueUnicode()
browser.getControl(name="field.image_name").value = image_name
@@ -400,6 +408,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
self.setUpDistroSeries()
oci_project = self.factory.makeOCIProject(pillar=self.distribution)
[git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v2.0-20.04"])
+ git_ref_identity = git_ref.repository.identity
+ git_ref_path = git_ref.path
browser = self.getViewBrowser(
oci_project, view_name="+new-recipe", user=self.person
)
@@ -408,8 +418,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl(name="field.name").value = "recipe-name"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
browser.getControl("Create OCI recipe").click()
login_person(self.person)
recipe = getUtility(IOCIRecipeSet).getByName(
@@ -499,6 +509,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
)
oci_project = self.factory.makeOCIProject(pillar=distribution)
[git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v2.0-20.04"])
+ git_ref_identity = git_ref.repository.identity
+ git_ref_path = git_ref.path
browser = self.getViewBrowser(
oci_project, view_name="+new-recipe", user=self.person
)
@@ -506,8 +518,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl("Description").value = "Recipe description"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
official_control = browser.getControl("Official recipe")
official_control.selected = True
browser.getControl("Create OCI recipe").click()
@@ -525,10 +537,14 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
# do it once
oci_project = self.factory.makeOCIProject(pillar=distribution)
[git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v2.0-20.04"])
+ git_ref_identity = git_ref.repository.identity
+ git_ref_path = git_ref.path
# and then do it again
oci_project2 = self.factory.makeOCIProject(pillar=distribution)
[git_ref2] = self.factory.makeGitRefs(paths=["refs/heads/v3.0-20.04"])
+ git_ref2_identity = git_ref2.repository.identity
+ git_ref2_path = git_ref2.path
browser = self.getViewBrowser(
oci_project, view_name="+new-recipe", user=self.person
)
@@ -536,8 +552,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl("Description").value = "Recipe description"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
official_control = browser.getControl("Official recipe")
official_control.selected = True
browser.getControl("Create OCI recipe").click()
@@ -554,8 +570,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser2.getControl("Description").value = "Recipe description"
browser2.getControl(
name="field.git_ref.repository"
- ).value = git_ref2.repository.identity
- browser2.getControl(name="field.git_ref.path").value = git_ref2.path
+ ).value = git_ref2_identity
+ browser2.getControl(name="field.git_ref.path").value = git_ref2_path
official_control = browser2.getControl("Official recipe")
official_control.selected = True
browser2.getControl("Create OCI recipe").click()
@@ -578,6 +594,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
)
oci_project = self.factory.makeOCIProject(pillar=distribution)
[git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v2.0-20.04"])
+ git_ref_identity = git_ref.repository.identity
+ git_ref_path = git_ref.path
browser = self.getViewBrowser(
oci_project, view_name="+new-recipe", user=self.person
)
@@ -585,8 +603,8 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl("Description").value = "Recipe description"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
official_control = browser.getControl("Official recipe")
official_control.selected = True
browser.getControl("Create OCI recipe").click()
@@ -600,6 +618,7 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
def test_create_recipe_doesnt_override_gitref_errors(self):
oci_project = self.factory.makeOCIProject()
[git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v2.0-20.04"])
+ git_ref_identity = git_ref.repository.identity
browser = self.getViewBrowser(
oci_project, view_name="+new-recipe", user=self.person
)
@@ -607,7 +626,7 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl("Description").value = "Recipe description"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.identity
+ ).value = git_ref_identity
browser.getControl(name="field.git_ref.path").value = "non-exist"
browser.getControl("Create OCI recipe").click()
@@ -760,6 +779,9 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
[new_git_ref] = self.factory.makeGitRefs(
paths=["refs/heads/v2.0-20.04"]
)
+ new_git_ref_display_name = new_git_ref.display_name
+ new_git_ref_identity = new_git_ref.repository.identity
+ new_git_ref_path = new_git_ref.path
self.factory.makeOCIPushRule(recipe=recipe)
browser = self.getViewBrowser(recipe, user=self.person)
@@ -769,8 +791,8 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser.getControl("Description").value = "New description"
browser.getControl(
name="field.git_ref.repository"
- ).value = new_git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = new_git_ref.path
+ ).value = new_git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = new_git_ref_path
browser.getControl("Build file path").value = "Dockerfile-2"
browser.getControl("Build directory context").value = "apath"
browser.getControl("Build daily").selected = True
@@ -784,7 +806,7 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
MatchesTagText(content, "oci-project"),
)
self.assertThat(
- "Source:\n%s\nEdit OCI recipe" % new_git_ref.display_name,
+ "Source:\n%s\nEdit OCI recipe" % new_git_ref_display_name,
MatchesTagText(content, "source"),
)
self.assertThat(
@@ -821,7 +843,9 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
browser = self.getViewBrowser(recipe, user=self.person)
browser.getLink("Edit OCI recipe").click()
- browser.getControl(name="field.git_ref.path").value = new_git_ref.path
+ browser.getControl(
+ name="field.git_ref.path"
+ ).value = "refs/heads/invalid"
browser.getControl("Update OCI recipe").click()
self.assertIn("Branch does not match format", browser.contents)
diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
index c9eb67c..6bde0c8 100644
--- a/lib/lp/snappy/browser/tests/test_snap.py
+++ b/lib/lp/snappy/browser/tests/test_snap.py
@@ -349,6 +349,8 @@ class TestSnapAddView(BaseTestSnapView):
self.useFixture(GitHostingFixture(blob=b""))
project = self.factory.makeProduct()
[git_ref] = self.factory.makeGitRefs()
+ git_ref_shortened_path = git_ref.repository.shortened_path
+ git_ref_path = git_ref.path
source_display = git_ref.display_name
browser = self.getViewBrowser(
project, view_name="+new-snap", user=self.person
@@ -357,8 +359,8 @@ class TestSnapAddView(BaseTestSnapView):
browser.getControl(name="field.vcs").value = "GIT"
browser.getControl(
name="field.git_ref.repository"
- ).value = git_ref.repository.shortened_path
- browser.getControl(name="field.git_ref.path").value = git_ref.path
+ ).value = git_ref_shortened_path
+ browser.getControl(name="field.git_ref.path").value = git_ref_path
browser.getControl("Create snap package").click()
content = find_main_content(browser.contents)
@@ -978,6 +980,9 @@ class TestSnapEditView(BaseTestSnapView):
usable_distro_series=[new_series]
)
[new_git_ref] = self.factory.makeGitRefs()
+ new_git_ref_display_name = new_git_ref.display_name
+ new_git_ref_identity = new_git_ref.repository.identity
+ new_git_ref_path = new_git_ref.path
archive = self.factory.makeArchive()
browser = self.getViewBrowser(snap, user=self.person)
@@ -990,8 +995,8 @@ class TestSnapEditView(BaseTestSnapView):
browser.getControl("Git", index=0).click()
browser.getControl(
name="field.git_ref.repository"
- ).value = new_git_ref.repository.identity
- browser.getControl(name="field.git_ref.path").value = new_git_ref.path
+ ).value = new_git_ref_identity
+ browser.getControl(name="field.git_ref.path").value = new_git_ref_path
browser.getControl("Build source tarball").selected = True
browser.getControl(
"Automatically build when branch changes"
@@ -1015,7 +1020,7 @@ class TestSnapEditView(BaseTestSnapView):
MatchesTagText(content, "distro_series"),
)
self.assertThat(
- "Source:\n%s\nEdit snap package" % new_git_ref.display_name,
+ "Source:\n%s\nEdit snap package" % new_git_ref_display_name,
MatchesTagText(content, "source"),
)
self.assertThat(
@@ -2090,7 +2095,8 @@ class TestSnapView(BaseTestSnapView):
paths=["refs/heads/master"],
information_type=InformationType.PRIVATESECURITY,
)
- snap = self.makeSnap(git_ref=ref, private=True)
+ with person_logged_in(self.person):
+ snap = self.makeSnap(git_ref=ref, private=True)
with admin_logged_in():
self.makeBuild(
snap=snap,
@@ -2132,7 +2138,8 @@ class TestSnapView(BaseTestSnapView):
paths=["refs/heads/master"],
information_type=InformationType.PRIVATESECURITY,
)
- snap = self.makeSnap(git_ref=ref, private=True)
+ with person_logged_in(self.person):
+ snap = self.makeSnap(git_ref=ref, private=True)
with admin_logged_in():
archive = self.factory.makeArchive(private=True)
self.makeBuild(
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index 48b1078..93fe69d 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -3902,14 +3902,14 @@ class TestSnapWebservice(TestCaseWithFactory):
def test_set_git_path(self):
# Setting git_path on a Git-based Snap works.
- ref_master, ref_next = self.factory.makeGitRefs(
+ ref_master, _ = self.factory.makeGitRefs(
paths=["refs/heads/master", "refs/heads/next"]
)
snap = self.makeSnap(git_ref=ref_master)
response = self.webservice.patch(
snap["self_link"],
"application/json",
- json.dumps({"git_path": ref_next.path}),
+ json.dumps({"git_path": "refs/heads/next"}),
)
self.assertEqual(209, response.status)
self.assertThat(
@@ -3917,7 +3917,7 @@ class TestSnapWebservice(TestCaseWithFactory):
ContainsDict(
{
"git_repository_link": Equals(snap["git_repository_link"]),
- "git_path": Equals(ref_next.path),
+ "git_path": Equals("refs/heads/next"),
}
),
)
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 33191fb..794ab5e 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2120,7 +2120,7 @@ class LaunchpadObjectFactory(ObjectFactory):
}
with GitHostingFixture():
refs_by_path = {
- ref.path: ref
+ ref.path: ProxyFactory(ref)
for ref in removeSecurityProxy(repository).createOrUpdateRefs(
refs_info, get_objects=True
)
