Skip to content

test(check-pending-questions): 18 behavioral tests for parsing + gate fns - #1139

Closed
bassilkhilo-ag2 wants to merge 5 commits into
sonichi:mainfrom
bassilkhilo-ag2:test/check-pending-questions-behavioral
Closed

bassilkhilo-ag2 wants to merge 5 commits into
sonichi:mainfrom
bassilkhilo-ag2:test/check-pending-questions-behavioral

Conversation

@bassilkhilo-ag2

Copy link
Copy Markdown
Collaborator

Summary

  • Adds tests/check-pending-questions.test.py — first test coverage for src/check-pending-questions.py
  • Tests the four core functions in isolation; no notifications fired, no live workspace mutations

Tests (18/18 passing)

Function Tests What's covered
presenter_mode_active() T1–T4 absent sentinel, future-dated, expired, malformed content (fail-closed)
get_waiting_questions() T5–T10 missing file, unanswered included, answered excluded, mixed sections, legacy Q1 format, no-status-field skipped
should_notify() T11–T13 no file, recent file (cooldown), >1h-old file
voice_client_connected() T14–T18 no log, client=true, client=false, most-recent-line-wins, no [Health] line

Test plan

  • python3 tests/check-pending-questions.test.py → 18/18 PASS
  • Module-level path globals monkeypatched to tempdir fixtures
  • No macOS notifications fired, no Discord DMs sent during tests

🤖 Generated with Claude Code

bassilkhilo-ag2 and others added 5 commits May 26, 2026 04:28
Covers: missing results/ no-op, mtime cutoff (archive old, skip fresh),
non-.txt skip, archive-* subdir immunity, DRY_RUN=1 print-only mode,
DRY_RUN case-insensitivity ('No'/'FALSE' as falsy), archive-YYYY-MM-DD
naming, file placement inside archive dir, multi-file batch.

Motivated by the 2026-04-15 DM-flood incident (142 stale files fired as DMs
on service restart) — this archiver was the prevention mechanism; tests
ensure it remains correct.

12/12 tests passing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- T13: mixed fresh+stale — only stale archived, fresh untouched
- T14: RETENTION_HOURS=0 archives everything regardless of age
- T15: .sending files (orphan proactive-delivery claims) are not
       archived — only .txt suffix is in scope

Total: 15/15 passing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Companion to send-allowlist-shared.test.py (architectural invariants).
This suite exercises the runtime security properties that matter most:

- Symlink-traversal: /tmp/sutando-<link>→outside is rejected via realpath
- Path traversal: ../../../etc/passwd neutralised by realpath collapse
- Intra-root escape: symlink within results/→outside results/ rejected
- Fail-closed: nonexistent paths, directories, empty string all False
- Trailing-sep guard: results-extra/ not matched by results/ root
- Prefix correctness: /tmp/sutando-* and /tmp/echo-* accepted; others not

No discord.py or workspace mutation — all root-based tests use
monkeypatching of SEND_ALLOWED_ROOTS with tempdir fixtures.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
No existing test coverage for src/event_log.py — this is the structured
event-logging module imported by every Sutando service (bridges, health
check, codex path).

Tests guard:
- File creation on first call, JSONL append semantics (T4, T10, T15)
- Required fields: ts (float, ms-rounded), node (non-empty), kind (T5–T7, T14, T16)
- Extra keyword fields included; non-JSON values stringified via repr (T8–T9)
- Crash-safe: never raises even when LOGS_DIR is unwritable (T12)
- get_log_path() format and dated-file naming (T1–T2)
- LOGS_DIR resolves under $SUTANDO_WORKSPACE (T3)
- Empty kind string handled gracefully (T13)
- All lines are valid JSONL (T11)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… fns

No prior test coverage for src/check-pending-questions.py (cron script
that fires macOS + Discord DM notifications for unanswered questions).

Module-level path globals monkeypatched to tempdir fixtures — no live
workspace mutations, no macOS notifications fired, no Discord DMs sent.

Functions tested:
  presenter_mode_active() — sentinel file + ISO expiry timestamp (T1–T4)
    - no file → False, future-dated → True, expired → False,
      malformed (non-digit content) → False (fail-closed)
  get_waiting_questions() — pending-questions.md section parser (T5–T10)
    - missing file → [], unanswered → included, answered → excluded,
      mixed sections, legacy Q1 format, no-status-field skipped
  should_notify() — 1-hour cooldown guard (T11–T13)
    - no file → True, recent file → False, >1h-old file → True
  voice_client_connected() — [Health] log line reader (T14–T18)
    - no log → False, client=true → True, client=false → False,
      most-recent line wins, no [Health] line → False

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NON-BLOCKING — LGTM (stack on #1138)

Marginal addition: tests/check-pending-questions.test.py (+340, 18 tests for parsing + gate functions). Other test files in the diff are stack base. Scope-clean.

Deferring the merge call to Susan.

@VasiliyRad VasiliyRad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. All 18 check-pending-questions assertions verified against the source — presenter_mode_active fail-closed on malformed sentinel (T4), get_waiting_questions markdown parsing including legacy Q1 format and case-insensitive "waiting" match (T5-T10), should_notify 1h cooldown gate (T11-T13), and voice_client_connected last-[Health]-line-wins (T14-T18) all match the implementation.

Monkeypatching via _patch context manager + tempdir isolation is clean. No non-test files modified. The three other test files in the diff belong to the stack base (#1136-#1138).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants