Skip to content

fix: critical bugs + 39 tests for comprehensive coverage#3

Merged
muras3 merged 3 commits into
mainfrom
claude/test-quality-and-bugfix
Feb 27, 2026
Merged

fix: critical bugs + 39 tests for comprehensive coverage#3
muras3 merged 3 commits into
mainfrom
claude/test-quality-and-bugfix

Conversation

@muras3

@muras3 muras3 commented Feb 27, 2026

Copy link
Copy Markdown
Owner

Summary

ユーザーが2分の手動検証で probe url --json がJSON出力しないバグを発見。これを契機にテスト品質を根本から見直し、網羅的なテストを追加。

振り返りメモ (Future Claude向け)

何が起きたか

  • Wave 1-2のCodexレビューで「テストが通る」ことに満足し、実際にコマンドを叩く検証を一切しなかった
  • Go の flag パッケージはフラグが位置引数の前に来ないと無視される仕様で、probe url --json が機能しなかった
  • E2Eテストは全て --json URL (フラグ先) の順で書かれており、現実の使い方 (URL --json) をカバーしていなかった

根本原因

  1. テストが実使用パターンを反映していなかった — 開発者が書くテストは「API呼び出し」で、ユーザーが叩く「コマンドライン」ではなかった
  2. E2Eテストが16件中4件しかなかった — help, invalid target, 各exit code, フラグ組み合わせ, stdout/stderr分離 が全て欠落
  3. Codexレビューもテストの網羅性を指摘しなかった — テストの「存在」はチェックしたが「品質」はチェックしなかった

教訓 (次回のClaude向け)

  • コードレビュー後に必ず実バイナリを全パターンで叩け — テストが通ることと動くことは別
  • フラグ位置のテストは必須 — CLIツールでは cmd args --flagcmd --flag args の両方を必ずテスト
  • E2Eテストは「ユーザーが実際に打つコマンド」をベースに設計しろ — API単体テストの延長で作るな
  • テスト網羅性のギャップ分析を先にやれ — テスト追加前に「何が足りないか」を体系的に洗い出す

修正内容

バグ修正 (6件)

重大度 バグ 修正
CRITICAL probe url --json でJSON出力しない フラグを位置引数と分離してからパース
CRITICAL --insecure が expired cert をバイパスしない insecure=true 時に cert expiry を warn に降格
MEDIUM --help の出力が stderr stdout に変更
LOW --method "" がエラーにならない バリデーション追加
LOW 未知フラグで二重usage表示 Go default usage を抑制
LOW 空文字列ターゲットが受理される 拒否に修正

テスト追加 (39件)

カテゴリ 件数 主な追加内容
E2E 16 help, invalid target, JSON on error, table出力, stdout/stderr分離, 全フラグ, 全exit code (0/1/10/20/30/40), flag ordering回帰テスト, http/bare hostname
CLI 5 invalid header, -- separator, unknown flag, invalid timeout, empty target
Target parser 5 port境界値 (0/1/65535/65536/-1), fragment, bare host+port
Contract 4 error構造, RFC3339, duration型, observations全フィールド
Runner 3 http scheme, empty layers, wall clock
Layer 6 DNS multi-IP, TCP empty obs, HTTP 3xx/5xx, TLS generic error

Codexレビュー結果

  • Codex (gpt-5.3-codex) が10件指摘、うち対処可能な4件を修正:
    • skipIfUnreachable が常にport 443のみチェック → 443/80両方に
    • TestStdoutStderrSeparation が stderr を検証していない → 追加
    • 空文字列ターゲット受理 → 拒否に
    • TestHelpFlag の stderr 空検証 → 追加

Test plan

  • go vet — pass
  • make test-race — all unit tests pass (race detector)
  • make build — binary builds
  • make e2e — 19 PASS, 1 SKIP (exit code warn)
  • 実バイナリ打鍵テスト 80+パターン実行済み

🤖 Generated with Claude Code

muras3 and others added 3 commits February 27, 2026 12:40
Go's flag package stops parsing at the first non-flag argument, so
`probe example.com --json` silently ignored --json. Pre-process args
to separate flags from positional arguments before parsing.

Also fix --help to not show "Error: flag: help requested".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bugs fixed:
- CRITICAL: --insecure now bypasses expired certs (warn instead of fail)
- MEDIUM: --help output to stdout (not stderr)
- LOW: --method "" validation
- LOW: suppress Go flag double usage display

Tests added (39 new):
- 16 E2E tests (help, invalid target, JSON on error, table output,
  stdout/stderr separation, all flags, exit codes, flag ordering)
- 5 CLI tests (invalid header, --, unknown flag, bad timeout, errors)
- 5 target parser tests (port boundaries, fragment, bare host+port)
- 4 contract tests (error structure, RFC3339, duration type, observations)
- 3 runner tests (http scheme, empty layers, wall clock)
- 6 layer tests (DNS multi-IP, TCP empty obs, HTTP 3xx/5xx, TLS generic)

All tests pass (race detector + E2E).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix skipIfUnreachable to try both port 443 and 80 (was always 443)
- Add stderr verification to TestStdoutStderrSeparation
- Reject empty string target in CLI parser
- Add stderr emptiness check to TestHelpFlag

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@muras3
muras3 merged commit 1b83fea into main Feb 27, 2026
1 check failed
@muras3
muras3 deleted the claude/test-quality-and-bugfix branch March 3, 2026 02:31
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