diff --git a/server/test/helpers/heartbeatTestHarness.ts b/server/test/helpers/heartbeatTestHarness.ts index f99be611c..d1166f238 100644 --- a/server/test/helpers/heartbeatTestHarness.ts +++ b/server/test/helpers/heartbeatTestHarness.ts @@ -10,6 +10,8 @@ import { InMemoryMonitorsRepository } from "./InMemoryMonitorsRepository.ts"; import { InMemoryIncidentsRepository } from "./InMemoryIncidentsRepository.ts"; import { createMockLogger } from "./createMockLogger.ts"; import type { Monitor } from "../../src/domain/monitors/monitor.type.ts"; +import type { Check } from "../../src/domain/checks/check.type.ts"; +import type { MonitorActionDecision } from "../../src/worker/worker.interface.ts"; import type { MonitorStatusResponse } from "../../src/types/network.ts"; import type { MaintenanceWindow } from "../../src/domain/maintenance-windows/maintenance-window.type.ts"; import type { EgressStatus } from "../../src/domain/egress/egress.type.ts"; @@ -117,7 +119,13 @@ export function createHeartbeatTestHarness(): HeartbeatTestHarness { nextEgressStatus = status; }; - const notificationReactor = new NotificationReactor(notificationsService as any); + // The real NotificationsService ignores decisions with shouldSendNotification false; the stub is reached only for + // the ones it would act on, so tests can assert on handleNotifications calls as "notifications sent". + const guardedNotificationsService = { + handleNotifications: (monitor: Monitor, check: Check, decision: MonitorActionDecision) => + decision.shouldSendNotification ? notificationsService.handleNotifications(monitor, check, decision) : Promise.resolve(false), + }; + const notificationReactor = new NotificationReactor(guardedNotificationsService as any); const incidentReactor = new IncidentReactor(incidentService as any); const reactorDispatcher = new ReactorDispatcher(logger, [notificationReactor, incidentReactor]); diff --git a/server/test/integration/evaluateCursorMultiWorker.test.ts b/server/test/integration/evaluateCursorMultiWorker.test.ts index a3fe75b04..5f40d2eef 100644 --- a/server/test/integration/evaluateCursorMultiWorker.test.ts +++ b/server/test/integration/evaluateCursorMultiWorker.test.ts @@ -133,7 +133,7 @@ describe("Evaluate cursor with more than one processing worker", () => { statusService: { updateMonitorStatus: async (check: Check, monitor: Monitor) => { evaluatedCheckIds.push(check.id); - return { monitor, statusChanged: false, prevStatus: monitor.status, code: 200, timestamp: 0 }; + return { monitor, statusChanged: false, prevStatus: monitor.status }; }, } as any, dispatcher: { dispatch: jest.fn().mockResolvedValue(undefined) } as any, diff --git a/server/test/unit/services/notificationMessageBuilder.test.ts b/server/test/unit/services/notificationMessageBuilder.test.ts index 199d80101..472542e09 100644 --- a/server/test/unit/services/notificationMessageBuilder.test.ts +++ b/server/test/unit/services/notificationMessageBuilder.test.ts @@ -144,15 +144,6 @@ describe("NotificationMessageBuilder", () => { expect(msg.severity).toBe("success"); expect(msg.content.title).toBe("Thresholds Resolved: Test Monitor"); }); - - it("uses notificationReason from decision, falling back to status_change", () => { - const monitor = makeMonitor({ status: "down" }); - const decision = makeDecision({ notificationReason: null }); - - const msg = builder.buildMessage(monitor, makeCheck(), decision, "https://app.example.com"); - - expect(msg.metadata.notificationReason).toBe("status_change"); - }); }); // ── determineNotificationType (via buildMessage) ───────────────────── diff --git a/server/test/unit/services/statusService.test.ts b/server/test/unit/services/statusService.test.ts index 50005312d..031df1a46 100644 --- a/server/test/unit/services/statusService.test.ts +++ b/server/test/unit/services/statusService.test.ts @@ -131,92 +131,86 @@ const makeExistingStats = (overrides?: Record) => ({ // ── Tests ──────────────────────────────────────────────────────────────────── describe("StatusService", () => { - // ── updateRunningStats ─────────────────────────────────────────────────── + // ── running stats (via updateMonitorStatus) ────────────────────────────── - describe("updateRunningStats", () => { - // The service now delegates the entire running-stats computation to the repository's - // atomic `updateByMonitorId`, so the service-level tests only verify forwarding and error - // handling. The math (running average, uptimePercentage, timeOfLastFailure state machine) - // is a repository-level concern and is exercised by the repository tests. + describe("running stats", () => { + // The service delegates the running-stats computation to the repository's atomic `updateByMonitorId`, + // so these cases only verify what is forwarded and that a failure never blocks the status update. + // The math (running average, uptimePercentage, timeOfLastFailure) is exercised by the repository tests. + + const run = async (check: Check) => { + const monitor = makeMonitor(); + const deps = createService(); + (deps.monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor); + (deps.monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); + const result = await deps.service.updateMonitorStatus(check, monitor); + return { ...deps, result }; + }; it("forwards a successful check to updateByMonitorId", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); + const { monitorStatsRepository } = await run(makeCheck({ responseTime: 50 })); - const result = await service.updateRunningStats(makeCheck({ responseTime: 50 }), makeMonitor()); - - expect(result).toBe(true); expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith( "mon-1", - expect.objectContaining({ - status: true, - responseTime: 50, - now: expect.any(Number), - }) + expect.objectContaining({ status: true, responseTime: 50, now: expect.any(Number) }) ); }); it("forwards a failed check to updateByMonitorId", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); + const { monitorStatsRepository } = await run(makeCheck({ status: false, responseTime: 100 })); - const result = await service.updateRunningStats(makeCheck({ status: false, responseTime: 100 }), makeMonitor()); - - expect(result).toBe(true); expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ status: false, responseTime: 100 })); }); it("passes 0 when responseTime is undefined", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - - await service.updateRunningStats(makeCheck({ responseTime: undefined as any }), makeMonitor()); + const { monitorStatsRepository } = await run(makeCheck({ responseTime: undefined as any })); expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ responseTime: 0 })); }); it("forwards responseTime of 0 as 0 (falsy but defined)", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - - await service.updateRunningStats(makeCheck({ responseTime: 0 }), makeMonitor()); + const { monitorStatsRepository } = await run(makeCheck({ responseTime: 0 })); expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ responseTime: 0 })); }); it("coerces a non-true status to false (e.g. undefined)", async () => { - const { service, monitorStatsRepository } = createService(); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockResolvedValue({}); - - await service.updateRunningStats(makeCheck({ status: undefined as any }), makeMonitor()); + const { monitorStatsRepository } = await run(makeCheck({ status: undefined as any })); expect(monitorStatsRepository.updateByMonitorId).toHaveBeenCalledWith("mon-1", expect.objectContaining({ status: false })); }); - it("returns false and logs error when updateByMonitorId throws", async () => { - const { service, logger, monitorStatsRepository } = createService(); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue(new Error("db write failed")); + it("warns once and still updates the status when updateByMonitorId throws", async () => { + const monitor = makeMonitor({ statusWindow: [true, true, true, true], statusWindowSize: 5, status: "up" }); + const { service, logger, monitorsRepository, monitorStatsRepository } = createService(); + (monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor); + (monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue(new Error("stats db down")); - const result = await service.updateRunningStats(makeCheck(), makeMonitor()); + const result = await service.updateMonitorStatus(makeCheck({ status: true }), monitor); - expect(result).toBe(false); - expect(logger.error).toHaveBeenCalledWith( + expect(result.monitor.status).toBe("up"); + expect(monitorsRepository.updateStatusWindowAndChecks).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( expect.objectContaining({ service: "StatusService", - method: "updateRunningStats", - message: "db write failed", + method: "tryUpdateRunningStats", + message: "stats db down", + stack: expect.any(String), }) ); + expect(logger.error).not.toHaveBeenCalled(); }); - it("logs error with 'Unknown error' for non-Error exceptions", async () => { - const { service, logger, monitorStatsRepository } = createService(); + it("warns with 'Unknown error' for non-Error rejections", async () => { + const monitor = makeMonitor(); + const { service, logger, monitorsRepository, monitorStatsRepository } = createService(); + (monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor); (monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue("string error"); - const result = await service.updateRunningStats(makeCheck(), makeMonitor()); + await service.updateMonitorStatus(makeCheck(), monitor); - expect(result).toBe(false); - expect(logger.error).toHaveBeenCalledWith(expect.objectContaining({ message: "Unknown error", stack: undefined })); + expect(logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: "Unknown error", stack: undefined })); }); }); @@ -640,37 +634,6 @@ describe("StatusService", () => { expect(result.monitor.status).toBe("down"); }); - it("logs a warning but still succeeds when running-stats update fails mid-flight", async () => { - const monitor = makeMonitor({ statusWindow: [true, true, true, true], statusWindowSize: 5, status: "up" }); - const { service, logger, monitorsRepository, monitorStatsRepository } = createService(); - (monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor); - (monitorsRepository.updateById as jest.Mock).mockImplementation((_id: unknown, _tid: unknown, m: unknown) => Promise.resolve(m)); - (monitorStatsRepository.updateByMonitorId as jest.Mock).mockRejectedValue(new Error("stats db down")); - - const result = await service.updateMonitorStatus(makeCheck({ status: true }), monitor); - - expect(result.monitor.status).toBe("up"); - expect(logger.warn).toHaveBeenCalledWith( - expect.objectContaining({ - service: "StatusService", - method: "updateMonitorStatus", - message: expect.stringContaining("Stats update failed"), - }) - ); - }); - - it("returns code and timestamp in result", async () => { - const monitor = makeMonitor({ statusWindow: [true, true, true, true], statusWindowSize: 5 }); - const { service, monitorsRepository } = createService(); - (monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor); - (monitorsRepository.updateById as jest.Mock).mockImplementation((_id: unknown, _tid: unknown, m: unknown) => Promise.resolve(m)); - - const result = await service.updateMonitorStatus(makeCheck({ statusCode: 201 }), monitor); - - expect(result.code).toBe(201); - expect(result.timestamp).toBeGreaterThan(0); - }); - // ── Hardware threshold breach tests ────────────────────────────────── describe("hardware threshold breaches", () => { diff --git a/server/test/unit/services/workerPipeline.test.ts b/server/test/unit/services/workerPipeline.test.ts index bbb389573..fbaf2eb73 100644 --- a/server/test/unit/services/workerPipeline.test.ts +++ b/server/test/unit/services/workerPipeline.test.ts @@ -64,8 +64,6 @@ const makeStatusChange = (overrides: Partial & { status?: st monitor: { id: "m1", status } as any, statusChanged: false, prevStatus: "up", - code: 200, - timestamp: 0, ...rest, } as StatusChangeResult; }; @@ -706,7 +704,7 @@ describe("WorkerPipeline", () => { it("returns an evaluation that opens an incident on a down transition", async () => { const check = makeCheck({ status: false, statusCode: 500, message: "Error" }); - const statusChange = makeStatusChange({ monitor: makeMonitor({ status: "down" }), statusChanged: true, prevStatus: "up", code: 500 }); + const statusChange = makeStatusChange({ monitor: makeMonitor({ status: "down" }), statusChanged: true, prevStatus: "up" }); const { pipeline } = createPipeline({ statusService: { updateMonitorStatus: jest.fn().mockResolvedValue(statusChange) }, }); @@ -802,7 +800,7 @@ describe("WorkerPipeline", () => { }); it("creates incident and notifies when monitor goes down", async () => { - const decision = await decide(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up", code: 500 })); + const decision = await decide(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up" })); expect(decision).toMatchObject({ shouldCreateIncident: true, shouldSendNotification: true, @@ -854,7 +852,7 @@ describe("WorkerPipeline", () => { it("leaves threshold breaches off a down decision even when the status change carries them", async () => { const thresholdBreaches = { cpu: true, memory: false, disk: false, temp: false }; - const decision = await decide(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up", code: 500, thresholdBreaches })); + const decision = await decide(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up", thresholdBreaches })); expect(decision).not.toHaveProperty("thresholdBreaches"); }); @@ -876,8 +874,8 @@ describe("WorkerPipeline", () => { it("starts every decision clean after a transition on the same pipeline", async () => { const updateMonitorStatus = jest .fn() - .mockResolvedValueOnce(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up", code: 500 })) - .mockResolvedValueOnce(makeStatusChange({ status: "down", statusChanged: false, prevStatus: "down", code: 500 })); + .mockResolvedValueOnce(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up" })) + .mockResolvedValueOnce(makeStatusChange({ status: "down", statusChanged: false, prevStatus: "down" })); const { pipeline } = createPipeline({ statusService: { updateMonitorStatus } }); const failing = makeCheck({ status: false, statusCode: 500, message: "Error" });