Skip to content

test(core): add 26 edge case tests across core components - #427

Merged
frankbria merged 1 commit into
mainfrom
feature/issue-114-edge-case-testing
Mar 9, 2026
Merged

test(core): add 26 edge case tests across core components#427
frankbria merged 1 commit into
mainfrom
feature/issue-114-edge-case-testing

Conversation

@frankbria

@frankbria frankbria commented Mar 9, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #114

  • Adds 26 edge case tests across 4 core component areas, adapted from the original v1 plan to the current v2 architecture
  • Registers edge_case pytest marker for targeted selection (pytest -m edge_case)
  • All existing 1454 core tests continue to pass with zero regressions

Edge Cases by Component

Component Test File Tests Coverage
Agent Execution test_react_agent_edge_cases.py 7 Stall detector disabled states, timeout boundaries, activity reset, error propagation
Task Management test_tasks_edge_cases.py 6 Empty title, invalid transitions, missing tasks, empty workspace
Quality Gates test_gates_edge_cases.py 7 Empty repos, unknown gates, ruff parsing, status aggregation
Context Management test_context_edge_cases.py 6 Missing tasks, encoding fallbacks, token budget overflow, keyword extraction

Deviations from Original Plan

The original issue (#114) referenced v1 code (WorkerAgent, flash_save, Database class) that no longer exists. Tests were adapted to target the actual v2 core modules (ReactAgent/StallDetector, tasks, gates, context).

Test plan

  • uv run pytest -m edge_case -v — 26/26 passing
  • uv run pytest tests/core/ -q — 1454 passing, 0 regressions
  • uv run ruff check — clean

Summary by CodeRabbit

  • Tests
    • Added comprehensive edge-case test coverage for context management, quality gates, task management, and stall detection systems to improve reliability and ensure correct behavior under boundary conditions and error scenarios.

Add comprehensive edge case testing for boundary conditions and error
handling across 4 core component areas:

- Agent execution (7 tests): stall detector disabled states, timeout
  boundaries, activity reset, error propagation with edge inputs
- Task management (6 tests): empty title, invalid transitions, missing
  tasks, empty workspace queries
- Quality gates (7 tests): empty repos, unknown gates, ruff parsing,
  status aggregation, mixed statuses
- Context management (6 tests): missing tasks, encoding fallbacks,
  token budget overflow, keyword extraction, relevance scoring

Register `edge_case` pytest marker for targeted test selection.
@coderabbitai

coderabbitai Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR adds comprehensive edge case testing across four core components: context management, quality gates, stall detection, and task management. It introduces a new edge_case pytest marker and four new test modules with 99–140 lines each of boundary condition and error scenario tests.

Changes

Cohort / File(s) Summary
Pytest Configuration
pytest.ini
Added edge_case marker for tagging boundary condition and error handling tests.
Context Management Tests
tests/core/test_context_edge_cases.py
Added 7 edge case tests covering missing task IDs, UTF-8 encoding errors, token budget overflows, stopword filtering, and relevance calculation with empty keyword sets.
Quality Gates Tests
tests/core/test_gates_edge_cases.py
Added edge case tests for empty gate detection, unknown gate handling, Ruff error parsing, and GateResult summary behavior with mixed statuses.
Stall Detection Tests
tests/core/test_react_agent_edge_cases.py
Added edge case tests for disabled StallDetector (timeout ≤ 0), timer reset behavior, boundary conditions on timeout equality, and error object construction with extreme inputs.
Task Management Tests
tests/core/test_tasks_edge_cases.py
Added edge case tests for empty task titles, invalid state transitions, missing task retrieval, and empty workspace listing across all status filters.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Hoppy tests through edge case lands,
Boundaries bent by curious paws,
Where nulls and timeouts make their stands,
And errors fail by design's applause!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.47% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive The PR addresses several issue #114 objectives but adapts them to v2 architecture; coverage of database operations objectives is missing or unclear. Clarify whether database operations edge cases (concurrent writes, locks, invalid UTF-8, foreign keys, NULLs) were intentionally deferred or if task/gates/context tests provide equivalent coverage for the v2 architecture.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: adding 26 edge case tests across core components.
Out of Scope Changes check ✅ Passed All changes are directly related to adding edge case tests and registering the edge_case pytest marker as specified in issue #114.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/issue-114-edge-case-testing

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude

claude Bot commented Mar 9, 2026

Copy link
Copy Markdown

Code Review: Edge Case Tests (#427)

Good addition — the tests are well-structured, target real v2 modules, and the docstrings clearly explain intent. A few things worth addressing:


Issue 1: test_unknown_gate_auto_detected_skipped doesn't test production code

This test constructs a GateResult manually and checks that the dataclass can hold a SKIPPED check — it doesn't call run() at all:

def test_unknown_gate_auto_detected_skipped(self):
    check = GateCheck(name="hypothetical-unknown", status=GateStatus.SKIPPED, ...)
    result = GateResult(passed=True, checks=[check])
    assert result.passed is True

This would pass even if gates.py never produced SKIPPED for auto-detected unknown gates. To actually cover the gates_explicitly_provided=False path, you'd need to call run() with gates=None and an injected unknown gate name in the list returned by _detect_available_gates. For example, patching _detect_available_gates to return ["unknown-gate-xyz"] and calling run(workspace) without an explicit gates arg would exercise the real codepath.


Issue 2: test_stall_detector_records_activity_resets_timer — fragile call counting

The monkeypatch counts calls to time.monotonic() positionally (index 0–3) to return different values:

times = [base_time, base_time + 100.0, base_time + 100.0, base_time + 100.0]
return times[call_count - 1] if call_count <= len(times) else base_time + 100.0

This works today, but breaks silently if the implementation adds or removes any time.monotonic() call (e.g., for a log statement or an additional check). A time.sleep-free alternative is to set _last_activity directly after construction:

detector = StallDetector(timeout_s=10)
detector._last_activity -= 100  # force stalled state
assert detector.is_stalled() is True
detector.record_activity()
assert detector.is_stalled() is False

This is less brittle and still tests the same invariant.


Minor: test_gate_result_summary_mixed_statuses assertion is misleading

assert "errors" not in summary

This passes because no GateStatus.ERROR check is in the fixture, not because "errors" can't appear. The assertion tests that 0-count statuses are omitted from the summary (which is the actual invariant from the implementation). Renaming to something like assert "0 errors" not in summary or removing the assertion and adding a GateStatus.ERROR check would make intent clearer.


What's working well

  • The StallDetector tests correctly document the > timeout_s vs >= timeout_s boundary (test 6), and the disabled-state behavior for 0 and negative timeouts.
  • test_tokens_remaining_when_over_budget correctly verifies max(0, ...) clamping.
  • test_parse_ruff_errors_mixed_matching does a precise regression test on the regex parser — good.
  • State machine transition test (DONE → BACKLOG) correctly uses the actual InvalidTransitionError.current/.target attributes.
  • test_calculate_relevance_no_keywords pins the exact bonus values (0.1 + 0.12) which will catch scoring regressions.
  • The edge_case pytest marker registration in pytest.ini is clean.

Summary: Two actionable issues (the hollow auto-detect test and the fragile monkeypatch), one minor clarity issue. No regressions expected. Overall a solid first pass of edge case coverage.

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (2)
tests/core/test_context_edge_cases.py (1)

90-99: Consider documenting the magic numbers.

The assertion score >= 0.22 is derived from 0.1 + 0.12 (extension bonus + filename bonus). While the inline comment explains these values, they appear to be implementation details that could change.

This is acceptable for now, but if these bonus values are configurable or change frequently, consider importing them as constants from the context module.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/core/test_context_edge_cases.py` around lines 90 - 99, The test
test_calculate_relevance_no_keywords uses magic numbers (0.1 and 0.12) to assert
the expected minimum relevance for a FileInfo with extension ".py" and filename
"main.py"; update the test to reference the actual bonus constants (or
configuration) used by ContextLoader._calculate_relevance instead of hardcoding
values: import or access the extension bonus and filename bonus symbols from the
module that defines ContextLoader (or expose them on ContextLoader) and compute
expected_min = extension_bonus + filename_bonus so the test remains correct if
those values change.
tests/core/test_gates_edge_cases.py (1)

49-63: Consider testing auto-detection flow directly.

This test constructs a GateResult manually to verify the invariant that SKIPPED-only results pass. While this validates the data model, it doesn't exercise the actual auto-detection code path that would produce such a result.

Consider adding a test that exercises _detect_available_gates or run() in a scenario where unknown gates are encountered during auto-detection, to verify the end-to-end behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/core/test_gates_edge_cases.py` around lines 49 - 63, Replace the
manual-only GateResult test with (or add) an end-to-end unit test that exercises
the auto-detection path: invoke the component that performs detection (call
_detect_available_gates on the relevant runner/manager or call run() for a full
execution) in a scenario where an unknown gate name is present (e.g., supply a
fake/invalid gate in the input or plugin list), then assert the produced
GateResult shows passed==True and that the corresponding GateCheck has status
GateStatus.SKIPPED; target the _detect_available_gates and run methods in your
test so the actual auto-detection logic is exercised rather than constructing
GateResult by hand.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/core/test_context_edge_cases.py`:
- Around line 90-99: The test test_calculate_relevance_no_keywords uses magic
numbers (0.1 and 0.12) to assert the expected minimum relevance for a FileInfo
with extension ".py" and filename "main.py"; update the test to reference the
actual bonus constants (or configuration) used by
ContextLoader._calculate_relevance instead of hardcoding values: import or
access the extension bonus and filename bonus symbols from the module that
defines ContextLoader (or expose them on ContextLoader) and compute expected_min
= extension_bonus + filename_bonus so the test remains correct if those values
change.

In `@tests/core/test_gates_edge_cases.py`:
- Around line 49-63: Replace the manual-only GateResult test with (or add) an
end-to-end unit test that exercises the auto-detection path: invoke the
component that performs detection (call _detect_available_gates on the relevant
runner/manager or call run() for a full execution) in a scenario where an
unknown gate name is present (e.g., supply a fake/invalid gate in the input or
plugin list), then assert the produced GateResult shows passed==True and that
the corresponding GateCheck has status GateStatus.SKIPPED; target the
_detect_available_gates and run methods in your test so the actual
auto-detection logic is exercised rather than constructing GateResult by hand.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3936b5ea-50df-4563-aa8f-a1a855eda9c9

📥 Commits

Reviewing files that changed from the base of the PR and between 09ca25c and d63a3af.

📒 Files selected for processing (5)
  • pytest.ini
  • tests/core/test_context_edge_cases.py
  • tests/core/test_gates_edge_cases.py
  • tests/core/test_react_agent_edge_cases.py
  • tests/core/test_tasks_edge_cases.py

@frankbria
frankbria merged commit 3e20dec into main Mar 9, 2026
11 checks passed
@frankbria
frankbria deleted the feature/issue-114-edge-case-testing branch March 24, 2026 23:29
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.

[Phase 3→4] Add edge case testing across core components

1 participant