Skip to content

test: 現状を追認するテストを追加#24

Merged
ncaq merged 8 commits intomasterfrom
current-test
Apr 18, 2026
Merged

test: 現状を追認するテストを追加#24
ncaq merged 8 commits intomasterfrom
current-test

Conversation

@ncaq
Copy link
Copy Markdown
Owner

@ncaq ncaq commented Apr 18, 2026

修正を開始する前に、
現状の動作をなるべく壊さないように現状を追認するテストを簡単に追加しました。
あまり深いテストはまだ書いていません。
テスト用の簡単な開発基盤も構築しました。

ncaq added 5 commits April 18, 2026 14:28
`emacsWithPackagesFromPackageRequires`を使用することで、
`Package-Requires`ヘッダから依存パッケージを自動解決します。
これにより依存の二重管理を避けつつ、
`emacs -Q --batch`によるクリーンな環境でのテスト実行が可能になります。

`nix-fast-build`の`checks`としてERTテストが実行されるため、
既存のCIパイプラインに自然に統合されます。
今後の挙動修正で正しい動作を壊さないための回帰テストです。
パス変換ロジック、TRAMPパス処理、モードのフック管理など主要な関数をカバーしています。
ポート付きSSHパスのhop文字列組み立てバグは`:expected-result :failed`で記録しています。
elisp-autofmtはbatchモードで実行されるため`.editorconfig`が読まれず、
`fill-column`がデフォルトの70のままになっていました。
ファイル内変数で設定することでEmacsでの保存時と`nix fmt`のフォーマット結果を一致させます。
原形の単数形を使う命名規則に合わせます。
テストファイルにはpackage-lintが不適切な警告を大量に出すため、
本体は`melpa`チェック、テストファイルは`checkdoc`と`byte-compile`に分けています。
@ncaq ncaq self-assigned this Apr 18, 2026
@ncaq ncaq added Type: Refactoring Includes cleanup Type: CI Changes to CI configuration files and scripts Type: Testing Adding missing tests or correcting existing tests Type: Dependencies Dependency issues or Changes to dependency files labels Apr 18, 2026
Comment thread flake.nix Outdated
Comment thread flake.nix Outdated
Comment thread test/auto-sudoedit-test.el
@claude
Copy link
Copy Markdown

claude Bot commented Apr 18, 2026

コードレビュー結果

テストインフラの追加とERTテストの導入、良いPRです。テストはcl-letfによるモックが適切に使われており、主要な変換関数のカバレッジが網羅的です。既知のポートバグを:expected-result :failedで記録している点も良い実践です。

Critical

  • inherit (pkgs) emacsForIt ビルドエラー (flake.nix:102-103): emacsForItletバインディングで定義されたローカル変数であり、pkgsの属性ではありません。nix build .#emacsForIt実行時にattributeエラーが発生します。マージ前に修正が必要です。(インラインコメントに修正案あり)

Medium

  • テストカバレッジ: パッケージの中核であるauto-sudoeditフック関数(ファイルバッファ/diredバッファの切り替え、auto-sudoedit-askによる確認ダイアログ、recentf-list操作など)が未テストです。またauto-sudoedit-current-userのtrampパス分岐もカバーされていません。初回PRとしてのカバレッジは十分ですが、今後の拡充を推奨します。
  • nixpkgsの二重インスタンス化 (flake.nix:44-48): import nixpkgs { overlays = [...]; }によりflake-partsのpkgsとは別にnixpkgs全体が評価されます。emacs-overlayの副作用を分離する意図があれば合理的なトレードオフですが、Nixの遅延評価により実害は限定的です。

Low

  • nixpkgs-stableのfollows未設定 (flake.nix:9-11): emacs-overlayのトランジティブ依存であるnixpkgs-stablefollowsでピン留めされていません。(インラインコメントに詳細あり)
  • ポートバグコメントの具体性 (test/auto-sudoedit-test.el:85): :expected-result :failedのコメントにバグの具体的な内容を追記すると将来の修正時に有用です。(インラインコメントに詳細あり)

セキュリティ

セキュリティ上の問題は検出されませんでした。CIワークフローの権限設定(permissions: {}, persist-credentials: false)、アクションのコミットハッシュピン留めは適切です。


🤖 Generated with Claude Code

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

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コードレビュー (2回目)

前回のレビュー指摘のうち、以下が対応済みであることを確認しました:

  • inherit (pkgs) emacsForIt のビルドエラー → コミット 0e5c146 で修正済み
  • nixpkgs-stablefollows 未設定 → コミット d5ca23d で追加済み

全体評価

ERTテスト基盤の導入として良くまとまっています。emacsWithPackagesFromPackageRequiresによる依存自動解決、Nix checksへのERT統合、テストの網羅性いずれも適切です。セキュリティ上の問題も見つかりませんでした。

新規指摘 (前回未指摘分のみ)

[test-coverage] Medium: auto-sudoedit-sudoedit (インタラクティブコマンド)のテストが欠落しています。auto-sudoedit-pathの結果をfind-fileに渡す結合部分の検証がないため、find-fileをスタブ化して正しいパスが渡されることを確認するテストの追加を検討してください。

その他、インラインコメントでマルチホップテストの検証項目について1件指摘しています。

🤖 Generated with Claude Code

@ncaq ncaq merged commit 448018e into master Apr 18, 2026
3 checks passed
@ncaq ncaq deleted the current-test branch April 18, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: CI Changes to CI configuration files and scripts Type: Dependencies Dependency issues or Changes to dependency files Type: Refactoring Includes cleanup Type: Testing Adding missing tests or correcting existing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant