Skip to content

fix(bridge): add setup cleanup to audit_log_appends test#380

Merged
fagemx merged 1 commit intomainfrom
fix/373-audit-log-test-cleanup
Mar 27, 2026
Merged

fix(bridge): add setup cleanup to audit_log_appends test#380
fagemx merged 1 commit intomainfrom
fix/373-audit-log-test-cleanup

Conversation

@fagemx
Copy link
Copy Markdown
Owner

@fagemx fagemx commented Mar 27, 2026

Summary

  • Add fs::remove_file(audit_log_path(pid)) at the start of bg_detect::tests::audit_log_appends to ensure a clean slate before each run
  • Follows the same setup-cleanup pattern used by sibling tests (should_run_returns_true_when_never_run, increment_session_count_works)
  • Fixes consistent test failure where stale data from previous runs caused assert_eq!(content.lines().count(), 1) to fail (e.g., left: 14, right: 1)

Closes #373

Test plan

  • Run audit_log_appends twice in succession -- both pass
  • Run full edda-bridge-claude test suite (525 tests) -- all pass
  • Clippy clean

🤖 Generated with Claude Code

Add fs::remove_file at test start to ensure clean state, preventing
stale data accumulation from previous runs. Follows the same pattern
used by sibling tests in the same module.

Closes #373

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@fagemx
Copy link
Copy Markdown
Owner Author

fagemx commented Mar 27, 2026

Code Review: PR #380 (Round 1) — LGTM 🎉

All P0 and P1 issues have been resolved.

Summary

Single-line fix adds setup cleanup (fs::remove_file) to the audit_log_appends test in edda-bridge-claude/src/bg_detect.rs. This ensures the test doesn't fail when a stale audit log file exists from a previous test run (e.g., if the prior run crashed before teardown).

Why this is correct:

  • The test asserts exact line counts (lines().count() == 1), which breaks if stale data exists
  • This matches the established pattern in the same module — should_run_returns_true_when_never_run (line 863) and detect_state_persists_and_increments (line 949) both do identical setup cleanup
  • Using let _ = correctly discards the io::Result since the file may not exist on first run

Verdict: LGTM ✅

No critical or high-priority issues remaining. This PR is ready for merge.


Completed after 1 round(s) of automated review-fix loop

@fagemx fagemx merged commit 97fe585 into main Mar 27, 2026
7 checks passed
@fagemx fagemx deleted the fix/373-audit-log-test-cleanup branch March 27, 2026 06:11
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.

fix(bridge): audit_log_appends test fails due to missing setup cleanup

1 participant