launchpad-reviewers team mailing list archive

[Colin Watson <mp+429370@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:reliable-test-latest-log into launchpad:master.

Commit message:
Make TestRevisionStatusReport.test_latest_log reliable

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/429370

`RevisionStatusArtifact`s created in the same transaction have the same creation date by default.  Override this so that `latest_log` has a predictable result.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:reliable-test-latest-log into launchpad:master.

diff --git a/lib/lp/code/interfaces/revisionstatus.py b/lib/lp/code/interfaces/revisionstatus.py
index 7c05797..c0303bb 100644
--- a/lib/lp/code/interfaces/revisionstatus.py
+++ b/lib/lp/code/interfaces/revisionstatus.py
@@ -313,7 +313,7 @@ class IRevisionStatusReportSet(Interface):
 class IRevisionStatusArtifactSet(Interface):
     """The set of all revision status artifacts."""
 
-    def new(lfa, report, artifact_type):
+    def new(lfa, report, artifact_type, date_created=None):
         """Return a new revision status artifact.
 
         :param lfa: An `ILibraryFileAlias`.
diff --git a/lib/lp/code/model/revisionstatus.py b/lib/lp/code/model/revisionstatus.py
index ebc5914..518c5a8 100644
--- a/lib/lp/code/model/revisionstatus.py
+++ b/lib/lp/code/model/revisionstatus.py
@@ -25,7 +25,7 @@ from lp.code.interfaces.revisionstatus import (
     IRevisionStatusReport,
     IRevisionStatusReportSet,
 )
-from lp.services.database.constants import UTC_NOW
+from lp.services.database.constants import DEFAULT, UTC_NOW
 from lp.services.database.enumcol import DBEnum
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import convert_storm_clause_to_string
@@ -303,12 +303,14 @@ class RevisionStatusArtifact(StormBase):
         name="date_created", tzinfo=pytz.UTC, allow_none=True
     )
 
-    def __init__(self, library_file, report, artifact_type):
+    def __init__(
+        self, library_file, report, artifact_type, date_created=DEFAULT
+    ):
         super().__init__()
         self.library_file = library_file
         self.report = report
         self.artifact_type = artifact_type
-        self.date_created = UTC_NOW
+        self.date_created = date_created
 
     @property
     def download_url(self):
@@ -336,10 +338,12 @@ class RevisionStatusArtifact(StormBase):
 
 @implementer(IRevisionStatusArtifactSet)
 class RevisionStatusArtifactSet:
-    def new(self, lfa, report, artifact_type):
+    def new(self, lfa, report, artifact_type, date_created=DEFAULT):
         """See `IRevisionStatusArtifactSet`."""
         store = IStore(RevisionStatusArtifact)
-        artifact = RevisionStatusArtifact(lfa, report, artifact_type)
+        artifact = RevisionStatusArtifact(
+            lfa, report, artifact_type, date_created=date_created
+        )
         store.add(artifact)
         return artifact
 
diff --git a/lib/lp/code/model/tests/test_revisionstatus.py b/lib/lp/code/model/tests/test_revisionstatus.py
index fa292a9..afc7ff2 100644
--- a/lib/lp/code/model/tests/test_revisionstatus.py
+++ b/lib/lp/code/model/tests/test_revisionstatus.py
@@ -6,10 +6,12 @@
 import hashlib
 import io
 import os
+from datetime import timedelta
 from hashlib import sha1
 
 import requests
 from fixtures import FakeLogger, TempDir
+from storm.expr import Cast
 from storm.store import Store
 from testtools.matchers import (
     AnyMatch,
@@ -29,6 +31,7 @@ from lp.code.interfaces.revisionstatus import (
     IRevisionStatusReportSet,
 )
 from lp.services.auth.enums import AccessTokenScope
+from lp.services.database.constants import DEFAULT, UTC_NOW
 from lp.services.osutils import write_file
 from lp.services.webapp.authorization import check_permission
 from lp.testing import (
@@ -45,11 +48,16 @@ from lp.testing.pages import webservice_for_person
 class TestRevisionStatusReport(TestCaseWithFactory):
     layer = DatabaseFunctionalLayer
 
-    def makeRevisionStatusArtifact(self, report, artifact_type=None):
+    def makeRevisionStatusArtifact(
+        self, report, artifact_type=None, date_created=DEFAULT
+    ):
         # We don't need to upload files to the librarian in this test suite.
         lfa = self.factory.makeLibraryFileAlias(db_only=True)
         return self.factory.makeRevisionStatusArtifact(
-            lfa=lfa, report=report, artifact_type=artifact_type
+            lfa=lfa,
+            report=report,
+            artifact_type=artifact_type,
+            date_created=date_created,
         )
 
     def test_owner_public(self):
@@ -165,11 +173,16 @@ class TestRevisionStatusReport(TestCaseWithFactory):
 
     def test_latest_log(self):
         report = self.factory.makeRevisionStatusReport()
-        self.makeRevisionStatusArtifact(report=report)
-        self.makeRevisionStatusArtifact(report=report)
-        artifact3 = self.makeRevisionStatusArtifact(report=report)
+        artifacts = [
+            self.makeRevisionStatusArtifact(
+                report=report,
+                date_created=UTC_NOW
+                - Cast(timedelta(seconds=age), "interval"),
+            )
+            for age in (2, 1, 0)
+        ]
         with person_logged_in(report.git_repository.owner):
-            self.assertEqual(artifact3, report.latest_log)
+            self.assertEqual(artifacts[-1], report.latest_log)
 
 
 class TestRevisionStatusReportWebservice(TestCaseWithFactory):
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 1f32ed9..1667cfd 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2219,6 +2219,7 @@ class LaunchpadObjectFactory(ObjectFactory):
         report=None,
         artifact_type=None,
         restricted=False,
+        date_created=DEFAULT,
     ):
         """Create a new RevisionStatusArtifact."""
         if lfa is None:
@@ -2230,7 +2231,7 @@ class LaunchpadObjectFactory(ObjectFactory):
         if artifact_type is None:
             artifact_type = RevisionStatusArtifactType.LOG
         return getUtility(IRevisionStatusArtifactSet).new(
-            lfa, report, artifact_type
+            lfa, report, artifact_type, date_created=date_created
         )
 
     def makeBug(