Repository navigation
add channel connectivity and command #562
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { Command } from "commander"; | |
| import type Database from "better-sqlite3"; | ||
| import { getDatabaseForTesting } from "../../src/db/database"; | ||
| import { registerAlertsCommand } from "../../src/commands/alerts"; | ||
| import { registerAlertChannel } from "../../src/alerts/registry"; | ||
| import { | ||
| insertContract, | ||
| getAlertConfigsForContract, | ||
|
|
@@ -761,4 +762,95 @@ describe("alerts command", () => { | |
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("alerts test-all", () => { | ||
| beforeEach(() => { | ||
| mockDeliverSingleAlert.mockReset(); | ||
| insertAlertConfig(mockDb, { | ||
| contract_id: contractID, | ||
| channel_type: "webhook", | ||
| channel_target: "https://example.com/webhook", | ||
| threshold_ledgers: 1000, | ||
| }); | ||
| insertAlertConfig(mockDb, { | ||
| contract_id: contractID, | ||
| channel_type: "slack", | ||
| channel_target: "#ops-alerts", | ||
| threshold_ledgers: 2000, | ||
| }); | ||
| }); | ||
|
|
||
| it("sends a test event to every configured channel for the contract", async () => { | ||
| mockDeliverSingleAlert.mockResolvedValue(true); | ||
|
|
||
| const program = new Command(); | ||
| registerAlertsCommand(program); | ||
|
|
||
| await program.parseAsync([ | ||
| "node", "sorokeep", "alerts", "test-all", | ||
| "--contract", contractID, | ||
| ]); | ||
|
|
||
| expect(mockDeliverSingleAlert).toHaveBeenCalledTimes(2); | ||
| expect(mockDeliverSingleAlert.mock.calls[0]![0]).toBe("webhook"); | ||
| expect(mockDeliverSingleAlert.mock.calls[0]![1]).toBe("https://example.com/webhook"); | ||
| expect(mockDeliverSingleAlert.mock.calls[1]![0]).toBe("slack"); | ||
| expect(mockDeliverSingleAlert.mock.calls[1]![1]).toBe("#ops-alerts"); | ||
|
|
||
| for (const [, , event] of mockDeliverSingleAlert.mock.calls) { | ||
| expect(event.type).toBe("threshold_crossed"); | ||
| expect(event.contractId).toBe(contractID); | ||
| } | ||
| }); | ||
|
|
||
| it("exits with 1 when any delivery fails", async () => { | ||
| mockDeliverSingleAlert.mockResolvedValueOnce(true).mockResolvedValueOnce(false); | ||
|
|
||
| const program = new Command(); | ||
| registerAlertsCommand(program); | ||
|
|
||
| await expect( | ||
| program.parseAsync([ | ||
| "node", "sorokeep", "alerts", "test-all", | ||
| "--contract", contractID, | ||
| ]) | ||
| ).rejects.toThrow("process.exit called"); | ||
|
|
||
| expect(mockDeliverSingleAlert).toHaveBeenCalledTimes(2); | ||
| expect(consoleErrorSpy).toHaveBeenCalledWith( | ||
| expect.stringContaining("1 channel(s) failed") | ||
| ); | ||
| expect(exitSpy).toHaveBeenCalledWith(1); | ||
| }); | ||
| }); | ||
|
|
||
| describe("alerts channels", () => { | ||
| it("prints all five built-in channel names", () => { | ||
| parse(["alerts", "channels"]); | ||
|
|
||
| const output = consoleLogSpy.mock.calls.flat().join("\n"); | ||
| expect(output).toContain("webhook"); | ||
| expect(output).toContain("slack"); | ||
| expect(output).toContain("pagerduty"); | ||
| expect(output).toContain("discord"); | ||
| expect(output).toContain("telegram"); | ||
| }); | ||
|
|
||
| it("includes dynamically registered plugin channels in the output", () => { | ||
| registerAlertChannel({ | ||
| name: "matrix-listing-test", | ||
| channel: { send: vi.fn().mockResolvedValue(undefined) }, | ||
| targetOption: "url", | ||
| missingTargetError: "Error: --url is required when --type is matrix-listing-test.", | ||
| supportsSigning: false, | ||
| }); | ||
|
|
||
| parse(["alerts", "channels"]); | ||
|
|
||
| const output = consoleLogSpy.mock.calls.flat().join("\n"); | ||
| expect(output).toContain("matrix-listing-test"); | ||
| expect(output).toContain("url"); | ||
| expect(output).toContain("no"); | ||
| }); | ||
|
Comment on lines
+839
to
+854
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check whether the registry exposes any reset/unregister capability for tests.
rg -n 'export function' src/alerts/registry.tsRepository: AbdulmalikAlayande/sorokeep Length of output: 442 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== registry.ts =="
sed -n '1,90p' src/alerts/registry.ts
echo
echo "== imports and setup in alerts.test.ts around registry/cleanup =="
rg -n 'registerAlertChannel|_resetRegistryForTesting|afterEach|afterAll|beforeEach|beforeAll|alerts channels|consoleLogSpy' tests/commands/alerts.test.ts
echo
echo "== lines 800-870 =="
sed -n '800,870p' tests/commands/alerts.test.tsRepository: AbdulmalikAlayande/sorokeep Length of output: 5577 Reset the alert channel registry after registering the plugin channel.
🤖 Prompt for AI Agents |
||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Consider parallelizing channel tests and guarding against a hanging channel.
Channels are tested one at a time with a blocking
awaitinside the loop.deliverSingleAlert/the underlyingchannel.send()don't appear to enforce a timeout, so one slow or unresponsive channel delays reporting the status of every subsequent channel — for a contract with several channels this can maketest-allhang far longer than necessary, undermining the "quick connectivity check" purpose of the command.♻️ Suggested parallelization
This still doesn't cap the worst-case wall-clock time if a channel truly hangs — pairing it with a per-call timeout would fully close the gap, but that likely needs a change in
deliverSingleAlert/channel implementations outside this file.📝 Committable suggestion
🤖 Prompt for AI Agents