Fix test-framework failure accounting for runtime errors

[logbie]

Summary

Fixed two bugs in the wfl --test test-framework result accounting that caused runtime errors to be silently treated as passing tests and assertion failures to be double-reported in the failures list.

Key Changes

• Bug 1 — Runtime errors now count as failures: When a test body failed with a runtime error (e.g., undefined variable), the failure was recorded but failed_tests was never incremented. This caused the summary to show Failed: 0 and the process to exit 0, making crashing tests appear green to CI. Now runtime errors increment failed_tests alongside being recorded in the failures list.

• Bug 2 — Assertion failures no longer double-reported: The guard to skip already-recorded assertion failures compared the Display string (which is prefixed with "Runtime error at line ...:") against "Assertion failed:", which never matched. Now the guard inspects the raw RuntimeError::message field, which correctly begins with "Assertion failed:" for assertion failures, preventing duplicate entries.

Implementation Details

In src/interpreter/mod.rs, the TestBlock error handler now:

1. Inspects e.message (the raw error message) instead of e.to_string() (the Display form with line/column prefix) to correctly identify assertion failures
2. Increments results.failed_tests when recording non-assertion runtime errors, ensuring the failure count is accurate
3. Preserves the full Display string in the failure record so line/column information is still visible to users

Testing

Added comprehensive regression tests in tests/test_framework_counting_test.rs:

• Runtime errors are counted as failures and appear exactly once
• Failing assertions are recorded exactly once (not duplicated)
• Mixed test suites maintain the invariant total == passed + failed
• Short-circuiting on first assertion failure is preserved

All existing .test.wfl programs and test-framework validation continue to pass.

https://claude.ai/code/session_01EcnZgma17a36Wc7VazYWG2

---

Summary by CodeRabbit

• Bug Fixes

  • Corrected test-mode failure counts so runtime errors are included in failed test totals.
  • Test runs now return a failure status when runtime errors occur.
  • Prevented failed assertions from being reported more than once.
  • Test execution now stops the current test after its first failure while continuing with the remaining tests.
  • Improved consistency between total, passed, and failed test counts.

• Documentation

  • Added documentation describing test failure accounting and reporting behavior.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

📝 Walkthrough

Walkthrough

Test mode now counts runtime errors as failed tests, avoids duplicating assertion failures, stops failed test bodies, and adds regression tests and documentation for the resulting accounting behavior.

Changes

Test mode failure accounting

Layer / File(s)	Summary
TestBlock error accounting src/interpreter/mod.rs	Assertion detection uses RuntimeError.message; other runtime errors are recorded, increment failed_tests, and stop the current test body.
Regression coverage and documentation tests/test_framework_counting_test.rs, Dev diary/2026-07-10-test-mode-failure-accounting.md	Tests cover runtime failures, single assertion reporting, mixed totals, and multiple assertions; the dev diary documents the fixes.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related PRs

• WebFirstLanguage/wfl#273: Introduced the test-mode execution path targeted by this accounting fix.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately reflects the main runtime-error accounting fix in the test framework.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/wfl-test-verification-kb0iod

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[coderabbitai]

🧹 Nitpick comments (1)

src/interpreter/mod.rs (1)

7126-7138: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Track assertion failures by kind, not by message prefix. RuntimeError already has ErrorKind; tagging ExpectStatement failures as ErrorKind::AssertionFailure and checking e.kind here would avoid relying on "Assertion failed:" and remove the collision risk.

🤖 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 `@src/interpreter/mod.rs` around lines 7126 - 7138, Replace the message-prefix
check in the test failure handling block with an ErrorKind-based check: ensure
ExpectStatement-generated RuntimeErrors are tagged as
ErrorKind::AssertionFailure, then use e.kind to exclude assertion failures in
this logic. Update the relevant RuntimeError construction and the conditional
near TestFailure creation, preserving existing failure tracking behavior for
other error kinds.

🤖 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.

Nitpick comments:
In `@src/interpreter/mod.rs`:
- Around line 7126-7138: Replace the message-prefix check in the test failure
handling block with an ErrorKind-based check: ensure ExpectStatement-generated
RuntimeErrors are tagged as ErrorKind::AssertionFailure, then use e.kind to
exclude assertion failures in this logic. Update the relevant RuntimeError
construction and the conditional near TestFailure creation, preserving existing
failure tracking behavior for other error kinds.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: de1cc4c5-a3e2-450f-90c8-b2732634647d

📥 Commits

Reviewing files that changed from the base of the PR and between e2df05f and 679faa9.

📒 Files selected for processing (3)

• Dev diary/2026-07-10-test-mode-failure-accounting.md
• src/interpreter/mod.rs
• tests/test_framework_counting_test.rs