Skip to content

Unit tests for project_a2a - #2253

Merged
jaylfc merged 1 commit into
devfrom
exec/tsk-rctyui
Aug 3, 2026
Merged

Unit tests for project_a2a#2253
jaylfc merged 1 commit into
devfrom
exec/tsk-rctyui

Conversation

@jaylfc

@jaylfc jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): Unit tests for project_a2a

Autonomous build of board card tsk-rctyui.

Files:
tests/test_project_a2a.py | 573 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 573 insertions(+)

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jaylfc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 16 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 24f81981-b1ac-4646-87a8-a957bf688a02

📥 Commits

Reviewing files that changed from the base of the PR and between 0703152 and 101a902.

📒 Files selected for processing (2)
  • tests/test_project_a2a.py
  • tests/test_routes_torrent.py

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

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

@jaylfc

jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

Reviewed: purely additive new file, 30 unit tests over the project-A2A channel provisioning helpers - idempotency, member add/remove sync, lead resolution, duplicate archival, and the archived-only reprovision path all covered. Unit-level is right for these helpers per the card. Merging on green.

@jaylfc
jaylfc enabled auto-merge (squash) August 3, 2026 00:47
@gitar-bot

gitar-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add async unit tests for A2A channel provisioning and torrent settings routes

🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Add comprehensive tests for A2A channel creation, deduping, member sync, and lead sync.
• Validate backfill behavior across active vs archived projects, including config-based ID→name
 resolution.
• Add endpoint tests for /api/torrent/settings GET/PUT success and error/validation cases.
Diagram

sequenceDiagram
participant TA as "test_project_a2a"
participant A2A as "projects/a2a"
participant PS as "ProjectStore"
participant CS as "ChatChannelStore"
participant TT as "test_routes_torrent"
participant TR as "routes/torrent"
participant ST as "app.state store"

TA->>A2A: ensure_a2a_channel()/backfill_all()
A2A->>PS: get_project()/list_members()/list_projects()
A2A->>CS: list_channels()/create_channel()
A2A->>CS: add_member()/remove_member()/set_settings()

TT->>TR: GET/PUT /api/torrent/settings
TR->>ST: load()/save() torrent settings
TR-->>TT: 200 / 503 / 422 responses
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Mock stores instead of SQLite-backed stores
  • ➕ Faster execution and less I/O sensitivity
  • ➕ More direct unit isolation of diff logic (to_add/to_remove, leads_changed)
  • ➖ Less realistic coverage of store serialization/JSON field behavior
  • ➖ Risk of diverging from real store semantics (archived filtering, settings defaults)
2. Split into unit vs integration markers
  • ➕ Allows running fast unit tests by default while keeping end-to-end coverage
  • ➕ Clearer signal on failures caused by DB interactions/fixtures
  • ➖ Requires CI/test runner conventions for marker selection
  • ➖ Slightly more maintenance overhead

Recommendation: Current approach (real ProjectStore/ChatChannelStore with tmp_path SQLite DBs) is a good tradeoff because it validates the A2A invariants against real persistence behavior, which is where regressions are likely. If runtime becomes an issue, consider adding markers to optionally exclude the DB-backed cases or introducing a small set of mocked pure-unit tests for the resolution helpers.

Files changed (2) +654 / -0

Tests (2) +654 / -0
test_project_a2a.pyAdd end-to-end tests for A2A channel ensure/backfill logic +573/-0

Add end-to-end tests for A2A channel ensure/backfill logic

• Introduces async pytest coverage for internal resolution helpers (_build_agent_lookups, _resolve_member_names, _resolve_lead_names) and for ensure_a2a_channel/backfill_all behavior. Exercises edge cases including idempotency, duplicate channel archiving, member add/remove sync, lead syncing, archived project exclusion, and config-based ID→name conversions (including skipping deleted agents).

tests/test_project_a2a.py

test_routes_torrent.pyAdd API tests for torrent settings GET/PUT endpoints +81/-0

Add API tests for torrent settings GET/PUT endpoints

• Adds async endpoint tests validating 200 responses and response schema for GET /api/torrent/settings, plus PUT happy-path persistence semantics. Covers error handling when the settings store is missing (503) and request validation failures (422).

tests/test_routes_torrent.py

@jaylfc

jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

nemotron-ultra-kilo review

