test: bug fixes are tested at the helper, not at the wiring that broke (8 closed bugs)

未关闭
  • 哪里不对？
#750 0 条评论 0 个 reaction 已指派 1 人 在 GitHub 查看

维护者通常 1 天内回复

@bigcat88 已经在做这个了。

开始于 2026年8月21日。

评估

这个 Issue 还没有评估数据。

描述

audit:testing bug effort:medium

Pattern

An audit of all 74 closed-and-completed bugs in this repo found one shape repeating across at least eight of them:

The fix is split into a pure helper plus the wiring that calls it. The helper gets thorough tests. The wiring — the default, the call site, the flag — is the part that actually produced the user-visible bug, and it has no test.

Revert the wiring in each case below and the entire suite stays green while the original issue reproduces verbatim. These are not "no tests were written" bugs; every one of them shipped tests. They are tests aimed one layer below the defect.

This is filed as one issue rather than eight, because the fix is a review convention plus a handful of small tests, not eight separate investigations.

Instances

Each row: the fix that landed, the part that is covered, and the exact line whose reversion is invisible to CI.

Closed bug Covered Uncovered wiring — revert this and CI stays green
#322 CivitAI token broken check_civitai_url parsing, ~20 cases at tests/comfy_cli/command/models/test_models.py:142-235 comfy_cli/command/models/models.py:417 builds Authorization: Bearer on the foreground path and forwards it at :421/:433. Zero Bearer assertions exist under tests/comfy_cli/command/models/. The titled failure mode — the token not being sent — is untested.
#397 cu126 installed where cu130 needed resolve_cuda_wheel (tests/comfy_cli/test_cuda_detect.py:267-292) and install.execute(cuda_tag=…) comfy_cli/cmdline.py:383 _resolve_cuda, called at :610. git grep _resolve_cuda origin/main -- tests/ = 0 files (control: 1 prod file). Tests pass cuda_tag in directly, bypassing the resolver that was the defect.
#472 outdated ROCm ROCm URL construction, parametrized at tests/comfy_cli/test_install_python_resolution.py:283-293 The defaults at comfy_cli/cmdline.py:465, install.py:50, install.py:173. Every ROCm test passes rocm_version explicitly, so reverting all three defaults passes.
#311 default_launch_extras ignored The resolver, tests/comfy_cli/test_workspace_manager.py:326,340,385 comfy_cli/command/launch.py:428-434, which applies the extras. The only launch_extras hit in tests/ is test_config_manager.py:163, a config-read test.
#271 comfy node bisect broke parse_cm_output, 11 assertions at tests/comfy_cli/command/test_bisect_parse.py The call site comfy_cli/command/custom_nodes/bisect_custom_nodes.py:164. Reverting start() to the old inline comprehension leaves all 11 parser assertions green.
#205 set-default / --workspace The marker-fallback branch, tests/comfy_cli/test_workspace_manager.py:188 The git-success branch comfy_cli/workspace_manager.py:109 (return True, str(repo.working_dir)) — the other half of PR #401's return-type change. Only reachable via tests/e2e/test_e2e.py:83, which is skipped unless TEST_E2E=true.
#430 visual glitches + error The error half — markup metachars, exit code, no traceback The glitch half: transient=True at comfy_cli/ui.py:38 and comfy_cli/file_utils.py:349 has no assertion. (The 11 transient hits in tests/ are all the word "transient network" in oauth/cloud tests, not this flag.)
#403 cm-cli not found The FileNotFoundError arm, tests/comfy_cli/test_cmdline_python_resolution.py:37-40 The subprocess.CalledProcessError arm of the same except at comfy_cli/cmdline.py:691, and the sibling guard at install.py:315-318, which has no test at all.

Why this is worth a convention, not eight tickets

The repo's test suite is large and mostly good — 201 test files, ~11,181 assertions on origin/main. The failure is not effort or skill; it is that "did you add a test?" is satisfiable by testing the helper, and a helper test is easier to write than a wiring test. So the easy test displaces the load-bearing one.

Two cheap guards would catch most of it:

  1. Review checklist item: for a bug fix, name the line whose reversion the new test detects. If that line is not the one in the bug report's stack/repro, the test is aimed at the wrong layer.
  2. Prefer one call-site test over N helper tests. In several rows above the helper already had coverage before the fix; the fix added more helper cases.

Related, same family, different mechanism: #344 has no test on its branch at all (filed separately as #749), and #342's redactor is applied at one of several sinks (filed as #748).

Not claimed

  • No assertion that any of these has actually regressed today. Each was verified present on origin/main; the claim is only that a future reversion is invisible.
  • #264 was investigated and dismissed: its fix (a click<=8.1.8 pin, PR #265) was deliberately removed by bc0d87f when typer was bumped, and uv.lock pins click 8.1.8 / typer 0.21.1, so there is no live exposure. Recording it so nobody re-files it. The one residual: there is no root comfy --help smoke test anywhere in tests/ — every --help test targets a subcommand — and the root app was the surface #264 broke.

Filed by a closed-bug regression audit (Comfy-Org/comfy-cli, origin/main @ 3ff9f55, merge-base 62f35f2).

主要语言
Python
星标
1k
派生
176
平均合并
1 天 11 小时
30 天内合并 PR
87

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

Comfy-Org/comfy-cli 的其他 Issue

查看 Comfy-Org/comfy-cli 的全部 Issue

相似的 Issue

更多 Python Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。