Decouple assertion evaluation from runner

[christianhelle]

Summary

Decouple assertion evaluation from runner, following strict TDD (Red-Green-Refactor) with micro-commits.

Changes

Runner decoupling (already present on main)

• runner/executor.rs: Returns Vec::new() for assertion_results — no assertion logic
• runner/executor_async.rs: Same, no assertion evaluation
• runner/response_processor.rs\': build_temp_result_for_assertions` removed

Processor assertion evaluation (verified via TDD)

• processor/executor.rs — process_single_request evaluates assertions after executor call
• processor/incremental_loop.rs — process_requests_incremental evaluates assertions after executor call

New integration tests

• processor/executor_tests.rs: 3 new tests verifying processor evaluates assertions on raw executor results
• processor/incremental_tests.rs: 2 new tests verifying incremental path evaluates assertions

TDD Cycle

1. RED (8a86683): Removed assertion evaluation from processor — tests failed as expected
2. GREEN (b679ffb): Re-added assertion evaluation — all tests pass
3. Added comprehensive integration tests covering both sync and async paths

Testing

• All 1009 core tests pass
• All 12 GUI tests pass
• All runner tests (93) pass without assertion logic
• Zero warnings

Summary by CodeRabbit

Release Notes

• Tests
  • Added comprehensive unit tests to validate assertion evaluation for HTTP responses, including status code verification and failure propagation.
  • Verified that assertions are properly evaluated when executors return raw results without pre-evaluated assertions.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds five unit tests across executor_tests.rs and incremental_tests.rs verifying that the processor evaluates assertion directives when the executor returns a raw HttpResult with empty assertion_results, covering pass, failure propagation, and no-assertion cases. Also includes a one-character trailing-comma fix in the executor.rs match arm.

Changes

Processor Assertion Evaluation Tests

Layer / File(s)	Summary
Trailing comma fix in executor match arm src/core/src/processor/executor.rs	Adds trailing comma after the Ok(mut result) arm block in the process_single_request match expression.
Batch processor assertion evaluation tests src/core/src/processor/executor_tests.rs	Adds three tests: assertion population on a raw result passes, assertion failure propagates to result failure, and no evaluation occurs when no assertion directives are present.
Incremental processor assertion evaluation tests src/core/src/processor/incremental_tests.rs	Adds two tests: assertion population when executor returns empty assertion_results, and failure propagation when status mismatches (404 vs expected 200).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• christianhelle/httprunner#99: Modifies the same executor_tests.rs and exercises process_http_files_with_executor-driven HttpResult handling, which is the same infrastructure the new assertion evaluation tests build on.
• christianhelle/httprunner#197: Changes the HTTP executor's success flag to depend on assertion pass/fail, directly related to the assertion result population and failure propagation behavior tested in this PR.
• christianhelle/httprunner#279: Implements the unified process_requests_incremental loop that evaluates assertions after execution — the exact behavior exercised by the two new incremental tests in this PR.

Suggested labels

enhancement, Core

🐇 A comma so tiny, one small trailing flick,
Then five tests came hopping in, quite slick!
"Assert the status!" the rabbit did cheer,
404 fails, but 200 is clear.
No assertions? No worries — empty and neat,
The processor hops on with results complete! 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The PR title accurately summarizes the main architectural change: decoupling assertion evaluation from the runner to the processor layer, which is the core objective of implementing PRD-3.
Description check	✅ Passed	The PR description is well-structured with clear sections, provides specific details about changes, includes testing information, and follows the template structure.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 tdd/decouple-assertions-from-runner

---

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 and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.53%. Comparing base (d43c005) to head (48431fc).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #280      +/-   ##
==========================================
- Coverage   92.56%   92.53%   -0.04%     
==========================================
  Files          68       68              
  Lines        8663     8663              
==========================================
- Hits         8019     8016       -3     
- Misses        644      647       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

🧹 Nitpick comments (2)

src/core/src/processor/executor_tests.rs (1)

2108-2113: ⚡ Quick win

Strengthen these tests to prove success is recomputed, not preserved.

Both tests currently seed HttpResult.success with the expected final value, which can mask regressions. Seed the opposite value and assert the processor flips it based on assertion outcomes.

Suggested test hardening