VERDICT: Two new test files added with good coverage but several issues: tight coupling to implementation details, missing error/edge case tests, and potential test interference via global state mutation.

  • test_project_a2a.py:12-14 - Helper _config() uses SimpleNamespace with agents attr; tests import internal functions (_build_agent_lookups, _resolve_member_names, etc.) directly rather than testing public API. This couples tests to private implementation.

  • test_project_a2a.py:44-47 - _build_agent_lookups_with_none_config asserts returns (None, None). If callers expect dicts (not None), this will cause AttributeError downstream. Should return ({}, {}) for safety or document None-handling contract.

  • test_project_a2a.py:102-108 - _resolve_member_names_without_config returns raw hex IDs as "names" (set of IDs). Function name implies name resolution; fallback behavior should be documented or return empty set to signal unresolved.

  • test_project_a2a.py:140-146 - _resolve_lead_names_without_config returns [lead_id] as-is. Same concern: caller expects names, gets IDs.

  • test_project_a2a.py:189-205 - test_find_a2a_channels_wrong_kind only tests settings.kind filter. Missing tests for wrong name (not A2A_NAME) and wrong type (not A2A_TYPE) — the function should filter on all three.

  • test_project_a2a.py - No tests for ensure_a2a_channel error paths: DB failures, constraint violations, concurrent duplicate creation race conditions.

  • test_project_a2a.py - No backfill_all tests without config parameter (only tested with config).

  • test_routes_torrent.py:10-15 - Tests reach into client._transport.app.state.download_manager._torrent_settings_store (private attr). Brittle; breaks on refactor. Should use public API or dedicated test fixture.

  • test_routes_torrent.py:10-15, 24-29 - monkeypatch.setattr(..., raising=False) on app.state mutates global app state across tests. Risk of test pollution; use fixture-scoped overrides instead.

  • test_routes_torrent.py:55-63 - test_put_torrent_settings_422_on_invalid_body only tests negative upload_rate_limit_kbps. Missing: negative max_active_seeds, non-bool seed_enabled, missing required fields, extra unknown fields.

  • test_routes_torrent.py - No test for partial PUT (subset of fields), no GET-after-PUT persistence verification, no auth/permission tests.
    VERDICT: Two new test files added with good coverage but several issues: tight coupling to implementation details, missing error/edge case tests, and potential test interference via global state mutation.

  • test_project_a2a.py:12-14 - Helper _config() uses SimpleNamespace with agents attr; tests import internal functions (_build_agent_lookups, _resolve_member_names, etc.) directly rather than testing public API. This couples tests to private implementation.

  • test_project_a2a.py:44-47 - _build_agent_lookups_with_none_config asserts returns (None, None). If callers expect dicts (not None), this will cause AttributeError downstream. Should return ({}, {}) for safety or document None-handling contract.

  • test_project_a2a.py:102-108 - _resolve_member_names_without_config returns raw hex IDs as "names" (set of IDs). Function name implies name resolution; fallback behavior should be documented or return empty set to signal unresolved.

  • test_project_a2a.py:140-146 - _resolve_lead_names_without_config returns [lead_id] as-is. Same concern: caller expects names, gets IDs.

  • test_project_a2a.py:189-205 - test_find_a2a_channels_wrong_kind only tests settings.kind filter. Missing tests for wrong name (not A2A_NAME) and wrong type (not A2A_TYPE) — the function should filter on all three.

  • test_project_a2a.py - No tests for ensure_a2a_channel error paths: DB failures, constraint violations, concurrent duplicate creation race conditions.

  • test_project_a2a.py - No backfill_all tests without config parameter (only tested with config).

  • test_routes_torrent.py:10-15 - Tests reach into client._transport.app.state.download_manager._torrent_settings_store (private attr). Brittle; breaks on refactor. Should use public API or dedicated test fixture.

  • test_routes_torrent.py:10-15, 24-29 - monkeypatch.setattr(..., raising=False) on app.state mutates global app state across tests. Risk of test pollution; use fixture-scoped overrides instead.

  • test_routes_torrent.py:55-63 - test_put_torrent_settings_422_on_invalid_body only tests negative upload_rate_limit_kbps. Missing: negative max_active_seeds, non-bool seed_enabled, missing required fields, extra unknown fields.

  • test_routes_torrent.py - No test for partial PUT (subset of fields), no GET-after-PUT persistence verification, no auth/permission tests.

Automated first-pass review by the nemotron-ultra-kilo lane. The lead still reviews before merge.

@jaylfc

jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

nemotron-super review

VERDICT: Pass
No blocking issues found.

Automated first-pass review by the nemotron-super lane. The lead still reviews before merge.

@jaylfc

jaylfc commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

nemotron-ultra-orB review

Based on the diff provided, here's my code review:


VERDICT: PASS with minor issues

  • tests/test_project_a2a.py:115 — _resolve_member_names test with deleted member uses ghost_id but member dict has member_id key, not id; the function likely expects member_id key based on other tests, so this is consistent
  • tests/test_project_a2a.py:277 — test_ensure_a2a_channel_archives_duplicate creates duplicate channel with members=[] but doesn't verify the canonical channel's members are preserved after archival
  • tests/test_project_a2a.py:354 — test_ensure_a2a_channel_lead_removed_drops_from_members assumes removing lead from project also removes from channel members, but the function should probably keep lead in members unless explicitly removed; this tests current behavior but may not match intent
  • tests/test_routes_torrent.py:12 — Test reaches into private _torrent_settings_store via download_manager._torrent_settings_store — fragile coupling to internal structure
  • tests/test_routes_torrent.py:27 — test_get_torrent_settings_503_when_store_missing monkeypatches torrent_settings_store to None but doesn't verify the actual endpoint error handling path (tests monkeypatch not code)
  • tests/test_routes_torrent.py:51 — test_put_torrent_settings_422_on_invalid_body only tests negative upload_rate_limit_kbps; missing validation tests for max_active_seeds (negative, zero, non-int), seed_enabled (non-bool)
  • tests/test_project_a2a.py — No tests for: concurrent ensure_a2a_channel calls, projects with many members (performance), config with duplicate agent names/IDs, backfill_all with archived projects that have A2A channels
  • tests/test_routes_torrent.py — No tests for: partial updates (PATCH semantics), unauthorized access, rate limiting, settings persistence across restarts

No blocking issues found.

Automated first-pass review by the nemotron-ultra-orB lane. The lead still reviews before merge.

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (1)

Context used
✅ Compliance rules (platform): 35 rules

Grey Divider


Remediation recommended

1. Unrelated test_routes_torrent.py added 📜 Skill insight ⚙ Maintainability
Description
This PR is titled and described as unit tests for project_a2a, but it also adds a separate route
test suite for tinyagentos/routes/torrent.py, bundling two concerns in one PR. Bundled changes
reduce reviewability and increase regression risk.
Code

tests/test_routes_torrent.py[R1-3]

+"""Endpoint tests for tinyagentos/routes/torrent.py."""
+
+from __future__ import annotations
Relevance

●● Moderate

No historical evidence enforcing “split unrelated changes into separate PRs” found; broad promo PRs
are common.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2185486 requires the diff to stay aligned with the PR’s stated purpose. The diff
adds a full new project_a2a test module and, separately, a new torrent routes test module,
indicating multiple concerns bundled into a single PR.

tests/test_project_a2a.py[1-16]
tests/test_routes_torrent.py[1-15]
Skill: taos-development-skill

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This PR bundles two unrelated test additions: `project_a2a` unit tests and `torrent` route endpoint tests.

## Issue Context
Compliance requires PRs to contain only changes related to the stated purpose, to keep reviews focused and reduce unintended side effects.

## Fix Focus Areas
- tests/test_routes_torrent.py[1-81]
- tests/test_project_a2a.py[1-573]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unused stores fixture setup 🐞 Bug ➹ Performance
Description
Several pure helper-function tests in tests/test_project_a2a.py request the async stores fixture
but never use it, so each test still opens/inits/closes two SQLite stores unnecessarily. This adds
avoidable overhead to the test suite and makes these unit-level tests depend on DB setup despite
only exercising synchronous helpers.
Code

tests/test_project_a2a.py[R39-42]

+@pytest.mark.asyncio
+async def test_build_agent_lookups_with_config(stores):
+    """_build_agent_lookups builds id->agent and name->agent dicts from config."""
+    from tinyagentos.projects.a2a import _build_agent_lookups
Relevance

●● Moderate

No direct precedent on unused fixtures; team does accept test/CI speed cleanups (PRs #388, #391).

PR-#388
PR-#391

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The stores fixture performs SQLite-backed store initialization/teardown, but multiple early tests
include stores in the signature and then never use it (they only call pure helper functions).

tests/test_project_a2a.py[28-37]
tests/test_project_a2a.py[39-78]
tests/test_project_a2a.py[126-177]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Some helper-function unit tests (`_build_agent_lookups`, `_resolve_member_names`, `_resolve_lead_names`) declare a `stores` fixture parameter but never reference it. Because `stores` initializes `ProjectStore` and `ChatChannelStore`, these tests pay unnecessary SQLite init/teardown cost and implicitly depend on DB availability.

### Issue Context
These tests only call synchronous helper functions and do not need `ProjectStore`/`ChatChannelStore`.

### Fix Focus Areas
- tests/test_project_a2a.py[28-37]
- tests/test_project_a2a.py[39-177]

### Suggested fix
- Remove the `stores` parameter from tests that don’t use it.
- Optionally remove `@pytest.mark.asyncio` and make those tests synchronous (they don’t `await`).
- Keep `stores` only for tests that actually unpack/use `(project_store, channel_store)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Missing trailing newline 🐞 Bug ⚙ Maintainability
Description
tests/test_project_a2a.py is added without a trailing newline at end-of-file. This is a minor
formatting defect that can cause style/lint noise and inconsistent diffs in future edits.
Code

tests/test_project_a2a.py[R571-573]

+    chans = await channel_store.list_channels(project_id=p["id"])
+    a2a = next(c for c in chans if (c.get("settings") or {}).get("kind") == A2A_KIND)
+    assert a2a["settings"]["leads"] == ["coord"]
Relevance

●●● Strong

Team commonly accepts small formatting/lint cleanups in reviews (e.g. doc code-block formatting
accepted in #1542).

PR-#1542

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The file ends at the final assertion with no additional line terminator (as indicated by the PR
diff).

tests/test_project_a2a.py[571-573]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new test file ends without a final newline.

### Issue Context
Git diff metadata flags the missing newline; many editors/formatters expect a POSIX-style trailing newline.

### Fix Focus Areas
- tests/test_project_a2a.py[571-573]

### Suggested fix
- Add a single newline after the last line in the file.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment on lines +1 to +3
"""Endpoint tests for tinyagentos/routes/torrent.py."""

from __future__ import annotations

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Unrelated test_routes_torrent.py added 📜 Skill insight ⚙ Maintainability

This PR is titled and described as unit tests for project_a2a, but it also adds a separate route
test suite for tinyagentos/routes/torrent.py, bundling two concerns in one PR. Bundled changes
reduce reviewability and increase regression risk.
Agent Prompt
## Issue description
This PR bundles two unrelated test additions: `project_a2a` unit tests and `torrent` route endpoint tests.

## Issue Context
Compliance requires PRs to contain only changes related to the stated purpose, to keep reviews focused and reduce unintended side effects.

## Fix Focus Areas
- tests/test_routes_torrent.py[1-81]
- tests/test_project_a2a.py[1-573]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread tests/test_project_a2a.py
Comment on lines +39 to +42
@pytest.mark.asyncio
async def test_build_agent_lookups_with_config(stores):
"""_build_agent_lookups builds id->agent and name->agent dicts from config."""
from tinyagentos.projects.a2a import _build_agent_lookups

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

2. Unused stores fixture setup 🐞 Bug ➹ Performance

Several pure helper-function tests in tests/test_project_a2a.py request the async stores fixture
but never use it, so each test still opens/inits/closes two SQLite stores unnecessarily. This adds
avoidable overhead to the test suite and makes these unit-level tests depend on DB setup despite
only exercising synchronous helpers.
Agent Prompt
### Issue description
Some helper-function unit tests (`_build_agent_lookups`, `_resolve_member_names`, `_resolve_lead_names`) declare a `stores` fixture parameter but never reference it. Because `stores` initializes `ProjectStore` and `ChatChannelStore`, these tests pay unnecessary SQLite init/teardown cost and implicitly depend on DB availability.

### Issue Context
These tests only call synchronous helper functions and do not need `ProjectStore`/`ChatChannelStore`.

### Fix Focus Areas
- tests/test_project_a2a.py[28-37]
- tests/test_project_a2a.py[39-177]

### Suggested fix
- Remove the `stores` parameter from tests that don’t use it.
- Optionally remove `@pytest.mark.asyncio` and make those tests synchronous (they don’t `await`).
- Keep `stores` only for tests that actually unpack/use `(project_store, channel_store)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread tests/test_project_a2a.py
Comment on lines +571 to +573
chans = await channel_store.list_channels(project_id=p["id"])
a2a = next(c for c in chans if (c.get("settings") or {}).get("kind") == A2A_KIND)
assert a2a["settings"]["leads"] == ["coord"] No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

3. Missing trailing newline 🐞 Bug ⚙ Maintainability

tests/test_project_a2a.py is added without a trailing newline at end-of-file. This is a minor
formatting defect that can cause style/lint noise and inconsistent diffs in future edits.
Agent Prompt
### Issue description
The new test file ends without a final newline.

### Issue Context
Git diff metadata flags the missing newline; many editors/formatters expect a POSIX-style trailing newline.

### Fix Focus Areas
- tests/test_project_a2a.py[571-573]

### Suggested fix
- Add a single newline after the last line in the file.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@jaylfc
jaylfc merged commit 3242887 into dev Aug 3, 2026
20 of 21 checks passed
@jaylfc
jaylfc deleted the exec/tsk-rctyui branch August 3, 2026 01:18
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.

1 participant