Skip to content

test(core): add integration tests for agentd-core migrate subcommand#1133

Open
geoffjay wants to merge 2 commits intoissue-1125from
issue-1127
Open

test(core): add integration tests for agentd-core migrate subcommand#1133
geoffjay wants to merge 2 commits intoissue-1125from
issue-1127

Conversation

@geoffjay
Copy link
Copy Markdown
Owner

Adds crates/core/tests/migrate_cli.rs with 4 scenarios exercised via std::process::Command against isolated temp databases: up+status round-trip, status on empty DB, down+status rollback, and non-TTY down without --yes exits non-zero.

Closes #1127

@geoffjay geoffjay added the review-agent Used to invoke a review by an agent tracking this label label Apr 16, 2026
@geoffjay
Copy link
Copy Markdown
Owner Author

geoffjay commented Apr 16, 2026

Four scenarios exercised via std::process::Command against isolated
temp databases (--db-path flag added in #1125):

1. migrate up + status round-trip: verifies all migrations applied
2. status on empty database: verifies all migrations shown as pending
3. migrate down --yes + status: verifies latest migration rolled back
4. migrate down without --yes on non-TTY: verifies non-zero exit + hint

Closes #1127
Copy link
Copy Markdown
Owner Author

@geoffjay geoffjay left a comment

Choose a reason for hiding this comment

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

Review: test(core): add integration tests for agentd-core migrate subcommand

LGTM — cannot self-approve, posting as comment.

Stack note

This branch (issue-1127) is stacked on:

  • #1132 (issue-1125): feat(core): add clap CLI with migrate subcommand to agentd-core binarystill open
  • #1131 (issue-1124): feat(core): expose apply/status/rollback migration helpers from lib.rsstill open
  • #1130, #1129 below that

The entire upstack must merge before this lands. The tests depend on the migrate subcommand and --db-path flag introduced in #1132.

What was verified

  • Binary target: [[bin]] name = "agentd-core" is declared in Cargo.tomlenv!("CARGO_BIN_EXE_agentd-core") resolves correctly at test time. ✓
  • tempfile dev-dep: present at tempfile = "3.14" in [dev-dependencies]. ✓
  • --db-path flag: all three MigrateAction variants (Status, Up, Down) expose #[arg(long)] db_path: Option<PathBuf>. ✓
  • Output string assertions cross-checked against main.rs:
    • "up to date" / "applied" — emitted by run_migrate_up
    • "0 pending" — appears in the summary line "N applied, 0 pending"
    • "0 applied" (empty DB) — summary line is "0 applied, N pending"
    • "rolled back" — emitted as "✓ rolled back {migration}"
    • stderr.contains("--yes") || .contains("TTY") — the anyhow::bail! message is "stdin is not a TTY — pass --yes to confirm rollback without a prompt" — hits both branches ✓
  • Non-zero exit on missing TTY: anyhow::bail! propagates through main() -> Result<()> which prints to stderr and exits non-zero. ✓
  • Isolation: each test creates its own TempDir — no shared state between runs. ✓
  • Scope: single new file, no changes to production code. ✓

Non-blocking suggestions

  1. test_status_on_empty_database — the DB file does not exist before the call; the test succeeds because migration_status_for_path (via SeaORM) creates the file on first connection. Worth a one-line comment documenting this implicit side-effect so the intent is clear to future readers.

  2. Missing --all coverage: migrate down --all --yes is not exercised. A test verifying that all N migrations end up pending after --all would complete the matrix. Non-blocking for this PR.

  3. Idempotency: running migrate up twice and asserting the second run still exits zero with "0 pending" would guard against non-idempotent migration regressions. Nice-to-have.

@geoffjay geoffjay added merge-queue Approved by reviewer, queued for merge by conductor and removed review-agent Used to invoke a review by an agent tracking this label labels Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-queue Approved by reviewer, queued for merge by conductor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant