Skip to content

Refs/heads/copilot/analyze test coverage#1

Merged
annawiewer merged 6 commits intomainfrom
copilot/analyze-test-coverage
Apr 7, 2026
Merged

Refs/heads/copilot/analyze test coverage#1
annawiewer merged 6 commits intomainfrom
copilot/analyze-test-coverage

Conversation

@annawiewer
Copy link
Copy Markdown
Owner

@annawiewer annawiewer commented Apr 7, 2026

  • Analyze existing test coverage (151 tests across test_components.py, test_advanced.py, etc.)
  • Add tests for scanner.py: matches_wunsch(), filter_relevant_changes(), AvailabilityChange, PortalSnapshot edge cases
  • Add tests for api.py: _classify_intent(), _deep_merge(), _snapshot_to_dict(), log persistence helpers, endpoint edge cases
  • Add tests for notify_tools.py: get_apprise() caching, legacy wrappers, should_notify_now() edge cases
  • Add tests for history.py: _prune(), empty stats, error counting, sorted stats
  • Add tests for config_model.py: address fields, config defaults, all-disabled edge cases
  • Address code review feedback: extract no_token_env fixture, use module-level import os, reduce duplication
  • Fix test_chat_keyword_fallback_scan: patch _scan_portals with AsyncMock so it is awaitable (prevents TypeError when the background task awaits it)
  • Fix _classify_intent in api.py: capture trailing_question flag before punctuation stripping so the trailing-?help heuristic actually fires; update test to assert help
  • All 110 new tests pass; 257 total tests pass (1 pre-existing failure unrelated to changes — requires Playwright browsers)

Copilot AI and others added 2 commits April 7, 2026 11:02
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new pytest suite focused on exercising previously uncovered branches across the backend (config models, scanner matching/filtering, history pruning/stats, notification helpers, and API helpers/endpoints).

Changes:

  • Introduces tests/test_coverage_gaps.py with targeted unit tests for config_model, scanner, history, and tools/notify_tools.
  • Adds direct tests for API internals (_classify_intent, _deep_merge, _snapshot_to_dict) and log persistence helpers.
  • Expands FastAPI endpoint coverage for chat and a few error/edge cases.

Comment on lines +1180 to +1190
resp = client.post("/api/scan/../../etc/passwd")
# Should be rejected by validation
assert resp.status_code in (400, 404)

def test_toggle_invalid_portal_name(self, client):
"""PUT /api/portals/<invalid>/toggle with special chars returns 400."""
resp = client.put(
"/api/portals/../../evil/toggle",
json={"enabled": True},
)
assert resp.status_code in (400, 404)
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

Similar to the scan test: /api/portals/../../evil/toggle likely won’t hit the route handler because the path includes extra segments, so you may only be testing routing (404) rather than the portal-name validation (400). Prefer a single-segment invalid portal name (e.g., evil!) and assert the specific validation status code.

Suggested change
resp = client.post("/api/scan/../../etc/passwd")
# Should be rejected by validation
assert resp.status_code in (400, 404)
def test_toggle_invalid_portal_name(self, client):
"""PUT /api/portals/<invalid>/toggle with special chars returns 400."""
resp = client.put(
"/api/portals/../../evil/toggle",
json={"enabled": True},
)
assert resp.status_code in (400, 404)
resp = client.post("/api/scan/evil!")
# Use a single path segment so the request reaches validation logic.
assert resp.status_code == 400
def test_toggle_invalid_portal_name(self, client):
"""PUT /api/portals/<invalid>/toggle with special chars returns 400."""
resp = client.put(
"/api/portals/evil!/toggle",
json={"enabled": True},
)
# Use a single path segment so the request reaches validation logic.
assert resp.status_code == 400

Copilot uses AI. Check for mistakes.
Comment on lines +1255 to +1258
with patch("wiesn_agent.api._scan_portals") as mock_scan:
mock_scan.return_value = []
resp = client.post("/api/chat", json={"message": "scan"})
assert resp.status_code == 200
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

_scan_portals is an async function, but it’s patched with a regular Mock and return_value = []. When the background _run_chat_scan task awaits it, this will raise a TypeError (awaiting a non-awaitable), potentially adding noisy “Scan failed …” entries and making the test less deterministic. Patch with AsyncMock(return_value=[]) (or patch _run_chat_scan / asyncio.create_task to avoid spawning the background task).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fixed in 11c3c5e. Changed the mock to AsyncMock(return_value=[]) so _run_chat_scan can await it without a TypeError. Also fixed _classify_intent in api.py to capture the trailing-? flag before stripping punctuation (the heuristic was dead code before), and updated the test to assert the now-correct help return value.

annawiewer and others added 3 commits April 7, 2026 13:40
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…classify_intent

Agent-Logs-Url: https://github.com/annawiewer/wiesn-agent/sessions/0f2afbc1-379e-47da-ae64-56df1a6721c7

Co-authored-by: annawiewer <138025423+annawiewer@users.noreply.github.com>
@annawiewer annawiewer merged commit c2a6cf8 into main Apr 7, 2026
3 of 6 checks passed
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.

3 participants