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

Đang mở
  • Có gì không ổn?
#750 0 bình luận 0 reaction 1 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

@bigcat88 đang làm issue này rồi.

Từ ngày 21/8/2026.

Đánh giá

Issue này chưa được đánh giá.

Mô tả

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).

Ngôn ngữ chính
Python
Star
1k
Fork
176
Merge trung bình
1 ngày 11 giờ
Pull request đã merge (30 ngày)
87

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của Comfy-Org/comfy-cli

Tất cả issue của Comfy-Org/comfy-cli

Issue tương tự

Thêm issue về Python

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.