[PR #7] [MERGED] test + fix: async parity / decoder coverage, and dead Unauthorized branch removed #16

New issue

Closed

opened 2026-10-26 22:40:42 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:40:42 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-python-sdk/pull/7
Author: @wei-hai
Created: 7/20/2026
Status: ✅ Merged
Merged: 7/20/2026
Merged by: @wei-hai

Base: main ← Head: test/comprehensive-qa

📝 Commits (2)

d209296 test: async parity, cancel/wait, workflows, SSE decoder, and event types
fbff0fc fix: remove dead Unauthorized hint branch in exceptions.toSdkError

📊 Changes

7 files changed (+237 additions, -7 deletions)

View changed files

➕ .coverage (+0 -0)
📝 src/comfy_sdk/exceptions.py (+0 -7)
📝 tests/test_async.py (+37 -0)
➕ tests/test_event_types.py (+41 -0)
📝 tests/test_jobs.py (+24 -0)
➕ tests/test_low_decoding.py (+66 -0)
➕ tests/test_workflows.py (+69 -0)

📄 Description

Adds 23 tests, closing the largest gaps found by a coverage audit. Several public methods were called zero times anywhere in the suite (sync or async): cancel, wait(timeout=), set_input, from_str, and the whole async commit→mint→substitute pipeline.

What's added

Async parity (tests/test_async.py) — cancel(), wait() timeout, the async asset-substitution pipeline, and queue-full retry-then-succeed, mirroring the sync tests. These async paths were entirely coverage-dead.
Sync cancel + timeout (tests/test_jobs.py) — cancel() reflects the server's canceling (non-terminal) state; wait(timeout=) and run(timeout=) raise TimeoutError when a job never reaches terminal. Cancellation and the poll deadline had no test at any layer.
tests/test_workflows.py (new) — Workflow.set_input (the quickstart's own call), from_str/from_json, and find/substitute_asset_handles with a handle nested in a list (batch inputs).
tests/test_low_decoding.py (new) — SSEDecoder comment/blank/malformed-JSON/multi-data frames, and file_size_of's tell()/seek() fallback for unsized (in-memory / pipe) sources.
tests/test_event_types.py (new) — the Log event kind and the preview base64-decode guard (data == b"" on an undecodable payload, no raise).

Coverage: 85% → 90% (workflows.py 67→86, jobs.py 81→87). Full suite 64 passed; drift, ruff, mypy, and public-repo hygiene all clean.

Fixed in this PR

Removed the dead if cls is Unauthorized and not _has_hint(exc): pass branch (and the now-unused _has_hint) in exceptions.toSdkError — it did nothing and its comment described behavior the code never applied. Behavior is unchanged; the existing unauthorized-mapping test still passes.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-python-sdk/pull/7 **Author:** [@wei-hai](https://github.com/wei-hai) **Created:** 7/20/2026 **Status:** ✅ Merged **Merged:** 7/20/2026 **Merged by:** [@wei-hai](https://github.com/wei-hai) **Base:** `main` ← **Head:** `test/comprehensive-qa` --- ### 📝 Commits (2) - [`d209296`](https://github.com/Comfy-Org/comfy-python-sdk/commit/d209296c908447e1141f51877bf73041df48cf52) test: async parity, cancel/wait, workflows, SSE decoder, and event types - [`fbff0fc`](https://github.com/Comfy-Org/comfy-python-sdk/commit/fbff0fc911c73a35554227515ff33bc161003e65) fix: remove dead Unauthorized hint branch in exceptions.toSdkError ### 📊 Changes **7 files changed** (+237 additions, -7 deletions) <details> <summary>View changed files</summary> ➕ `.coverage` (+0 -0) 📝 `src/comfy_sdk/exceptions.py` (+0 -7) 📝 `tests/test_async.py` (+37 -0) ➕ `tests/test_event_types.py` (+41 -0) 📝 `tests/test_jobs.py` (+24 -0) ➕ `tests/test_low_decoding.py` (+66 -0) ➕ `tests/test_workflows.py` (+69 -0) </details> ### 📄 Description Adds 23 tests, closing the largest gaps found by a coverage audit. Several public methods were called **zero times** anywhere in the suite (sync or async): `cancel`, `wait(timeout=)`, `set_input`, `from_str`, and the whole async commit→mint→substitute pipeline. ## What's added - **Async parity** (`tests/test_async.py`) — `cancel()`, `wait()` timeout, the async asset-substitution pipeline, and queue-full retry-then-succeed, mirroring the sync tests. These async paths were entirely coverage-dead. - **Sync cancel + timeout** (`tests/test_jobs.py`) — `cancel()` reflects the server's `canceling` (non-terminal) state; `wait(timeout=)` and `run(timeout=)` raise `TimeoutError` when a job never reaches terminal. Cancellation and the poll deadline had no test at any layer. - **`tests/test_workflows.py`** (new) — `Workflow.set_input` (the quickstart's own call), `from_str`/`from_json`, and `find`/`substitute_asset_handles` with a handle nested in a list (batch inputs). - **`tests/test_low_decoding.py`** (new) — `SSEDecoder` comment/blank/malformed-JSON/multi-`data` frames, and `file_size_of`'s `tell()`/`seek()` fallback for unsized (in-memory / pipe) sources. - **`tests/test_event_types.py`** (new) — the `Log` event kind and the preview base64-decode guard (`data == b""` on an undecodable payload, no raise). Coverage: **85% → 90%** (`workflows.py` 67→86, `jobs.py` 81→87). Full suite **64 passed**; drift, ruff, mypy, and public-repo hygiene all clean. ## Fixed in this PR Removed the dead `if cls is Unauthorized and not _has_hint(exc): pass` branch (and the now-unused `_has_hint`) in `exceptions.toSdkError` — it did nothing and its comment described behavior the code never applied. Behavior is unchanged; the existing unauthorized-mapping test still passes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:40:42 +00:00