@@ fn test_processor_evaluates_assertions_on_raw_executor_result() {
-        let mock = MockHttpExecutor::new(vec![HttpResult {
+        let mock = MockHttpExecutor::new(vec![HttpResult {
             request_name: None,
             status_code: 200,
-            success: true,
+            success: false, // should be overwritten to true by assertion evaluation
             error_message: None,
@@
         assert_eq!(http_result.assertion_results.len(), 1);
+        assert!(http_result.success, "processor should set success=true when assertions pass");
@@ fn test_processor_evaluates_assertions_and_marks_failure() {
         let mock = MockHttpExecutor::new(vec![HttpResult {
             request_name: None,
             status_code: 404,
-            success: false,
+            success: true, // should be overwritten to false by assertion evaluation
             error_message: None,

Also applies to: 2131-2133, 2167-2171, 2196-2199

🤖 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/core/src/processor/executor_tests.rs` around lines 2108 - 2113, The tests
are seeding HttpResult.success with the expected final value, which prevents
them from catching regressions where success is not being recomputed. In all
MockHttpExecutor instantiations (at the locations spanning 2108-2113, 2131-2133,
2167-2171, and 2196-2199), change the success field to have the opposite boolean
value from what the test expects to verify, then ensure the subsequent
assertions confirm the processor correctly recomputes and flips the success
value based on the assertion outcomes rather than preserving the initial mock
value.

src/core/src/processor/incremental_tests.rs (1)

461-465: ⚡ Quick win

Apply the same success-recompute hardening in incremental tests.

These tests also pre-seed success to expected outcomes. Invert the initial values so the test verifies processor recomputation rather than passthrough behavior.

Suggested test hardening

@@ fn test_incremental_evaluates_assertions_on_raw_executor_result() {
     let raw_result = HttpResult {
         request_name: None,
         status_code: 200,
-        success: true,
+        success: false, // should be recomputed to true
         error_message: None,
@@
     assert_eq!(http_result.assertion_results.len(), 1);
+    assert!(http_result.success, "processor should set success=true when assertions pass");
@@ fn test_incremental_evaluates_assertions_and_marks_failure() {
     let raw_result = HttpResult {
         request_name: None,
         status_code: 404,
-        success: false,
+        success: true, // should be recomputed to false
         error_message: None,

Also applies to: 490-498, 522-526, 551-554

🤖 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/core/src/processor/incremental_tests.rs` around lines 461 - 465, The test
cases in the incremental_tests.rs file are pre-seeding the success field with
expected outcomes, which means they are not actually verifying that the
processor recomputes this field. To properly harden these tests, invert the
initial boolean value of the success field in all HttpResult initializations
throughout the test cases (at lines around 461-465, 490-498, 522-526, and
551-554). Change true to false and false to true so that the tests verify the
processor correctly recomputes the success field to reach the expected outcome,
rather than just passing through the pre-seeded value.

🤖 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/core/src/processor/executor_tests.rs`:
- Around line 2108-2113: The tests are seeding HttpResult.success with the
expected final value, which prevents them from catching regressions where
success is not being recomputed. In all MockHttpExecutor instantiations (at the
locations spanning 2108-2113, 2131-2133, 2167-2171, and 2196-2199), change the
success field to have the opposite boolean value from what the test expects to
verify, then ensure the subsequent assertions confirm the processor correctly
recomputes and flips the success value based on the assertion outcomes rather
than preserving the initial mock value.

In `@src/core/src/processor/incremental_tests.rs`:
- Around line 461-465: The test cases in the incremental_tests.rs file are
pre-seeding the success field with expected outcomes, which means they are not
actually verifying that the processor recomputes this field. To properly harden
these tests, invert the initial boolean value of the success field in all
HttpResult initializations throughout the test cases (at lines around 461-465,
490-498, 522-526, and 551-554). Change true to false and false to true so that
the tests verify the processor correctly recomputes the success field to reach
the expected outcome, rather than just passing through the pre-seeded value.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0595327d-cf51-4c6a-9eb8-5104ff17d58a

📥 Commits

Reviewing files that changed from the base of the PR and between 23753cf and 48431fc.

📒 Files selected for processing (3)

• src/core/src/processor/executor.rs
• src/core/src/processor/executor_tests.rs
• src/core/src/processor/incremental_tests.rs