Skip to content

test: 全関数をカバーするテストを追加#26

Merged
ncaq merged 6 commits intomasterfrom
extend-test
Apr 18, 2026
Merged

test: 全関数をカバーするテストを追加#26
ncaq merged 6 commits intomasterfrom
extend-test

Conversation

@ncaq
Copy link
Copy Markdown
Owner

@ncaq ncaq commented Apr 18, 2026

未テストだったauto-sudoedit-sudoeditauto-sudoedit(hook本体)のテストを追加しました。
hookのテストでは以下を検証しています:

  • sudoパスへの変換
  • 同ユーザ時の無変更
  • nilパス時の安全性
  • auto-sudoedit-askによる確認ダイアログの承認/拒否
  • recentfの条件付きpop

モードのテストにはトグル復元とlighter表示の確認も追加しました。

未テストだった`auto-sudoedit-sudoedit`と`auto-sudoedit`(hook本体)のテストを追加しました。
hookのテストでは以下を検証しています:

- sudoパスへの変換
- 同ユーザ時の無変更
- nilパス時の安全性
- `auto-sudoedit-ask`による確認ダイアログの承認/拒否
- recentfの条件付きpop

モードのテストにはトグル復元とlighter表示の確認も追加しました。
@ncaq ncaq self-assigned this Apr 18, 2026
@ncaq ncaq added the Type: Testing Adding missing tests or correcting existing tests label Apr 18, 2026
Comment thread test/auto-sudoedit-test.el
Comment thread test/auto-sudoedit-test.el Outdated
Comment thread test/auto-sudoedit-test.el Outdated
Comment thread test/auto-sudoedit-test.el
@claude
Copy link
Copy Markdown

claude bot commented Apr 18, 2026

Code Review Summary

以下の観点でレビューを実施しました: コード品質、テストカバレッジ、ドキュメント正確性、パフォーマンス、セキュリティ、依存関係。

総合評価

質の高いテストの追加です。ERTのテストパターンに従い、cl-letfによるモックも適切に使用されています。テスト名は一貫した関数名/シナリオの命名規則に従っており可読性が高く、docstringも充実しています。with-temp-bufferによるバッファの分離、letによる変数のスコープ制限なども適切です。

Critical/Highの問題はありません。パフォーマンス・セキュリティ・依存関係についても問題なしです。

インラインコメントで指摘した項目 (Medium)

  1. diredバッファパスのテスト欠落 - auto-sudoeditフック関数のdired分岐がテストされていません
  2. trampファイル書き込み可能ケースのテスト欠落 - 既にtrampパスで書き込み可能な場合のガード条件が未検証です
  3. auto-sudoedit-mode/toggleテストの前提条件 - モードが既に有効な場合にテストが正しく動作しない可能性があります
  4. docstringの曖昧さ - auto-sudoedit/file-buffer-not-writableのdocstringが実際のテスト内容と微妙にずれています

その他の軽微な指摘 (Low)

  • set-visited-file-nameのモックシグネチャが実際の関数より引数が少ない(&optional _で2引数まで)。(lambda (path &rest _) ...)とする方がより堅牢です
  • revert-bufferがモックされているものの、実際に呼び出されたことの検証(should)がありません。ソースコードの(revert-buffer t t)はhookの重要な副作用です

🤖 Generated with Claude Code

Kyoseiレビューの指摘4点に対応しました:

- diredバッファのhookテストを追加
- trampファイルが書き込み可能な場合のスキップテストを追加
- `auto-sudoedit-mode/toggle`テストの堅牢性を改善
- docstringの曖昧な表現を修正
@ncaq
Copy link
Copy Markdown
Owner Author

ncaq commented Apr 18, 2026

set-visited-file-nameのモックシグネチャが実際の関数より引数が少ない(&optional _で2引数まで)。(lambda (path &rest _) ...)とする方がより堅牢です

そこはLisp的にはどうでも良さそうな気がする。

hookテストで`revert-buffer`をモックしていましたが、
実際に呼び出されたことの検証がありませんでした。
`revert-buffer`は確認なしでバッファを再読み込みする重要な副作用なので、
呼び出しと引数`(t t)`の両方を検証するようにしました。
Comment thread test/auto-sudoedit-test.el
Comment thread test/auto-sudoedit-test.el Outdated
Comment thread test/auto-sudoedit-test.el
@claude
Copy link
Copy Markdown

claude bot commented Apr 18, 2026

Kyosei Code Review (2nd round)

前回のレビュー指摘(diredテスト追加、tramp書き込み可能テスト追加、toggleテスト堅牢性、docstring修正、revert-buffer検証追加)が全てコミット876da71, fae674bで対応されていることを確認しました。

新規指摘 (Medium x3)

# 重大度 箇所 内容
1 Medium L272-283 trampパスが書き込み不可の場合にsudo再オープンされるテストが不足(現在は書き込み可能→スキップのみ)
2 Medium L298-305 dired-unadvertise/dired-advertiseの呼び出し検証がない
3 Medium L285-291 buffer-file-namedired-directoryの両方がnon-nilの場合のテストが存在しない

良い点

  • cl-letfによる外部依存のモック化が適切で、テストがファイルシステムの実際の状態に依存しない
  • テスト名が関数名/シナリオの規約で統一されており、テスト結果が読みやすい
  • auto-sudoedit-askの承認/拒否の両パスがカバーされている
  • revert-bufferの呼び出しと引数(t t)の検証が追加された(fae674b)
  • recentf-listのエッジケース(先頭要素が異なる場合)もカバーされている
  • unwind-protectによるモードテストの確実なクリーンアップ

