launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28243
[Merge] ~cjwatson/launchpad:revision-status-reports-without-result into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:revision-status-reports-without-result into launchpad:master.
Commit message:
Fix display of RevisionStatusReports with result=None
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/417343
A revision status report whose build job hasn't started yet may have `result=None`. Fix display of such reports not to crash, and improve icon selection for revision status reports more generally in the process.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:revision-status-reports-without-result into launchpad:master.
diff --git a/lib/lp/app/browser/configure.zcml b/lib/lp/app/browser/configure.zcml
index 753ce04..9419f7f 100644
--- a/lib/lp/app/browser/configure.zcml
+++ b/lib/lp/app/browser/configure.zcml
@@ -610,6 +610,13 @@
name="image"
/>
+ <adapter
+ for="lp.code.interfaces.revisionstatus.IRevisionStatusReport"
+ provides="zope.traversing.interfaces.IPathAdapter"
+ factory="lp.app.browser.tales.RevisionStatusReportImageDisplayAPI"
+ name="image"
+ />
+
<!-- TALES badges: namespace -->
<adapter
diff --git a/lib/lp/app/browser/tales.py b/lib/lp/app/browser/tales.py
index 905ba0d..304fb1b 100644
--- a/lib/lp/app/browser/tales.py
+++ b/lib/lp/app/browser/tales.py
@@ -60,6 +60,7 @@ from lp.blueprints.interfaces.specification import ISpecification
from lp.blueprints.interfaces.sprint import ISprint
from lp.bugs.interfaces.bug import IBug
from lp.buildmaster.enums import BuildStatus
+from lp.code.enums import RevisionStatusResult
from lp.code.interfaces.branch import IBranch
from lp.layers import LaunchpadLayer
from lp.registry.interfaces.distribution import IDistribution
@@ -1207,6 +1208,45 @@ class SnapBuildRequestImageDisplayAPI(ObjectImageDisplayAPI):
}
+class RevisionStatusReportImageDisplayAPI(ObjectImageDisplayAPI):
+ """Adapter for IRevisionStatusReport objects to an image.
+
+ Used for image:icon.
+ """
+ icon_template = (
+ '<img width="%(width)s" height="14" alt="%(alt)s" '
+ 'title="%(title)s" src="%(src)s" />')
+
+ def icon(self):
+ """Return the appropriate <img> tag for the result icon."""
+ icon_map = {
+ RevisionStatusResult.WAITING: {"src": "/@@/build-needed"},
+ RevisionStatusResult.RUNNING: {"src": "/@@/processing"},
+ RevisionStatusResult.SUCCEEDED: {"src": "/@@/yes"},
+ RevisionStatusResult.FAILED: {"src": "/@@/no"},
+ RevisionStatusResult.SKIPPED: {"src": "/@@/yes-gray"},
+ RevisionStatusResult.CANCELLED: {
+ "src": "/@@/build-failed",
+ "width": "16",
+ },
+ }
+
+ result = self._context.result
+ if result is None:
+ result = RevisionStatusResult.WAITING
+ alt = "[%s]" % result.name
+ title = result.title
+ source = icon_map[result].get("src")
+ width = icon_map[result].get("width", "14")
+
+ return self.icon_template % {
+ "alt": alt,
+ "title": title,
+ "src": source,
+ "width": width,
+ }
+
+
class BadgeDisplayAPI:
"""Adapter for IHasBadges to the images for the badges.
diff --git a/lib/lp/app/doc/tales.txt b/lib/lp/app/doc/tales.txt
index e68f86d..90c5080 100644
--- a/lib/lp/app/doc/tales.txt
+++ b/lib/lp/app/doc/tales.txt
@@ -229,6 +229,32 @@ But the 'failed to build' build is 16x14:
>>> print(test_tales("build/image:icon", build=build))
<img width="16" height="14"...src="/@@/build-failed" />
+Revision status reports have an icon for each result.
+
+ >>> from lp.code.enums import RevisionStatusResult
+
+ >>> report = factory.makeRevisionStatusReport()
+ >>> print(test_tales("report/image:icon", report=report))
+ <img width="14" height="14"...src="/@@/build-needed" />
+ >>> report.transitionToNewResult(RevisionStatusResult.WAITING)
+ >>> print(test_tales("report/image:icon", report=report))
+ <img width="14" height="14"...src="/@@/build-needed" />
+ >>> report.transitionToNewResult(RevisionStatusResult.RUNNING)
+ >>> print(test_tales("report/image:icon", report=report))
+ <img width="14" height="14"...src="/@@/processing" />
+ >>> report.transitionToNewResult(RevisionStatusResult.SUCCEEDED)
+ >>> print(test_tales("report/image:icon", report=report))
+ <img width="14" height="14"...src="/@@/yes" />
+ >>> report.transitionToNewResult(RevisionStatusResult.FAILED)
+ >>> print(test_tales("report/image:icon", report=report))
+ <img width="14" height="14"...src="/@@/no" />
+ >>> report.transitionToNewResult(RevisionStatusResult.SKIPPED)
+ >>> print(test_tales("report/image:icon", report=report))
+ <img width="14" height="14"...src="/@@/yes-gray" />
+ >>> report.transitionToNewResult(RevisionStatusResult.CANCELLED)
+ >>> print(test_tales("report/image:icon", report=report))
+ <img width="16" height="14"...src="/@@/build-failed" />
+
All objects can be represented as a boolean icon.
>>> print(test_tales("context/image:boolean", context=None))
diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
index 8e501cf..f774c48 100644
--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
@@ -1669,6 +1669,10 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
result_summary="120/120 tests passed",
url="https://foo.com";,
result=RevisionStatusResult.SUCCEEDED)
+ pending_report = self.factory.makeRevisionStatusReport(
+ user=bmp.source_git_repository.owner,
+ git_repository=bmp.source_git_repository,
+ title="Build", commit_sha1=sha1)
self.useFixture(GitHostingFixture(log=[
{
@@ -1695,12 +1699,18 @@ class TestBranchMergeProposalView(TestCaseWithFactory):
Tag(
"first report link", "a", text=report1.title,
attrs={"href": report1.url})))
-
self.assertThat(
reports_section[0],
Tag(
"first report summary", "td",
text=report1.result_summary))
+ self.assertThat(
+ reports_section[0],
+ Within(
+ Tag("pending report title", "td"),
+ Tag(
+ "pending report link", "a", text=pending_report.title,
+ attrs={"href": None})))
# Ensure we don't display an empty expander for those commits
# that do not have status reports created for them - means we
diff --git a/lib/lp/code/browser/tests/test_gitref.py b/lib/lp/code/browser/tests/test_gitref.py
index 552e49a..18a94f1 100644
--- a/lib/lp/code/browser/tests/test_gitref.py
+++ b/lib/lp/code/browser/tests/test_gitref.py
@@ -191,20 +191,22 @@ class TestGitRefView(BrowserTestCase):
paths=["refs/heads/branch"])
log = self.makeCommitLog()
- # create 2 status reports for 2 of the 5 commits available here
+ # create status reports for 2 of the 5 commits available here
report1 = self.factory.makeRevisionStatusReport(
user=repository.owner, git_repository=repository,
title="CI", commit_sha1=log[0]["sha1"],
result_summary="120/120 tests passed",
url="https://foo1.com";,
result=RevisionStatusResult.SUCCEEDED)
-
report2 = self.factory.makeRevisionStatusReport(
user=repository.owner, git_repository=repository,
title="Lint", commit_sha1=log[1]["sha1"],
result_summary="Invalid import in test_file.py",
url="https://foo2.com";,
result=RevisionStatusResult.FAILED)
+ pending_report = self.factory.makeRevisionStatusReport(
+ user=repository.owner, git_repository=repository,
+ title="Build", commit_sha1=log[1]["sha1"])
self.hosting_fixture.getLog.result = list(log)
# XXX jugmac00 2022-03-14
@@ -251,6 +253,13 @@ class TestGitRefView(BrowserTestCase):
soupmatchers.Tag(
"second report summary", "td",
text=report2.result_summary))
+ self.assertThat(
+ reports_section[1],
+ soupmatchers.Within(
+ soupmatchers.Tag("pending report title", "td"),
+ soupmatchers.Tag(
+ "pending report link", "a", text=pending_report.title,
+ attrs={"href": None})))
# Ensure we don't display an empty expander for those commits
# that do not have status reports created for them - means we
diff --git a/lib/lp/code/templates/git-macros.pt b/lib/lp/code/templates/git-macros.pt
index a990c18..997f96a 100644
--- a/lib/lp/code/templates/git-macros.pt
+++ b/lib/lp/code/templates/git-macros.pt
@@ -234,7 +234,7 @@
<table class="listing" tal:condition="batch">
<tbody>
<tr tal:repeat="report batch">
- <td tal:content="structure report/result/enumvalue:SUCCEEDED/image:boolean" />
+ <td class="icon" tal:content="structure report/image:icon" />
<td><a tal:attributes="href report/url|nothing" tal:content="report/title" /></td>
<td tal:content="report/result_summary" />
</tr>
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index e145133..d8179b9 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -122,7 +122,6 @@ from lp.code.enums import (
GitRepositoryType,
RevisionControlSystems,
RevisionStatusArtifactType,
- RevisionStatusResult,
TargetRevisionControlSystems,
)
from lp.code.errors import UnknownBranchTypeError
@@ -1876,8 +1875,6 @@ class BareLaunchpadObjectFactory(ObjectFactory):
commit_sha1 = hashlib.sha1(self.getUniqueBytes()).hexdigest()
if result_summary is None:
result_summary = self.getUniqueUnicode()
- if result is None:
- result = RevisionStatusResult.RUNNING
return getUtility(IRevisionStatusReportSet).new(
user, title, git_repository, commit_sha1, url,
result_summary, result, ci_build=ci_build)
