infralog

テストの期待値が間違っていると、実装の方が壊される

テスト設計

目次
  1. 落ちたテストは、実装を直す方向に修復されやすい
  2. 何を計算していたか
  3. 埋めた期待値が間違っていた
  4. 検出と、修復の方向
  5. 一つ見つけたら、全部を疑う
  6. テストが無い関数は、別種のリスク
  7. 検討したが採らなかった選択肢
  8. まとめ
  9. 関連記事

落ちたテストは、実装を直す方向に修復されやすい

仕様書にテストケースを書いた。期待値は手で計算して埋めた。その期待値が間違っていた。

怖いのは間違えたこと自体ではなく、修復の方向が2つあることだ。テストが落ちたとき、実装を期待値に合わせることもできるし、期待値を計算し直すこともできる。前者を選ぶと、テストは緑になり、中身は壊れたまま残る。

今回は期待値の方が誤りだと判定できた。その判定がどう成立したかを書く。

何を計算していたか

検索流入のデータを週次で集計する処理を書いていた。集計自体は単純だが、2箇所に落とし穴がある。

CTR は行ごとの平均ではなく、合計から再計算する。 クリック数と表示回数をそれぞれ合計してから割る。行ごとの CTR を平均すると、表示回数1回の行と1000回の行が同じ重みになり、実態と乖離する。

平均掲載順位は表示回数で重み付けする。 こちらも単純平均だと、ほとんど表示されていないクエリの50位が全体を押し下げる。

どちらも「単純にやると間違う」種類の計算なので、テストで固定する価値がある。そう考えて、仕様書に具体的な数値のテストケースを書いた。

埋めた期待値が間違っていた

データはこうした。

クリック表示回数掲載順位
520012.4
31508.1
05034.2

CTR は合計から 8 / 400 = 0.02。これは合っていた。行ごとの平均を取ると (0.025 + 0.02 + 0) / 3 = 0.015 になるので、この2つが違う値になるデータを選んだのは正しかった。同じ値になるデータでテストを書くと、実装が単純平均でも通ってしまう。

問題は加重平均の方で、仕様書にはこう書いていた。

// (12.4*200 + 8.1*150 + 34.2*50) / 400 = 13.585
expect(totals.position).toBeCloseTo(13.585, 3);

コメントに計算式まで書いてある。しかし実際に計算すると、

12.4 * 200 = 2480
 8.1 * 150 = 1215
34.2 *  50 = 1710
           = 5405
5405 / 400 = 13.5125

13.5125 が正しい。 13.585 がどこから出てきたのか、いまだに再現できない。式は正しく書いてあるのに答えだけ違う。桁の打ち間違いだと思うが、由来を説明できない誤りが一番たちが悪い。 同種の誤りが他にもある可能性を否定できないからだ。

検出と、修復の方向

実装を別の担当に渡す形で進めていた。仕様書のコードをそのまま写して実装し、テストを走らせると落ちる。

ここで実装側の式を期待値に合わせて書き換えていたら、加重平均が壊れた状態で「テストは通る」コードができていた。しかも式のコメントは正しいままなので、後から読んでも気づけない。

実際には、期待値の方を計算し直して訂正し、実装の式は仕様書のまま残すという対処が取られた。式が正しく、定数だけが誤っていたのだから、これが正しい。

判定の根拠は単純で、計算をもう一度やっただけだ。式は仕様書のコメントに書いてある。それを独立に実行すれば、どちらが正しいかは一意に決まる。「テストが落ちた」という事実だけでは方向は決まらないが、計算し直せる種類の期待値なら方向は決まる。

そのあと、仕様書側の数値も訂正した。ここを放置すると、コードと仕様書が恒久的に食い違い、次に読んだ人がまた仕様書を信じる。

一つ見つけたら、全部を疑う

この訂正のあと、レビューではテストに書いたすべての期待値を独立に再計算することにした。一つ手計算の誤りが出た以上、他の期待値を信用する理由がない。

合計値、合計から再計算した CTR、加重平均、並び順、空データの場合。全部を式から追い直して、他に誤りが無いことを確認した。結果として追加の誤りは無かったが、確認しないまま「たぶん他は合っている」で進めるのとは意味が違う。

テストが無い関数は、別種のリスク

同じコードに、テストが一つも無い関数があった。「直近の完了した週」を返す処理で、ISO 週番号の計算を含んでいる。仕様書がテストを用意していなかった。

期待値が間違っているテストは「緑なのに壊れている」状態を作るが、テストが無い関数は「そもそも何も言えない」状態になる。危険の質が違う。

こちらは境界になる日付を並べて手で確認した。全曜日、年をまたぐ週、ISO で53週ある年、閏日。年をまたぐ週が ISO-8601 どおり翌年の第1週になること、53週ある年で第53週が存在することまで見て、正しいと判断した。

ただし恒久的なテストは無いままだ。 次に誰かが触ったら、また無防備になる。これは「いま正しい」と「これから壊れない」が別物である、という話でもある。

検討したが採らなかった選択肢

期待値を手計算せず、実装の出力をそのまま期待値にする。 いわゆるゴールデンテスト。手計算の誤りは構造的に消えるが、実装が最初から間違っていた場合、その間違いが正解として固定される。 今回テストで守りたかったのは「単純平均にしていないこと」で、実装の出力を信じてしまうとその保証が消える。採らなかった。

期待値をテスト内で計算する。 expect(totals.position).toBeCloseTo(sum(p*i)/sum(i)) のように書けば手計算は要らない。ただしこれはテストが実装を写経するだけになり、式そのものが間違っていた場合に何も検出しない。定数で書くことに意味がある。

プロパティベーステストにする。 「加重平均は最小値と最大値の間に入る」のような性質なら自動生成で検査できる。単純平均との差を検出できる性質を設計できれば有効だが、今回は入力が3行の固定データで済む規模だったので、費用対効果で見送った。

まとめ

関連記事