Skip to content

Commit 19ba268

Browse files
jonfroehlichclaude
andcommitted
Pin what may clear the nightly-jobs overdue alarm (#4928)
The alarm exists to catch a scheduler that has stopped, so the load-bearing rule is what counts as evidence it is still running: only a scheduled run that succeeded. A run someone triggered by hand from /adminapi shows the code works, not that anything still fires it, and a run merely in flight has proved nothing yet -- and both land under the same job_name as the nightly one, so this rule is all that separates them. That rule lived only in an expression inside a private method. Anyone simplifying it back to "the latest run, whatever it was" would reintroduce both bugs silently, since the panel looks healthier afterwards, not worse. Five cases, driven through getDbHealth against seeded history, including a control that pins overdue can be false at all -- without it the suite would pass against an implementation that never cleared. Confirmed the suite discriminates by reverting the fix in a throwaway copy: the hand-triggered and in-flight cases fail (false was not equal to true, and its converse), the other three pass on both implementations. Enrolled in the ci.yml allowlist, since a spec absent from it is dead weight while CI stays green -- the trap this PR set out to close. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent e6df51e commit 19ba268

2 files changed

Lines changed: 148 additions & 1 deletion

File tree

.github/workflows/ci.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,11 @@ jobs:
305305
# transition records the admin trend and nightly-job panels read. Each seeds its own rows rather than hunting
306306
# for them, so they assert real behavior against an empty seed: the expiry stamp only lands on the false ->
307307
# true edge, a run row is open exactly while it is running, and a wrapped job's failure propagates unchanged.
308+
# - NightlyJobStatusSpec (#4928): pins what may clear the Health panel's overdue alarm -- only a scheduled run
309+
# that succeeded. A hand-triggered run and an in-flight one are both recorded under the same job_name as the
310+
# nightly one, so this rule is the only thing keeping either from masking a scheduler that has stopped.
308311
- name: Run gating/auth tests (health dashboard + route auth posture + geodesic distances)
309-
run: sbt 'set Test / parallelExecution := false' 'testOnly controllers.HealthDashboardSpec service.HealthServiceSpec controllers.RouteAuthPostureSpec models.street.GeodesicDistanceSpec service.ExploreTutorialRouteSpec controllers.MobileDetectionSpec service.PanoDataServiceSpec models.utils.ConfigTableVoidedArchiveSpec controllers.api.StatsApiSpec controllers.ExploreSubmissionSpec controllers.ValidateSubmissionSpec controllers.ExploreNoImageryRateLimitSpec models.pano.PanoExpiredAtSpec models.utils.BackgroundJobRunTableSpec service.JobRunServiceSpec service.StreetLifecycleServiceSpec models.audit.OutdatedImageryFlagSyncSpec models.audit.OutdatedImageryRoutingSpec models.street.UpToDateCoverageSpec'
312+
run: sbt 'set Test / parallelExecution := false' 'testOnly controllers.HealthDashboardSpec service.HealthServiceSpec controllers.RouteAuthPostureSpec models.street.GeodesicDistanceSpec service.ExploreTutorialRouteSpec controllers.MobileDetectionSpec service.PanoDataServiceSpec models.utils.ConfigTableVoidedArchiveSpec controllers.api.StatsApiSpec controllers.ExploreSubmissionSpec controllers.ValidateSubmissionSpec controllers.ExploreNoImageryRateLimitSpec models.pano.PanoExpiredAtSpec models.utils.BackgroundJobRunTableSpec service.JobRunServiceSpec service.NightlyJobStatusSpec service.StreetLifecycleServiceSpec models.audit.OutdatedImageryFlagSyncSpec models.audit.OutdatedImageryRoutingSpec models.street.UpToDateCoverageSpec'
310313
env:
311314
DATABASE_URL: jdbc:postgresql://localhost:5432/sidewalk
312315
DATABASE_USER: sidewalk
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
package service
2+
3+
import actor.ScheduledJobs
4+
import models.utils.MyPostgresProfile.api._
5+
import models.utils.{BackgroundJobRunTable, JobRunStatus, JobRunTrigger, MyPostgresProfile}
6+
import org.scalatest.BeforeAndAfterAll
7+
import org.scalatestplus.play.PlaySpec
8+
import org.scalatestplus.play.guice.GuiceOneAppPerSuite
9+
import play.api.Application
10+
import play.api.cache.AsyncCacheApi
11+
import play.api.db.slick.DatabaseConfigProvider
12+
import play.api.inject.guice.GuiceApplicationBuilder
13+
import slick.dbio.DBIO
14+
15+
import java.time.OffsetDateTime
16+
import scala.concurrent.duration._
17+
import scala.concurrent.{Await, Future}
18+
19+
/**
20+
* DB-backed test for how the Health dashboard decides a nightly job is overdue (#4928).
21+
*
22+
* The panel exists to catch a scheduler that has silently stopped, so the property that matters is what is allowed to
23+
* clear its alarm: only a *scheduled* run that *succeeded*. A run someone triggered by hand from /adminapi proves the
24+
* code works, not that anything is still firing it, and a run that is merely in flight has not proved anything yet.
25+
* Both are recorded under the same `job_name` as the nightly one, so nothing but this rule separates them.
26+
*
27+
* Every case asserts against a seeded history rather than whatever the connected database holds, and the control case
28+
* pins that `overdue` can be false at all — without it, every assertion here would pass on a bug that hard-coded true.
29+
*
30+
* Requires a Postgres database (DATABASE_URL / DATABASE_USER / DATABASE_PASSWORD, as in dev/CI); the scheduling actors
31+
* are disabled so a real run can't land mid-test.
32+
*/
33+
class NightlyJobStatusSpec extends PlaySpec with BeforeAndAfterAll with GuiceOneAppPerSuite {
34+
35+
override def fakeApplication(): Application =
36+
new GuiceApplicationBuilder().disable[modules.ActorModule].build()
37+
38+
private val healthService = app.injector.instanceOf[HealthService]
39+
private val jobRunTable = app.injector.instanceOf[BackgroundJobRunTable]
40+
private val cacheApi = app.injector.instanceOf[AsyncCacheApi]
41+
private val dbConfig = app.injector.instanceOf[DatabaseConfigProvider].get[MyPostgresProfile]
42+
43+
private def await[T](f: Future[T]): T = Await.result(f, 120.seconds)
44+
private def run[T](action: DBIO[T]): T = Await.result(dbConfig.db.run(action), 60.seconds)
45+
46+
/** The imagery sweep, because it is the one job with a real hand-trigger route (`/adminapi/checkImagery`). */
47+
private val jobName = ScheduledJobs.CheckImageExpiry.name
48+
49+
private def clearRuns(): Unit = {
50+
val _ = run(sqlu"DELETE FROM background_job_run WHERE job_name = $jobName")
51+
}
52+
53+
/** Seeds one finished run. */
54+
private def seedFinished(
55+
trigger: JobRunTrigger.Value,
56+
status: JobRunStatus.Value,
57+
startedAt: OffsetDateTime
58+
): Unit = {
59+
val id = run(jobRunTable.insertRunning(jobName, trigger, startedAt))
60+
val error = if (status == JobRunStatus.Failed) Some("seeded failure") else None
61+
val _ = run(jobRunTable.finish(id, status, startedAt.plusMinutes(1), None, error))
62+
}
63+
64+
/** Seeds a run that is still open, as a job the app died in the middle of would be. */
65+
private def seedRunning(startedAt: OffsetDateTime): Unit = {
66+
val _ = run(jobRunTable.insertRunning(jobName, JobRunTrigger.Scheduled, startedAt))
67+
}
68+
69+
/**
70+
* The panel's row for this job.
71+
*
72+
* The service caches its job read, so the cache is dropped first — otherwise every case after the first would
73+
* assert against the previous one's history.
74+
*/
75+
private def jobStatus(): NightlyJobStatus = {
76+
await(cacheApi.removeAll())
77+
await(healthService.getDbHealth).nightlyJobs
78+
.find(_.jobName == jobName)
79+
.getOrElse(fail(s"$jobName is missing from the nightly-jobs roster"))
80+
}
81+
82+
override def beforeAll(): Unit = { super.beforeAll(); clearRuns() }
83+
override def afterAll(): Unit = { clearRuns(); super.afterAll() }
84+
85+
"the nightly-jobs panel" should {
86+
"report a job with no runs at all as overdue" in {
87+
clearRuns()
88+
val job = jobStatus()
89+
job.lastStatus mustBe "never_run"
90+
job.overdue mustBe true
91+
}
92+
93+
"clear the alarm for a recent scheduled success" in {
94+
// The control. Every other case asserts overdue is true, so without this one they would all pass against an
95+
// implementation that never cleared.
96+
clearRuns()
97+
seedFinished(JobRunTrigger.Scheduled, JobRunStatus.Succeeded, OffsetDateTime.now.minusHours(2))
98+
val job = jobStatus()
99+
job.lastStatus mustBe "succeeded"
100+
job.overdue mustBe false
101+
}
102+
103+
"keep the alarm raised when only a hand-triggered run has succeeded recently" in {
104+
clearRuns()
105+
seedFinished(
106+
JobRunTrigger.Scheduled,
107+
JobRunStatus.Succeeded,
108+
OffsetDateTime.now.minusHours(ScheduledJobs.OverdueAfterHours + 12)
109+
)
110+
seedFinished(JobRunTrigger.Manual, JobRunStatus.Succeeded, OffsetDateTime.now)
111+
112+
val job = jobStatus()
113+
// The manual run is still the last run and is still shown as one -- it just can't stand in for the scheduler.
114+
job.lastStatus mustBe "succeeded"
115+
job.lastTriggeredBy mustBe Some("manual")
116+
job.overdue mustBe true
117+
}
118+
119+
"not raise the alarm while a run is in flight, if the last scheduled run succeeded" in {
120+
clearRuns()
121+
seedFinished(JobRunTrigger.Scheduled, JobRunStatus.Succeeded, OffsetDateTime.now.minusHours(2))
122+
seedRunning(OffsetDateTime.now)
123+
124+
val job = jobStatus()
125+
job.lastStatus mustBe "running"
126+
job.overdue mustBe false
127+
}
128+
129+
"keep the alarm raised for a job that has been failing since its last success" in {
130+
clearRuns()
131+
seedFinished(
132+
JobRunTrigger.Scheduled,
133+
JobRunStatus.Succeeded,
134+
OffsetDateTime.now.minusHours(ScheduledJobs.OverdueAfterHours + 12)
135+
)
136+
seedFinished(JobRunTrigger.Scheduled, JobRunStatus.Failed, OffsetDateTime.now)
137+
138+
val job = jobStatus()
139+
job.lastStatus mustBe "failed"
140+
job.overdue mustBe true
141+
job.failuresInWindow must be >= 1
142+
}
143+
}
144+
}

0 commit comments

Comments
 (0)