Repository navigation
test(e2e): verify monitoring daemon execution cycles - #221
Conversation
|
@jaynomyaro Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughA new end-to-end Vitest test file ( ChangesDaemon Execution E2E Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/daemon-execution.test.ts`:
- Around line 84-87: The test teardown currently stops the daemon and restores
timers, but it leaves the better-sqlite3 connection open. Update the afterEach
cleanup in the daemon execution test to also close the database connection by
calling db.close(), alongside stopDaemon() and vi.useRealTimers(), so the test
fixture releases its handle properly.
- Around line 291-310: The test in startDaemon is order-dependent because
mockGetEntryTTLs uses chained once-mocks that assume a specific contract
iteration order. Update the mock setup to return results based on the input
contract key (or otherwise make the assertions order-agnostic) so CONTRACT_OK
still succeeds and the failing contract still produces one error regardless of
processing order.
- Around line 247-312: The resilience suite is missing coverage for database
write locks and sequence errors. Extend the existing “Error resilience in real
execution” tests in daemon-execution.test.ts around startDaemon and onCycle to
simulate a SQLITE_BUSY failure and a sequence error from the acceptance
criteria, then verify the daemon still completes the failed cycle with an error
and successfully runs the next cycle afterward. Reuse the same mock-driven
pattern used for mockGetEntryTTLs and assert the daemon keeps progressing across
cycles despite each failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f7c6ab37-362b-469d-8e4a-8f212b05156f
📒 Files selected for processing (1)
tests/e2e/daemon-execution.test.ts
| afterEach(() => { | ||
| stopDaemon(); | ||
| vi.useRealTimers(); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Close the test database in teardown.
afterEach stops timers/daemon but does not close the better-sqlite3 connection. Add db.close() to avoid leaked handles across long test runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/daemon-execution.test.ts` around lines 84 - 87, The test teardown
currently stops the daemon and restores timers, but it leaves the better-sqlite3
connection open. Update the afterEach cleanup in the daemon execution test to
also close the database connection by calling db.close(), alongside stopDaemon()
and vi.useRealTimers(), so the test fixture releases its handle properly.
| describe("Error resilience in real execution", () => { | ||
| it("continues daemon execution after RPC failures", async () => { | ||
| seedContract(db, "CONTRACT_RESILIENT", "testnet", [ | ||
| { keyXdr: "resilient-key", type: "instance", liveUntil: LEDGER + 50000 }, | ||
| ]); | ||
|
|
||
| // First cycle fails | ||
| mockGetEntryTTLs.mockRejectedValueOnce(new Error("RPC timeout")); | ||
|
|
||
| // Second cycle succeeds | ||
| mockGetEntryTTLs.mockResolvedValueOnce({ | ||
| latestLedger: LEDGER, | ||
| entries: [ | ||
| { entryKeyXdr: "resilient-key", liveUntilLedgerSeq: LEDGER + 48000, lastModifiedLedgerSeq: LEDGER - 10, remainingTTL: 48000 }, | ||
| ], | ||
| }); | ||
|
|
||
| const onCycle = vi.fn(); | ||
| await startDaemon(db, "testnet", { intervalMs: 5000, onCycle }); | ||
|
|
||
| // First cycle should complete with error | ||
| expect(onCycle).toHaveBeenCalledTimes(1); | ||
| expect(onCycle.mock.calls[0][0]).toBeNull(); | ||
| expect(onCycle.mock.calls[0][1]).toBeInstanceOf(Error); | ||
|
|
||
| // Advance to trigger second cycle | ||
| await vi.advanceTimersByTimeAsync(5000); | ||
|
|
||
| // Second cycle should succeed | ||
| expect(onCycle).toHaveBeenCalledTimes(2); | ||
| const result = onCycle.mock.calls[1][0] as MonitorCycleResult; | ||
| expect(result).not.toBeNull(); | ||
| expect(result.contractsChecked).toBe(1); | ||
| }); | ||
|
|
||
| it("handles partial contract failures gracefully", async () => { | ||
| seedContract(db, "CONTRACT_OK", "testnet", [ | ||
| { keyXdr: "ok-key", type: "instance", liveUntil: LEDGER + 50000 }, | ||
| ]); | ||
| seedContract(db, "CONTRACT_FAIL", "testnet", [ | ||
| { keyXdr: "fail-key", type: "instance", liveUntil: LEDGER + 50000 }, | ||
| ]); | ||
|
|
||
| // First contract succeeds, second fails | ||
| mockGetEntryTTLs | ||
| .mockResolvedValueOnce({ | ||
| latestLedger: LEDGER, | ||
| entries: [ | ||
| { entryKeyXdr: "ok-key", liveUntilLedgerSeq: LEDGER + 48000, lastModifiedLedgerSeq: LEDGER - 10, remainingTTL: 48000 }, | ||
| ], | ||
| }) | ||
| .mockRejectedValueOnce(new Error("Connection refused")); | ||
|
|
||
| const onCycle = vi.fn(); | ||
| await startDaemon(db, "testnet", { intervalMs: 5000, onCycle }); | ||
|
|
||
| const result = onCycle.mock.calls[0][0] as MonitorCycleResult; | ||
| expect(result.contractsChecked).toBe(2); | ||
| expect(result.entriesUpdated).toBe(1); | ||
| expect(result.errors).toHaveLength(1); | ||
|
|
||
| // Verify the successful contract was updated | ||
| const okEntries = getEntriesForContract(db, "CONTRACT_OK"); | ||
| expect(okEntries[0].live_until_ledger).toBe(LEDGER + 48000); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Missing unstable-condition coverage required by this PR.
The resilience suite validates RPC and partial contract failures, but it does not simulate database write locks (SQLITE_BUSY) or sequence errors from the linked acceptance criteria. Please add explicit tests proving the daemon continues subsequent cycles after each of those failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/daemon-execution.test.ts` around lines 247 - 312, The resilience
suite is missing coverage for database write locks and sequence errors. Extend
the existing “Error resilience in real execution” tests in
daemon-execution.test.ts around startDaemon and onCycle to simulate a
SQLITE_BUSY failure and a sequence error from the acceptance criteria, then
verify the daemon still completes the failed cycle with an error and
successfully runs the next cycle afterward. Reuse the same mock-driven pattern
used for mockGetEntryTTLs and assert the daemon keeps progressing across cycles
despite each failure.
| mockGetEntryTTLs | ||
| .mockResolvedValueOnce({ | ||
| latestLedger: LEDGER, | ||
| entries: [ | ||
| { entryKeyXdr: "ok-key", liveUntilLedgerSeq: LEDGER + 48000, lastModifiedLedgerSeq: LEDGER - 10, remainingTTL: 48000 }, | ||
| ], | ||
| }) | ||
| .mockRejectedValueOnce(new Error("Connection refused")); | ||
|
|
||
| const onCycle = vi.fn(); | ||
| await startDaemon(db, "testnet", { intervalMs: 5000, onCycle }); | ||
|
|
||
| const result = onCycle.mock.calls[0][0] as MonitorCycleResult; | ||
| expect(result.contractsChecked).toBe(2); | ||
| expect(result.entriesUpdated).toBe(1); | ||
| expect(result.errors).toHaveLength(1); | ||
|
|
||
| // Verify the successful contract was updated | ||
| const okEntries = getEntriesForContract(db, "CONTRACT_OK"); | ||
| expect(okEntries[0].live_until_ledger).toBe(LEDGER + 48000); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Order-dependent mocking makes this test flaky.
This test assumes contract processing order by chaining mockResolvedValueOnce then mockRejectedValueOnce. If contract iteration order changes, the wrong contract fails and assertions become nondeterministic. Prefer argument-based mock behavior (or order-agnostic assertions).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/daemon-execution.test.ts` around lines 291 - 310, The test in
startDaemon is order-dependent because mockGetEntryTTLs uses chained once-mocks
that assume a specific contract iteration order. Update the mock setup to return
results based on the input contract key (or otherwise make the assertions
order-agnostic) so CONTRACT_OK still succeeds and the failing contract still
produces one error regardless of processing order.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | Generic High Entropy Secret | 7efd589 | tests/commands/channels.test.ts | View secret |
| - | - | Generic High Entropy Secret | 617e5cd | tests/rpc/client.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
2 participants
Summary
Add end-to-end tests to validate monitoring daemon execution cycles, ensuring scheduled monitoring tasks run reliably, execute at expected intervals, and properly handle success, failure, and recovery scenarios.
Changes
Add E2E test suite for monitoring daemon lifecycle
Verify daemon startup and initialization behavior
Validate scheduled execution at configured intervals
Verify task completion and execution status reporting
Test daemon behavior across multiple execution cycles
Validate persistence of execution metadata and timestamps
Verify recovery after transient failures
Add assertions for logging and monitoring outputs
Improve test utilities for daemon orchestration and timing validation
Test Scenarios
Execution Cycle Validation
Daemon starts successfully
Scheduled jobs execute at expected intervals
Consecutive execution cycles complete successfully
Execution timestamps are updated correctly
Metrics and status information are recorded accurately
Failure Handling
Temporary task failures do not stop the daemon
Failed executions are logged correctly
Retry and recovery mechanisms function as expected
Subsequent cycles continue after failure recovery
Shutdown & Restart
Graceful daemon shutdown
Restart resumes monitoring operations correctly
No duplicate executions after restart
Execution state remains consistent across restarts
Expected Outcomes
Monitoring daemon executes tasks on schedule
Execution history reflects completed cycles accurately
Failure scenarios are handled without service interruption
Monitoring infrastructure remains stable under extended operation
Motivation
The monitoring daemon is responsible for critical background operations. Verifying execution cycles through end-to-end testing helps ensure long-running reliability, prevents scheduling regressions, and provides confidence that monitoring processes continue operating correctly in production environments.
Checklist
Added E2E tests for daemon execution cycles
Verified scheduled execution timing
Added failure and recovery scenario coverage
Added restart and shutdown validation
Verified execution metadata and logging
Improved test utilities for daemon orchestration
Confirmed stable behavior across multiple execution cycles..closed #206