Hare/yoi
1
1
Fork 0
Code Actions Packages Releases Activity
develop
yoi/docs/report/test-validity-20260612/session-metrics.md

6.8 KiB
Raw Permalink Blame History

テスト妥当性レビュー: session-metrics

  • 評価: 概ね良い

確認範囲

  • 対象 crate: crates/session-metrics
  • 読んだ主なファイル:
    • crates/session-metrics/src/lib.rs
    • crates/session-metrics/README.md
    • 関連する実利用側テスト: crates/pod/tests/session_metrics_test.rs
  • crate の責務:
    • LogEntry::Extension { domain: "metrics" } 上に載せる軽量な metrics 型を定義する
    • Metric の serde 表現と builder helper を提供する
    • record_metric で session-store::save_extension に metrics domain として append する
    • RestoredState.extensions 相当の (domain, payload) 列から metrics domain の Metric だけを抽出する

現在のテストがよくカバーしていること

session-metrics 自体の unit test は 4 件で、薄い crate の主要な純粋ロジックはおおむね押さえています。

  • metric_round_trip_via_json
    • Metric::now(...)
    • with_dimension
    • with_value
    • with_correlation_id
    • JSON round-trip 後の等価性 を確認している。
  • metric_serializes_minimal_form_compactly
    • 空の dimensions、None の value / correlation_id が JSON に出ないことを確認している。
    • session log に載せる payload を不要に膨らませない invariant の確認として妥当。
  • fold_skips_other_domains
    • metrics_from_extensions が DOMAIN == "metrics" の entry だけを拾うことを確認している。
    • extension domain の分離という crate 境界に合っている。
  • fold_skips_undeserializable_payloads
    • metrics domain でも deserialize できない payload を落として継続することを確認している。
    • コメントにある「schema 変更で読めない payload は無視する」方針に対応している。

加えて、pod crate 側の crates/pod/tests/session_metrics_test.rs が実利用パスをかなりよく補っています。

  • prune metrics の skip / fire / post_request が session log に残り、metrics_from_extensions で復元できる。
  • correlation_id による prune.fire と prune.post_request の join 前提を確認している。
  • record_metric 経由の metrics write failure が本体 run を abort しないことを確認している。
  • metrics 以前の old session が clean に replay できることを確認している。

このため、crate 単体テストは小さいものの、利用側の integration coverage まで含めると現在の責務に対する妥当性は高めです。

不足 / 疑問のあるテスト

  • record_metric 自体の直接テストが session-metrics crate 内にありません。
    • 現状は pod 側 integration test で間接的に成功・失敗パスを踏んでいます。
    • ただし、この crate の public API としては record_metric が DOMAIN を正しく使い、StoreError をそのまま返し、restore 後に Metric として読めることを直接確認するテストがある方がよいです。
  • serde default の後方互換テストが片方向です。
    • 現在は「空/None の field を serialization で省略する」ことは確認しています。
    • しかし「省略された JSON を deserialize して dimensions = {}、value = None、correlation_id = None になる」ことは直接確認していません。
  • metrics_from_extensions の順序保持は実質的に確認されていますが、意図としてはもう少し明示してもよいです。
    • fold_skips_other_domains で a, b の順序は見ています。
    • これは十分とも言えますが、session-log の時系列を壊さないことが重要ならテスト名か assertion message で invariant として明記すると読みやすいです。
  • value: Option<f64> の非有限値の扱いは未定義に見えます。
    • serde_json は NaN / infinity の扱いが直感的でないため、metrics に非有限値が入る可能性を許すなら仕様化が必要です。
    • 現時点で producer が通常の token count / duration / savings だけなら優先度は低いです。
  • 実装詳細に寄りすぎた脆いテストは少ないです。
    • JSON の「field 名文字列を含まない」確認はやや表層的ですが、この crate では serde wire format 自体が API なので許容範囲です。
    • ただし厳密な JSON 文字列全体比較ではなく contains 否定に留めているため、過度には脆くありません。

追加の提案

優先度順に追加するなら以下です。

  1. record_metric_appends_metrics_extension_and_restores

    • FsStore か軽い fake Store を使う。
    • record_metric(...) を呼ぶ。
    • session_store::restore(...) 後に state.extensions から metrics_from_extensions(...) して、元の Metric が 1 件取れることを確認する。
    • これで DOMAIN、payload serde、session-store への append path を crate 側で直接守れる。

  2. record_metric_propagates_store_error

    • append が必ず失敗する minimal fake Store を置く。
    • record_metric が Err(StoreError) を返すことを確認する。
    • pod 側の「失敗しても run 継続」は pod の責務なので、ここでは error propagation だけで十分。

  3. metric_deserializes_minimal_legacy_payload

    • {"name":"x","ts":1} から Metric に deserialize する。
    • dimensions.is_empty()、value.is_none()、correlation_id.is_none() を確認する。
    • 現在の #[serde(default)] / optional field の後方互換意図を直接固定できる。

  4. 必要なら metric_non_finite_value_behavior_is_documented

    • 非有限値を禁止するなら constructor/helper 側で弾く設計にしてからテスト。
    • 許すなら JSON payload 上でどう表現されるかを明示する。
    • 現時点では必須ではなく、producer が非有限値を作る可能性が出た時点でよいです。

実行したコマンド

cd /home/hare/Projects/yoi && cargo test -p session-metrics

結果:

running 4 tests
test tests::fold_skips_other_domains ... ok
test tests::fold_skips_undeserializable_payloads ... ok
test tests::metric_round_trip_via_json ... ok
test tests::metric_serializes_minimal_form_compactly ... ok

test result: ok. 4 passed; 0 failed

cd /home/hare/Projects/yoi && cargo test -p pod --test session_metrics_test

結果:

running 5 tests
test old_sessions_without_metrics_replay_cleanly ... ok
test metric_write_failure_emits_warn_alert_and_does_not_abort_run ... ok
test prune_metrics_record_below_min_savings_skip ... ok
test prune_metrics_emit_skip_then_fire_with_post_request_join ... ok
test prune_metrics_fire_during_single_long_task_without_multiple_user_turns ... ok

test result: ok. 5 passed; 0 failed