セキュリティ/パフォーマンス

テストコード自体にセキュリティ上の問題やパフォーマンスの懸念はありません。全てのI/Oが適切にモック化されています。

総合判定

品質の高いテスト追加PRです。前回の指摘事項への対応も適切です。新規指摘3件はいずれもMediumで、テストカバレッジの更なる充実に関するものです。


Reviewed by Kyosei (code-quality, performance, security, test-coverage, documentation-accuracy)

- trampパスが書き込み不可の場合にsudo再オープンされるテストを追加
- diredテストで`dired-unadvertise`/`dired-advertise`の呼び出しを検証
@claude
Copy link
Copy Markdown

claude bot commented Apr 18, 2026

Kyosei Code Review (第3ラウンド)

前回の指摘事項(tramp書き込み不可テスト、dired-unadvertise/dired-advertise検証)が全て対応されていることを確認しました。

総合評価

非常に良質なテスト追加です。Critical/Highレベルの問題はありません。

良い点:

  • 全パブリック関数に対するテストが存在し、auto-sudoeditフック関数の主要な全分岐パスがカバーされている
  • cl-letfwith-temp-bufferによる適切なモック戦略とテスト分離
  • revert-buffer-args'(t t)と検証するなど、副作用の引数まで確認している
  • unwind-protectによるグローバル状態の確実なクリーンアップ
  • テスト命名規則(関数名/シナリオ)が既存テストと完全に統一されている
  • docstringが全テストで正確に記述されている

指摘:

  • (Low) auto-sudoedit/tramp-not-writable-reopensstring-match-pによる検証粒度 → インラインコメント参照

マージに支障はないと判断します。

Comment thread test/auto-sudoedit-test.el
Comment thread test/auto-sudoedit-test.el
@claude
Copy link
Copy Markdown

claude bot commented Apr 18, 2026

Kyosei Code Review (Round 4)

前回までのレビュー指摘に対する対応を確認しました。全体として非常に良い品質のテストコードです。

総評

  • セキュリティ: 問題なし。テストコードのみの変更で、本番コードへの影響はありません。
  • パフォーマンス: 問題なし。各テストは軽量なモック構成で、実I/Oは発生しません。
  • ドキュメント正確性: docstringは全てソースコードの実装と正確に一致しています。
  • コード品質: cl-letf/with-temp-buffer/unwind-protectの使い方が適切で、テスト間の副作用漏洩リスクはありません。
  • テストカバレッジ: 主要関数の重要なシナリオを網羅的にカバーしています。1件のカバレッジギャップをインラインコメントで指摘しました。

前回指摘の対応状況

指摘 状態
diredバッファテスト追加 ✅ 対応済み
tramp書き込み可能ケースのテスト ✅ 対応済み
tramp書き込み不可ケースのテスト ✅ 対応済み
dired-unadvertise/dired-advertiseの検証 ✅ 対応済み
revert-buffer呼び出し検証 ✅ 対応済み
toggleテストの堅牢性改善 ✅ 対応済み
docstringの曖昧さ修正 ✅ 対応済み

新規指摘 (1件)

  • Warning: auto-sudoedit-current-userのtrampパス分岐がテストされていない(インラインコメント参照)

🤖 Generated with Claude Code

ローカルパスのテストのみだったため、
trampパス時に`tramp-get-remote-uid`経由でリモートユーザー名を返す分岐のテストを追加しました。
Comment thread test/auto-sudoedit-test.el
@claude
Copy link
Copy Markdown

claude bot commented Apr 18, 2026

Kyosei Code Review (Round 5)

全サブエージェント(code-quality, test-coverage, performance, security, documentation-accuracy)によるレビューを実施しました。

総合評価

非常に高品質なテストPRです。4ラウンドのレビューを経て、主要な指摘は全て対応済みです。

良い点:

  • 全パブリック関数(auto-sudoedit-sudoedit, auto-sudoeditフック含む)にテストが揃い、カバレッジが大幅に向上
  • cl-letfによるモック、with-temp-bufferによるバッファ分離、unwind-protectによるクリーンアップが一貫して適切
  • 正常系・異常系(nilパス、同一ユーザー、ask確認/拒否、recentf条件分岐)が網羅的
  • docstringが全てソースコードの実装と正確に対応
  • テスト名が関数名/シナリオで統一されており可読性が高い

セキュリティ: 問題なし(テストのみの変更、副作用のあるI/Oは全てモック化済み)

パフォーマンス: 問題なし(実ファイルI/Oは最小限、全テストO(1))

新規指摘

インラインコメントで1件(Medium)の指摘を追加しました。非ファイル・非diredバッファ(shell-mode等)でのauto-sudoeditフックの分岐パスに関するテスト追加の提案です。

前ラウンドからの未対応指摘(参考)

  • auto-sudoedit/tramp-not-writable-reopensテストのstring-match-pによる部分一致検証 vs 他テストのequal完全一致の不統一(第3ラウンドで指摘済み)

🤖 Generated with Claude Code

@ncaq ncaq merged commit 7654129 into master Apr 18, 2026
5 checks passed
@ncaq ncaq deleted the extend-test branch April 18, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Testing Adding missing tests or correcting existing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant