diff --git a/CLAUDE.md b/CLAUDE.md index a82ae12..3fa9940 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -99,61 +99,32 @@ Common Lispでは同じパッケージ内で同名関数を再定義すると警 ### 高優先度 -#### 出力ファイル名生成関数の引数統一 ✅ **完了** -出力ファイル名生成関数の引数を統一し、APIの一貫性を向上: +#### FFmpeg実行エラーハンドリング強化 +`ffmpeg.lisp`の`run-cmd`関数を例外ベース方式に改善し、堅牢性を向上させる。 -**実施内容:** -- `generate-gif-output-filename`の引数を`(input &optional opts)`から`(opts)`に変更 -- `main.lisp`のGIFモードでの呼び出し方法を修正 -- 関連するテストケースを更新 -- パッケージエクスポートの追加(`visp-options-output`, `safe-parse-float`) - -**結果:** -すべての出力ファイル名生成関数が`opts`を第一引数として受け取るAPIに統一され、 -テストスイートでの検証とバイナリビルドでの動作確認も完了。 - -#### GIFモードのオプション制限強化 ✅ **完了** -GIFモードで`--input`オプションとの組み合わせエラーが適切に処理されていない問題を修正: - -**問題:** -- `visp --input test.mp4 --gif --dry-run`でオプション解析が競合し、入力ファイルが空文字列になる -- GIFモードは`visp --gif `の構文のみを受け付けるべきだが、`--input`オプションが禁止リストに含まれていない - -**解決策:** -1. `validate-gif-mode`関数の禁止オプションリストに`visp-options-input`を追加 -2. GIFモードでは`--gif [--output ] [--dry-run]`の構文のみを許可 -3. 不正な構文でのエラーメッセージを改善 - -**影響範囲:** -- `src/validate.lisp`: GIFモードバリデーションの強化 -- `t/test-validate.lisp`: 関連テストケースの追加 - -#### 関数名重複問題の解決 -`util.lisp`と`validate.lisp`で類似のパース関数が重複している問題を統合する: - -**問題:** -- `util.lisp:3-10`: `parse-float`関数 -- `util.lisp:12-18`: `safe-parse-float`関数(parse-floatのラッパー) -- `validate.lisp:284-288`: `parse-speed-float`関数(独自実装) -- `video.lisp:28`: `safe-parse-float`を参照しているが定義場所が曖昧 - -**解決策:** -1. 共通のパース関数を`util.lisp`に統合 -2. エラーハンドリングを統一 -3. 全ての呼び出し箇所を統一された関数に修正 - -#### ffmpeg実行エラーハンドリング強化 -`ffmpeg.lisp:14`の`run-cmd`関数でエラーハンドリングが不十分: - -**現在の問題:** -- ffmpeg実行失敗時のエラー情報が不十分 +**現在の課題:** +- FFmpeg実行失敗時のエラー情報が不十分 - 出力ディレクトリの書き込み権限チェックなし -- 処理中断時の一時ファイル削除なし - -**改善策:** -1. ffmpegの詳細エラーメッセージ取得 -2. 事前の書き込み権限チェック -3. 処理中断時のクリーンアップ処理 +- 処理中断時の一時ファイル削除機能なし + +**改善内容:** +- `visp-ffmpeg-error`を使用した構造化エラーハンドリング +- FFmpegの詳細なエラーメッセージとコマンド情報の取得 +- 事前の書き込み権限チェック +- 処理中断時のクリーンアップ処理 + +#### 残りのvalidate関数の例外ベース化 +まだ`(uiop:quit 1)`を使用している関数を例外ベースに移行する。 + +**対象関数:** +- `validate-codec`: エンコーダー可用性チェック +- `validate-gif-mode`: GIFモード特有のバリデーション +- `validate-merge-files`: マージファイルの検証 +- `validate-output`: 出力パス検証 + +**作業内容:** +- 各関数を例外ベースに変更 +- 対応するエラーケーステストの追加 ### 中優先度 diff --git a/src/exceptions.lisp b/src/exceptions.lisp new file mode 100644 index 0000000..01eff2a --- /dev/null +++ b/src/exceptions.lisp @@ -0,0 +1,95 @@ +(in-package :visp) + +;; ベースとなる例外クラス +(define-condition visp-error (error) + ((message :initarg :message + :reader visp-error-message + :documentation "エラーメッセージ") + (context :initarg :context + :reader visp-error-context + :initform nil + :documentation "エラーが発生したコンテキスト(入力値など)")) + (:documentation "VISPアプリケーションのベースエラークラス") + (:report (lambda (condition stream) + (format stream "VISP Error: ~A" + (visp-error-message condition))))) + +;; バリデーション関連のエラー +(define-condition visp-validation-error (visp-error) + () + (:documentation "入力値やオプションのバリデーションエラー") + (:report (lambda (condition stream) + (format stream "Validation Error: ~A~@[ (context: ~A)~]" + (visp-error-message condition) + (visp-error-context condition))))) + +;; オプション関連のエラー +(define-condition visp-option-error (visp-validation-error) + ((option-name :initarg :option-name + :reader visp-option-error-option-name + :initform nil + :documentation "エラーが発生したオプション名")) + (:documentation "コマンドラインオプションのエラー") + (:report (lambda (condition stream) + (format stream "Option Error~@[ in --~A~]: ~A~@[ (value: ~A)~]" + (visp-option-error-option-name condition) + (visp-error-message condition) + (visp-error-context condition))))) + +;; ファイル関連のエラー +(define-condition visp-file-error (visp-validation-error) + ((file-path :initarg :file-path + :reader visp-file-error-file-path + :initform nil + :documentation "エラーが発生したファイルパス")) + (:documentation "ファイル操作のエラー") + (:report (lambda (condition stream) + (format stream "File Error~@[ (~A)~]: ~A" + (visp-file-error-file-path condition) + (visp-error-message condition))))) + +;; ffmpeg実行関連のエラー +(define-condition visp-ffmpeg-error (visp-error) + ((command :initarg :command + :reader visp-ffmpeg-error-command + :initform nil + :documentation "実行に失敗したffmpegコマンド") + (exit-code :initarg :exit-code + :reader visp-ffmpeg-error-exit-code + :initform nil + :documentation "ffmpegの終了コード")) + (:documentation "ffmpeg実行時のエラー") + (:report (lambda (condition stream) + (format stream "FFmpeg Error~@[ (exit code: ~A)~]: ~A" + (visp-ffmpeg-error-exit-code condition) + (visp-error-message condition))))) + +;; ヘルパー関数: 簡単に例外を発生させるためのユーティリティ + +(defun error-option (message &key option-name context) + "オプションエラーを発生させる" + (error 'visp-option-error + :message message + :option-name option-name + :context context)) + +(defun error-file (message &key file-path context) + "ファイルエラーを発生させる" + (error 'visp-file-error + :message message + :file-path file-path + :context context)) + +(defun error-validation (message &key context) + "バリデーションエラーを発生させる" + (error 'visp-validation-error + :message message + :context context)) + +(defun error-ffmpeg (message &key command exit-code context) + "ffmpegエラーを発生させる" + (error 'visp-ffmpeg-error + :message message + :command command + :exit-code exit-code + :context context)) \ No newline at end of file diff --git a/src/main.lisp b/src/main.lisp index 97bd135..390b674 100644 --- a/src/main.lisp +++ b/src/main.lisp @@ -1,51 +1,74 @@ (in-package :visp) (defun main (&optional args) - (unless (ffmpeg-available-p) - (format t "~a ffmpeg not found in your system. Please install ffmpeg first.~%" (log-tag "error")) - (uiop:quit 1)) - - ;; 引数がなければコマンドラインから取得 - (unless args (setf args (uiop:command-line-arguments))) - (setf args (clean-args args)) - - (when (member "--help" args :test #'string=) - (print-help) - (uiop:quit 0)) - - (let ((opts (parse-args-to-options args))) - (dispatch-validation opts) - - (cond - ;; 1. GIF モード - ((visp-options-gif opts) - (let* ((input (visp-options-input opts)) - (fps (get-video-fps input)) - (output (generate-gif-output-filename opts)) - (cmd (build-gif-cmd opts output fps))) - (run-cmd cmd output (visp-options-dry-run opts)))) - - ;; 2. 結合モード - ((visp-options-merge-files opts) - (let* ((output (generate-merge-output-filename opts)) - (cmd (build-merge-cmd opts output))) - (run-cmd cmd output (visp-options-dry-run opts)))) - - ;; 3. バッチモード - ((visp-options-batch-files opts) - (dolist (file (visp-options-batch-files opts)) - ;; 出力ファイル名やコマンド生成を通常モードと共通化するため、input を一時的に設定 - (setf (visp-options-input opts) file) - ;; 出力ファイル名生成 - (let* ((ext (getf (visp-options-codec-info opts) :ext)) - (filename (generate-output-filename opts ext)) ;; ファイル名だけ - (output (output-path-in-same-directory file filename)) ;; ディレクトリと結合してフルパス化 - (cmd (build-cmd opts output))) - (run-cmd cmd output (visp-options-dry-run opts))))) - - ;; 4. 通常処理 - (t - (let* ((ext (getf (visp-options-codec-info opts) :ext)) - (output (generate-output-filename opts ext)) - (cmd (build-cmd opts output))) - (run-cmd cmd output (visp-options-dry-run opts))))))) + (handler-case + (progn + (unless (ffmpeg-available-p) + (format t "~a ffmpeg not found in your system. Please install ffmpeg first.~%" (log-tag "error")) + (uiop:quit 1)) + + ;; 引数がなければコマンドラインから取得 + (unless args (setf args (uiop:command-line-arguments))) + (setf args (clean-args args)) + + (when (member "--help" args :test #'string=) + (print-help) + (uiop:quit 0)) + + (let ((opts (parse-args-to-options args))) + (dispatch-validation opts) + + (cond + ;; 1. GIF モード + ((visp-options-gif opts) + (let* ((input (visp-options-input opts)) + (fps (get-video-fps input)) + (output (generate-gif-output-filename opts)) + (cmd (build-gif-cmd opts output fps))) + (run-cmd cmd output (visp-options-dry-run opts)))) + + ;; 2. 結合モード + ((visp-options-merge-files opts) + (let* ((output (generate-merge-output-filename opts)) + (cmd (build-merge-cmd opts output))) + (run-cmd cmd output (visp-options-dry-run opts)))) + + ;; 3. バッチモード + ((visp-options-batch-files opts) + (dolist (file (visp-options-batch-files opts)) + ;; 出力ファイル名やコマンド生成を通常モードと共通化するため、input を一時的に設定 + (setf (visp-options-input opts) file) + ;; 出力ファイル名生成 + (let* ((ext (getf (visp-options-codec-info opts) :ext)) + (filename (generate-output-filename opts ext)) ;; ファイル名だけ + (output (output-path-in-same-directory file filename)) ;; ディレクトリと結合してフルパス化 + (cmd (build-cmd opts output))) + (run-cmd cmd output (visp-options-dry-run opts))))) + + ;; 4. 通常処理 + (t + (let* ((ext (getf (visp-options-codec-info opts) :ext)) + (output (generate-output-filename opts ext)) + (cmd (build-cmd opts output))) + (run-cmd cmd output (visp-options-dry-run opts))))))) + + ;; 例外ハンドリング: VISP固有のエラー + (visp-validation-error (e) + (format t "~a ~a~%" (log-tag "error") (visp-error-message e)) + (uiop:quit 1)) + + (visp-ffmpeg-error (e) + (format t "~a ~a~%" (log-tag "error") (visp-error-message e)) + (when (visp-ffmpeg-error-command e) + (format t "~a Failed command: ~{~a~^ ~}~%" (log-tag "info") (visp-ffmpeg-error-command e))) + (uiop:quit 1)) + + (visp-error (e) + (format t "~a ~a~%" (log-tag "error") (visp-error-message e)) + (uiop:quit 1)) + + ;; 予期しないエラー + (error (e) + (format t "~a Unexpected error: ~a~%" (log-tag "error") e) + (format t "~a Please report this issue with the command you ran.~%" (log-tag "info")) + (uiop:quit 1)))) \ No newline at end of file diff --git a/src/package.lisp b/src/package.lisp index 39364dc..7d6618f 100644 --- a/src/package.lisp +++ b/src/package.lisp @@ -16,6 +16,23 @@ ;; help.lisp :print-help + ;; exceptions.lisp + :visp-error + :visp-error-message + :visp-error-context + :visp-validation-error + :visp-option-error + :visp-option-error-option-name + :visp-file-error + :visp-file-error-file-path + :visp-ffmpeg-error + :visp-ffmpeg-error-command + :visp-ffmpeg-error-exit-code + :error-option + :error-file + :error-validation + :error-ffmpeg + ;; validate.lisp :validate-merge-files :validate-gif-mode diff --git a/src/validate.lisp b/src/validate.lisp index e2a0891..2d9739b 100644 --- a/src/validate.lisp +++ b/src/validate.lisp @@ -5,17 +5,15 @@ (let ((input (visp-options-input opts))) ;; 入力ファイルの存在チェック (unless (and input (not (string= input ""))) - (format t "~a --gif mode requires a video file. Use: visp --gif ~%" - (log-tag "error")) - (uiop:quit 1)) - + (error-option "--gif mode requires a video file. Use: visp --gif " + :option-name "gif")) ;; GIFモード対応拡張子チェック(全動画形式対応) (let ((ext (input-extension input))) (unless (member ext +allowed-input-extensions+ :test #'string-equal) - (format t "~a --gif mode supports video files (.mp4, .mov, .flv, .avi, .webm), but got '~a'.~%" - (log-tag "error") ext) - (uiop:quit 1))) + (error-file "--gif mode supports video files (.mp4, .mov, .flv, .avi, .webm)" + :file-path input + :context ext))) ;; すべての他オプションを禁止(--dry-runのみ特別に許可) (let ((disallowed-options (list @@ -34,9 +32,8 @@ (visp-options-speed opts) (visp-options-merge-files opts)))) (when (some #'identity disallowed-options) - (format t "~a --gif mode does not accept any other options. Use: visp --gif [--dry-run]~%" - (log-tag "error")) - (uiop:quit 1))))) + (error-option "--gif mode does not accept any other options. Use: visp --gif [--dry-run]" + :option-name "gif"))))) (defun validate-merge-files (opts) "Validate options specific to --merge mode." @@ -55,25 +52,27 @@ (visp-options-hflip opts) (visp-options-vflip opts) (visp-options-speed opts)) - (format t "~a --merge cannot be combined with other options.~%" (log-tag "error")) - (uiop:quit 1)) + (error-option "--merge cannot be combined with other options" + :option-name "merge")) ;; ファイル数チェック (unless (and files (>= (length files) 2)) - (format t "~a At least two .mp4 files must be specified for --merge.~%" (log-tag "error")) - (uiop:quit 1)) + (error-option "At least two .mp4 files must be specified for --merge" + :option-name "merge" + :context (if files (format nil "~a file(s) provided" (length files)) "no files"))) ;; 拡張子チェック (dolist (file files) (unless (string-equal (input-extension file) ".mp4") - (format t "~a Only .mp4 files are supported for --merge (got: ~a).~%" (log-tag "error") file) - (uiop:quit 1))) + (error-file "Only .mp4 files are supported for --merge" + :file-path file + :context (input-extension file)))) ;; ファイル存在確認 (dolist (file files) (unless (probe-file file) - (format t "~a Input file '~a' does not exist.~%" (log-tag "error") file) - (uiop:quit 1))) + (error-file "Input file does not exist" + :file-path file))) ;; 動画情報取得 (let* ((infos (mapcar #'get-video-info files))) @@ -83,16 +82,15 @@ (let ((file (car pair)) (info (cdr pair))) (unless (and (getf info :fps) (getf info :width) (getf info :height)) - (format t "~a Failed to extract fps/width/height from file: ~a~%" (log-tag "error") file) - (format t "~a This file may be corrupted or not a valid video.~%" (log-tag "error")) - (uiop:quit 1)))) + (error-file "Failed to extract fps/width/height from file. This file may be corrupted or not a valid video" + :file-path file)))) ;; 音声混在チェック (let ((has-audio-list (mapcar (lambda (info) (getf info :has-audio)) infos))) (unless (or (every #'identity has-audio-list) (every #'not has-audio-list)) - (format t "~a --merge does not support mixing audio and non-audio files.~%" (log-tag "error")) - (uiop:quit 1))) + (error-option "--merge does not support mixing audio and non-audio files" + :option-name "merge"))) ;; fps混在チェック(warn) (let ((fps-list (remove-duplicates (mapcar (lambda (info) (getf info :fps)) infos) @@ -119,16 +117,17 @@ (let ((input (visp-options-input opts))) ;; 入力の有無は必須 (unless input - (format t "Usage: visp --input [--res 4k] [--mute] ...~%") - (uiop:quit 1)) + (error-option "Input file or directory is required" + :option-name "input" + :context "Usage: visp --input [--res 4k] [--mute] ...")) ;; ディレクトリ指定っぽい場合 (if (uiop:directory-pathname-p input) (progn ;; ディレクトリが存在するか (unless (uiop:directory-exists-p input) - (format t "~a Directory '~a' does not exist.~%" (log-tag "error") input) - (uiop:quit 1)) + (error-file "Directory does not exist" + :file-path input)) ;; 対象ファイルの取得 (let ((files (remove-if-not @@ -136,8 +135,8 @@ (member (input-extension p) +allowed-input-extensions+ :test #'string-equal)) (uiop:directory-files input)))) (when (null files) - (format t "~a No valid video files found in directory '~a'.~%" (log-tag "error") input) - (uiop:quit 1)) + (error-file "No valid video files found in directory" + :file-path input)) ;; batch-files にファイル名 (string 型) を格納 (setf (visp-options-batch-files opts) (mapcar #'namestring files)) @@ -148,15 +147,15 @@ (progn ;; ファイル存在チェック (unless (probe-file input) - (format t "~a Input file '~a' does not exist.~%" (log-tag "error") input) - (uiop:quit 1)) + (error-file "Input file does not exist" + :file-path input)) ;; 拡張子チェック (let ((ext (input-extension input))) (unless (member ext +allowed-input-extensions+ :test #'string-equal) - (format t "~a visp does not support the input file extension '~a'.~%" - (log-tag "error") ext) - (uiop:quit 1))) + (error-file "Unsupported input file extension" + :file-path input + :context ext))) ;; 動画情報の表示 (print-video-info (get-video-info input)))))) @@ -170,16 +169,15 @@ ;; --reverse と --loop は併用不可 (when (and rev repeat) - (format t "~a --loop and --reverse options cannot be used together.~%" - (log-tag "error")) - (uiop:quit 1)) + (error-option "--loop and --reverse options cannot be used together" + :option-name "reverse")) ;; --reverse がサポートする拡張子は限定 (when rev (unless (member ext '(".mp4" ".mov") :test #'string-equal) - (format t "~a --reverse option only supports .mov or .mp4 extensions.~%" - (log-tag "error")) - (uiop:quit 1)) + (error-option "--reverse option only supports .mov or .mp4 extensions" + :option-name "reverse" + :context ext)) ;; メモリ使用に関する警告 (format t "~a --reverse may consume a large amount of memory. Consider using small input files.~%" @@ -198,9 +196,9 @@ (let ((repeati (parse-integer repeat :junk-allowed t))) ;; stream_loopの仕様上(repeat >= 1)がマスト (unless (and (integerp repeati) (>= repeati 1)) - (format t "~a --loop must be an integer >= 1, but got '~a'.~%" - (log-tag "error") repeat) - (uiop:quit 1)) + (error-option "--loop must be an integer >= 1" + :option-name "loop" + :context repeat)) ;; repeat は数値に変換して保存 (setf (visp-options-repeat opts) repeati))))) @@ -212,8 +210,8 @@ ;; --res と --half の併用は禁止 (when (and res half) - (format t "~a --res and --half cannot be used together.~%" (log-tag "error")) - (uiop:quit 1)) + (error-option "--res and --half cannot be used together" + :option-name "res")) ;; --res が指定されていれば有効な解像度か確認 (when res @@ -222,10 +220,9 @@ (parse-dimensions res)))) (if dims (setf (visp-options-scale opts) dims) - (progn - (format t "~a visp does not support the resolution '~a'.~%" - (log-tag "error") res) - (uiop:quit 1))))))) + (error-option "Unsupported resolution" + :option-name "res" + :context res)))))) (defun validate-half (opts) "If --half is specified, get original resolution using ffprobe and set scaled dimensions." @@ -235,8 +232,9 @@ (destructuring-bind (w . h) dims (setf (visp-options-scale opts) (cons (floor w 2) (floor h 2)))) - (format t "~a Could not determine resolution for --half. Original size will be used.~%" - (log-tag "warn")))))) + (error-option "Could not determine resolution for --half" + :option-name "half" + :context (visp-options-input opts)))))) (defun validate-fps (opts) "Validate that --fps is a positive integer if specified." @@ -245,9 +243,9 @@ (let ((fpsi (parse-integer fps :junk-allowed t))) ;; fpsは必ず0より大きい整数 (unless (and (integerp fpsi) (> fpsi 0)) - (format t "~a --fps must be a positive integer, but got '~a'.~%" - (log-tag "error") fps) - (uiop:quit 1)) + (error-option "--fps must be a positive integer" + :option-name "fps" + :context fps)) ;; 明示的に整数に変換して再セット (setf (visp-options-fps opts) fpsi))))) @@ -258,16 +256,15 @@ ;; 指定された codec が visp の対応リストにない場合 (when (and key (not codec-info)) - (format t "~a visp does not support the codec '~a'.~%" - (log-tag "error") key) - (uiop:quit 1)) + (error-option "visp does not support this codec" + :option-name "codec" + :context key)) ;; codec はあるが、システムに encoder が存在しない場合 (when (and codec-info (not (encoder-available-p (getf codec-info :encoder)))) - (format t "~a ffmpeg on this system does not support the encoder '~a'.~%" - (log-tag "error") (getf codec-info :encoder)) - (uiop:quit 1)) + (error-ffmpeg "ffmpeg on this system does not support this encoder" + :context (getf codec-info :encoder))) ;; hapとproresはpix_fmtを指定しないといけない(そしてそれは固定値) (when (getf codec-info :pix_fmt) @@ -283,7 +280,9 @@ (when (visp-options-mono opts) (let ((codec (visp-options-codec opts))) (when (member codec '("prores" "hap") :test #'string=) - (error "The --mono option is not supported with codec ~A." codec))))) + (error-option "The --mono option is not supported with this codec" + :option-name "mono" + :context codec))))) (defun validate-speed (opts) @@ -293,9 +292,9 @@ (let ((speedf (parse-number speed))) ;; speedは必ず0より大きい数値 (unless (and (numberp speedf) (> speedf 0)) - (format t "~a --speed must be a positive number, but got '~a'.~%" - (log-tag "error") speed) - (uiop:quit 1)) + (error-option "--speed must be a positive number" + :option-name "speed" + :context speed)) ;; 明示的に数値に変換して再セット (setf (visp-options-speed opts) speedf))))) @@ -306,9 +305,9 @@ ;; 出力ファイルのディレクトリが存在するかチェック (let ((parent-dir (uiop:pathname-directory-pathname (pathname output)))) (unless (uiop:directory-exists-p parent-dir) - (format t "~a Output directory '~a' does not exist.~%" - (log-tag "error") (namestring parent-dir)) - (uiop:quit 1)))))) + (error-file "Output directory does not exist" + :file-path (namestring parent-dir) + :context output)))))) (defun validate-options (opts) (validate-input opts) diff --git a/t/test-exceptions.lisp b/t/test-exceptions.lisp new file mode 100644 index 0000000..ca6bae9 --- /dev/null +++ b/t/test-exceptions.lisp @@ -0,0 +1,97 @@ +(defpackage :visp.test.exceptions + (:use :cl :rove) + (:import-from :visp + ;; 例外クラス + :visp-error + :visp-error-message + :visp-error-context + :visp-validation-error + :visp-option-error + :visp-option-error-option-name + :visp-file-error + :visp-file-error-file-path + :visp-ffmpeg-error + :visp-ffmpeg-error-command + :visp-ffmpeg-error-exit-code + ;; ヘルパー関数 + :error-option + :error-file + :error-validation + :error-ffmpeg)) + +(in-package :visp.test.exceptions) + +(deftest visp-error-basic-tests + (testing "visp-error basic functionality" + (let ((err (make-condition 'visp-error + :message "Test error" + :context "test-context"))) + (ok (string= (visp-error-message err) "Test error")) + (ok (string= (visp-error-context err) "test-context"))) + + (let ((err (make-condition 'visp-error :message "Simple error"))) + (ok (string= (visp-error-message err) "Simple error")) + (ok (null (visp-error-context err)))))) + +(deftest visp-option-error-tests + (testing "visp-option-error functionality" + (let ((err (make-condition 'visp-option-error + :message "Invalid value" + :option-name "speed" + :context "abc"))) + (ok (string= (visp-error-message err) "Invalid value")) + (ok (string= (visp-option-error-option-name err) "speed")) + (ok (string= (visp-error-context err) "abc"))))) + +(deftest visp-file-error-tests + (testing "visp-file-error functionality" + (let ((err (make-condition 'visp-file-error + :message "File not found" + :file-path "/path/to/file.mp4"))) + (ok (string= (visp-error-message err) "File not found")) + (ok (string= (visp-file-error-file-path err) "/path/to/file.mp4"))))) + +(deftest visp-ffmpeg-error-tests + (testing "visp-ffmpeg-error functionality" + (let ((err (make-condition 'visp-ffmpeg-error + :message "Command failed" + :command '("ffmpeg" "-i" "input.mp4") + :exit-code 1))) + (ok (string= (visp-error-message err) "Command failed")) + (ok (equal (visp-ffmpeg-error-command err) '("ffmpeg" "-i" "input.mp4"))) + (ok (= (visp-ffmpeg-error-exit-code err) 1))))) + +(deftest error-helper-functions-tests + (testing "error-option helper" + (ok (signals visp-option-error + (error-option "Invalid speed" :option-name "speed" :context "abc")))) + + (testing "error-file helper" + (ok (signals visp-file-error + (error-file "File not found" :file-path "/nonexistent.mp4")))) + + (testing "error-validation helper" + (ok (signals visp-validation-error + (error-validation "Invalid input" :context "test")))) + + (testing "error-ffmpeg helper" + (ok (signals visp-ffmpeg-error + (error-ffmpeg "Command failed" :command '("ffmpeg") :exit-code 1))))) + +(deftest exception-hierarchy-tests + (testing "Exception inheritance hierarchy" + ;; visp-validation-error is a subtype of visp-error + (ok (signals visp-error + (error-validation "test"))) + + ;; visp-option-error is a subtype of visp-validation-error + (ok (signals visp-validation-error + (error-option "test"))) + + ;; visp-file-error is a subtype of visp-validation-error + (ok (signals visp-validation-error + (error-file "test"))) + + ;; visp-ffmpeg-error is a subtype of visp-error + (ok (signals visp-error + (error-ffmpeg "test"))))) \ No newline at end of file diff --git a/t/test-validate.lisp b/t/test-validate.lisp index 6da6bb7..e2e8517 100644 --- a/t/test-validate.lisp +++ b/t/test-validate.lisp @@ -4,7 +4,15 @@ :make-visp-options :parse-number :validate-speed - :validate-gif-mode)) + :validate-input + :validate-reverse + :validate-repeat + :validate-resolution + :validate-half + :validate-fps + :validate-gif-mode + :visp-option-error + :visp-file-error)) (in-package :visp.test.validate) @@ -41,7 +49,24 @@ (let ((opts (make-visp-options))) (setf (visp:visp-options-speed opts) nil) (visp:validate-speed opts) - (ok (null (visp:visp-options-speed opts)))))) + (ok (null (visp:visp-options-speed opts))))) + + (testing "Throws visp-option-error for invalid speed values" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-speed opts) "invalid") + (ok (signals visp-option-error (validate-speed opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-speed opts) "-1.5") + (ok (signals visp-option-error (validate-speed opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-speed opts) "0") + (ok (signals visp-option-error (validate-speed opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-speed opts) "abc") + (ok (signals visp-option-error (validate-speed opts))))))) (deftest validate-output-tests (testing "Validates output option with existing directory" @@ -91,10 +116,35 @@ (visp:validate-gif-mode opts) (ok t))) - ;; NOTE: Error case tests have been temporarily removed due to (uiop:quit 1) - ;; incompatibility with test framework. These will be added back when - ;; validation functions are refactored to use exceptions instead of process exit. - ;; See CLAUDE.md "テストコード全体の改修とエラーケーステストの追加" for details. + (testing "Error when no input provided in GIF mode" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-gif opts) t) + ;; input is nil by default + (ok (signals visp-option-error (validate-gif-mode opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-gif opts) t) + (setf (visp:visp-options-input opts) "") + (ok (signals visp-option-error (validate-gif-mode opts))))) + + (testing "Error when unsupported file format in GIF mode" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-gif opts) t) + (setf (visp:visp-options-input opts) "test.txt") + (ok (signals visp-file-error (validate-gif-mode opts))))) + + (testing "Error when --gif used with other options" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-gif opts) t) + (setf (visp:visp-options-input opts) "test.mp4") + (setf (visp:visp-options-res opts) "hd") + (ok (signals visp-option-error (validate-gif-mode opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-gif opts) t) + (setf (visp:visp-options-input opts) "test.mp4") + (setf (visp:visp-options-codec opts) "h264") + (ok (signals visp-option-error (validate-gif-mode opts))))) (testing "Allows --dry-run with --gif" (let ((opts (make-visp-options))) @@ -103,4 +153,98 @@ (setf (visp:visp-options-dry-run opts) t) ;; エラーが発生しないことをテスト (visp:validate-gif-mode opts) - (ok t)))) \ No newline at end of file + (ok t)))) + +(deftest validate-input-error-tests + (testing "Error when no input provided" + (let ((opts (make-visp-options))) + ;; input is nil by default + (ok (signals visp-option-error (validate-input opts))))) + + (testing "Error when input file does not exist" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-input opts) "/nonexistent/file.mp4") + (ok (signals visp-file-error (validate-input opts))))) + + (testing "Error when input directory does not exist" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-input opts) "/nonexistent/directory/") + (ok (signals visp-file-error (validate-input opts)))))) + +(deftest validate-reverse-error-tests + (testing "Error when --reverse and --loop used together" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-rev opts) t) + (setf (visp:visp-options-repeat opts) 2) + (setf (visp:visp-options-input opts) "test.mp4") + (ok (signals visp-option-error (validate-reverse opts))))) + + (testing "Error when --reverse used with unsupported extension" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-rev opts) t) + (setf (visp:visp-options-input opts) "test.avi") + (ok (signals visp-option-error (validate-reverse opts)))))) + +(deftest validate-repeat-error-tests + (testing "Error when --loop is not a positive integer" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-repeat opts) "invalid") + (ok (signals visp-option-error (validate-repeat opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-repeat opts) "0") + (ok (signals visp-option-error (validate-repeat opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-repeat opts) "-1") + (ok (signals visp-option-error (validate-repeat opts)))))) + +(deftest validate-resolution-error-tests + (testing "Error when --res and --half used together" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-res opts) "fhd") + (setf (visp:visp-options-half opts) t) + (ok (signals visp-option-error (validate-resolution opts))))) + + (testing "Error when --res has unsupported value" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-res opts) "unsupported-resolution") + (ok (signals visp-option-error (validate-resolution opts)))))) + +(deftest validate-fps-error-tests + (testing "Error when --fps is not a positive integer" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-fps opts) "invalid") + (ok (signals visp-option-error (validate-fps opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-fps opts) "0") + (ok (signals visp-option-error (validate-fps opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-fps opts) "-30") + (ok (signals visp-option-error (validate-fps opts)))))) + +(deftest validate-output-error-tests + (testing "Error when output directory does not exist" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-output opts) "/nonexistent/directory/output.mp4") + (ok (signals visp-file-error (validate-output opts)))))) + +(deftest validate-codec-error-tests + (testing "Error when unsupported codec specified" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-codec opts) "unsupported-codec") + (ok (signals visp-option-error (validate-codec opts)))))) + +(deftest validate-mono-error-tests + (testing "Error when --mono used with unsupported codecs" + (let ((opts (make-visp-options))) + (setf (visp:visp-options-mono opts) t) + (setf (visp:visp-options-codec opts) "prores") + (ok (signals visp-option-error (validate-mono opts)))) + + (let ((opts (make-visp-options))) + (setf (visp:visp-options-mono opts) t) + (setf (visp:visp-options-codec opts) "hap") + (ok (signals visp-option-error (validate-mono opts)))))) \ No newline at end of file diff --git a/visp.asd b/visp.asd index fbbfaeb..201b739 100644 --- a/visp.asd +++ b/visp.asd @@ -7,21 +7,23 @@ :components ((:module "src" :components ((:file "package") - (:file "const") - (:file "log") - (:file "help") - (:file "options") - (:file "util") - (:file "video") - (:file "ffmpeg") - (:file "validate") - (:file "main"))))) + (:file "const" :depends-on ("package")) + (:file "log" :depends-on ("package")) + (:file "help" :depends-on ("package")) + (:file "exceptions" :depends-on ("package")) + (:file "options" :depends-on ("package")) + (:file "util" :depends-on ("package")) + (:file "video" :depends-on ("package" "util")) + (:file "ffmpeg" :depends-on ("package" "util" "video")) + (:file "validate" :depends-on ("package" "exceptions" "util" "video")) + (:file "main" :depends-on ("package" "exceptions" "options" "util" "video" "ffmpeg" "validate" "help" "log")))))) (defsystem "visp/test" :depends-on (:visp :rove) :components ((:module "t" :components - ((:file "test-ffmpeg") + ((:file "test-exceptions") + (:file "test-ffmpeg") (:file "test-util") (:file "test-util-output") (:file "test-validate"))))