test: coverage improvements + doc updates#24
Conversation
Port TypeScript tests to Python for coverage improvement: - GChat format_converter: 37 tests covering to_ast, from_ast, render_postable, extract_plain_text (36% -> 80%+ target) - GChat workspace_events: 19 tests covering CRUD subscriptions, decode_pub_sub_message, auth token flows (47% -> 80%+ target) - Teams adapter: 42 tests covering fetch_messages, open_dm, _cache_user_context, _get_access_token, _get_graph_token, _validate_service_url, HTTP helpers (51% -> 70%+ target) - shared/buffer_utils: 19 tests covering to_buffer, to_buffer_sync, buffer_to_data_uri (27% -> 80%+ target) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Coverage: 79% → 82% overall. No modules below 60% (was 4). New tests: - test_gchat_format_extended.py: 29 GChat format converter tests (36%→72%) - test_gchat_workspace_events.py: workspace events unit tests (47%→53%) - test_teams_coverage.py: Teams adapter API + auth tests (51%→69%) - test_buffer_utils.py: buffer conversion tests (27%→100%) Docs: - TESTING.md: webhook fixture plan marked as completed - CHANGELOG.md: updated for 0.0.1a7 - Version bumped to 0.0.1a7 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request increments the SDK version to 0.0.1a7 and significantly expands test coverage across several core modules, including buffer utilities, Google Chat format conversion, Workspace Events, and the Teams adapter. It also updates documentation to reflect the completion of webhook JSON fixture integration. The review feedback identifies opportunities to improve test robustness by using pytest.raises instead of manual try-except blocks, adhering to PEP 8 by moving imports to the top level, and removing a redundant variable assignment in the test suite.
|
|
||
| import pytest | ||
|
|
||
| from chat_sdk.shared.buffer_utils import buffer_to_data_uri, to_buffer, to_buffer_sync |
There was a problem hiding this comment.
Import ValidationError at the top level to adhere to PEP 8 and avoid repetitive local imports throughout the test methods.
| from chat_sdk.shared.buffer_utils import buffer_to_data_uri, to_buffer, to_buffer_sync | |
| from chat_sdk.shared.buffer_utils import buffer_to_data_uri, to_buffer, to_buffer_sync | |
| from chat_sdk.shared.errors import ValidationError |
| from chat_sdk.shared.errors import ValidationError | ||
|
|
||
| try: | ||
| await to_buffer("invalid", "slack") | ||
| except ValidationError as e: | ||
| assert e.adapter == "slack" |
There was a problem hiding this comment.
The current try...except block will pass if to_buffer fails to raise a ValidationError. To ensure the exception is actually raised and to verify its attributes, use pytest.raises with the as excinfo pattern. The local import can be removed once moved to the top of the file.
with pytest.raises(ValidationError) as excinfo:
await to_buffer("invalid", "slack")
assert excinfo.value.adapter == "slack"| from chat_sdk.shared.errors import ValidationError | ||
|
|
||
| try: | ||
| to_buffer_sync("invalid", "teams") | ||
| except ValidationError as e: | ||
| assert e.adapter == "teams" |
There was a problem hiding this comment.
Similar to the async version, this try...except block should be replaced with pytest.raises to correctly verify that the exception is thrown and to inspect its attributes.
with pytest.raises(ValidationError) as excinfo:
to_buffer_sync("invalid", "teams")
assert excinfo.value.adapter == "teams"| tid = "teams:abc:def" | ||
| # Encode a proper thread ID |
There was a problem hiding this comment.
The initial assignment to tid is redundant as it is immediately overwritten on the following lines. This line can be removed to clean up the test code.
| tid = "teams:abc:def" | |
| # Encode a proper thread ID | |
| # Encode a proper thread ID |
1 participant
Coverage 79%→82%. No modules below 60%. Docs updated.