どうせレビューで落ちるPRに、テストもE2Eも全部流していた話

当ページのリンクには広告が含まれています。
この記事の結論
CIの待ち時間を削るなら、キャッシュや高速化より先に「どの順番で走らせるか」「そもそも走らせるか」を見直したほうが効くと考えた話です。自動レビューを一番前に置き、指摘が出たPRではテストもビルドもE2Eも走らせない構成に組み替えました。ただし順序を変えたことで、CIは成功しているのに本番へ出ないという別の事故も起こしています。

お疲れ様です!IT業界で働くアライグマです!

個人開発している開発関連ニュースのキュレーションサービス「DevPick」のCIを、2026年9月6日に組み替えました。ジョブを速くしたのではなく、走らせる順番と条件を変えただけです。今回は、その判断の理由と、変えた直後に踏んだ落とし穴について書きます。

目次

ランナー2台が、ずっと順番待ちをしていた

DevPickのCIは、GitHubが用意しているランナーではなく、自分の開発機2台をセルフホストランナーとして登録して回しています。クラウドの従量課金は発生しませんが、その代わり同時に走れるジョブは最大2つです。3つ目以降は順番待ちになります。

そこにジョブが並んでいました。PRを出すと、バックエンドのテスト、フロントエンドのビルド、E2E、そして自動レビューが動きます。このうち自動レビューだけは別のワークフローファイルに書かれていて、テストやビルドとは完全に独立して起動していました。独立しているということは、お互いの結果を待たないということです。PRを1本出すたびに、レビューとテストが同時にランナーを取り合っていました。

自動レビューは1回あたり8〜11分ランナーを専有します。さらに負荷試験も動いていて、直近60実行のうち40件がPR起因、1回あたり平均174秒、最長では576秒ランナーを1台占有していました。2台しかないうちの1台がそれだけ埋まれば、残りのジョブは当然待たされます。

調べていて気づいたのは、この構成が「全部並行に走らせたほうが速い」という前提で組まれていたことです。並行実行は、ランナーが潤沢にあるときには正しい判断です。しかし2台しかない環境では、並行に投げた分だけキューが伸びるだけで、体感の待ち時間はまったく縮んでいませんでした。ついでに言うと、直列化の理由を説明したコメントには「セルフホストランナーは1台」という古い記述が残っていて、実態と合っていませんでした。実際の直列化の理由は台数ではなく、テスト用のポートとRedisを専有するためです。

IT女子 アラ美
全部同時に走らせてるのに、結局待ってるって本末転倒じゃない。

ITアライグマ
おっしゃるとおりです。ランナーが2台しかないので、並べても順番待ちが伸びるだけでした。

レビューを一番前に置き、指摘があれば後続を走らせないことにした

考え直した結果、一番の無駄は「レビューで指摘が出て作り直すことが確定しているPRに対して、テストもビルドもE2Eも最後まで流していること」だと判断しました。指摘を受けたら直してpushし直すので、そのときのテスト結果は結局捨てることになります。捨てる結果のために、貴重なランナーを何分も使っていたわけです。

そこで、独立していた自動レビューのワークフローファイル(237行)を削除し、テストやデプロイが書かれているワークフローの中へレビューのジョブとして移設しました。移設先は290行ほど増えています。そのうえで、テストとビルドのジョブに needs を付けて、レビューの後ろへ繋ぎました。

jobs:
  claude_review:
    # PRのとき、かつ人間が出したPRのときだけ動かす
    if: ${{ github.event_name == 'pull_request' && github.event.sender.type == 'User' }}

  test-backend:
    needs: [claude_review]
    # レビューが成功したか、そもそも動かなかった(skipped)ときだけ走る
    if: ${{ !cancelled() && contains(fromJSON('["success", "skipped"]'), needs.claude_review.result) }}

動きはこうなりました。レビューで指摘が1件でも出たら、レビューのジョブを赤くします。すると後続のテスト・ビルド・E2Eは skipped のまま終わり、ランナーを1秒も使いません。指摘がゼロなら緑で終わり、後続がそのまま走ります。

ここで一点だけ慎重にしたのが、指摘の件数を数えられなかったときの扱いです。何らかの理由で件数が取れず空になったとき、それを「指摘なし」とみなして後続を走らせてしまうと、レビューが機能していないのに全部通ったように見えてしまいます。そのため、数えられなかった場合も後続を止める側に倒しました。判断がつかないときは安全側で止める、という考え方です。

なお、mainへのpushと、依存更新などのbotが出したPRではレビューのジョブ自体が動かず skipped になり、後続は従来どおり走ります。人間が書いたコードだけをレビューの対象にしたかったためです。Discordへの失敗通知も、レビューが赤いときは鳴らさないようにしました。レビュー指摘は「壊れた」ではなく「直すところがある」という状態なので、別ワークフローだった頃と同じく通知の対象外にしています。CIでAIを走らせるときの実行コストについては、AIにテストさせたら成功ログだけでコンテキストが埋まっていた話でも触れています。

IT女子 アラ美
どうせやり直すPRのテスト結果に何分も使ってたって、言われてみれば無駄ね。

ITアライグマ
はい。捨てる予定の結果を作るために待たされていたのが一番もったいなかったです。

別々のワークフローのままでは組めなかった

「順番を変えるだけなら、ワークフローファイルは分けたままでもできそうだ」と最初は考えました。実際にはできませんでした。

GitHub Actions の needs は、同じワークフローの中のジョブしか指定できません。別ファイルに書かれたレビューのジョブを、テストのジョブから needs で待つことは仕組み上できないのです。

代替案は2つありました。1つは workflow_run で、レビューのワークフローが終わったのをきっかけにテストのワークフローを起動する方法。もう1つは、テスト側のジョブの冒頭でAPIを叩き、レビューの完了を待ち続ける方法です。どちらも一見成立しそうですが、「待っている間、そのジョブはランナーを占有し続ける」という問題があります。

レビューは8〜11分かかります。待機するジョブが2台のうち1台を10分近く握ったままだと、肝心のレビュー自身が空いているランナーを確保できず、いつまでも始まりません。待つために待つ、という状態です。ランナーが潤沢な環境なら成立する設計が、2台では自分の首を絞めるだけでした。

結局、同じワークフローファイルへ統合して、ジョブの依存関係で直列化するしかありませんでした。ファイルを分けておくほうが見通しは良かったのですが、実行順序を制御したいなら同じグラフの中に置く必要がある、というのが今回の結論です。

IT女子 アラ美
待つためにランナーを1台使うって、何のために待ってるのか分かんなくなるわね。

ITアライグマ
まさにそこで詰まりました。待機自体がコストになる環境だと成立しない方法でした。

CIは成功しているのに、本番に出なくなった

この変更をマージした直後、同じ日のうちに事故が起きました。mainへマージしたのに、本番へデプロイされないのです。しかもCIの表示は成功でした。

原因は、GitHub Actions の skipped の伝わり方でした。ジョブがスキップされると、その状態は直接依存しているジョブだけでなく、さらにその先のジョブへ、依存の線をたどって遡るように伝わっていきます

今回の構成では、mainへのpushではレビューのジョブが動かず skipped になります。テストとビルドには「レビューが成功またはスキップなら走る」という条件を明示していたので、この2つはきちんと走りました。ところがその先のE2Eとデプロイには条件を書いておらず、既定の動作に任せていました。既定の動作とは「依存元がすべて成功していたら走る」というものですが、この判定にレビューのスキップが遡って効いてしまい、E2Eとデプロイまでスキップされたのです。

厄介なのは、スキップは失敗ではないという点です。ジョブが赤くなるわけではないので、CI全体としては成功で終わります。テストもビルドも実際に緑になっています。それでいて、本番には何も出ていない。テストは通っているのに定期ジョブが一度も動いていなかった話と同じで、緑色の表示は「動いた」の証明にはなりませんでした。

修正は、E2Eとデプロイにも実行条件を明示することでした。遡ってくる祖先の状態ではなく、直接つながっているジョブの結果だけを見るようにします。

  e2e:
    needs: [test-backend, build-frontend]
    # 祖先まで遡る既定の判定に任せず、直接のneedsの結果だけを見る
    if: ${{ !cancelled() && needs.test-backend.result == 'success' && needs.build-frontend.result == 'success' }}

これで祖先のスキップに巻き込まれなくなり、なおかつ前段が失敗したりスキップされたりしたときは従来どおり走りません。安全側の挙動は維持したまま、意図しない連鎖だけを断ち切った形です。デプロイについては、pushのときだけという条件と、コミットメッセージによるスキップ指定の判定はそのまま残しました。

もう1つ、自動レビューから指摘を受けて直したことがあります。テストコードのコメントに「前段がスキップされれば連鎖して効く」という、修正前の前提を説明した文章が残っていました。動作としては直っていても、説明が古いままだと、次にこのコードを読んだ人が「この条件は不要だ」と判断して消してしまいかねません。設計を変えたら、その設計の理由を書いた文章も同じコミットで直す必要があると再認識しました。

IT女子 アラ美
全部緑なのに本番に出てないって、一番気づきにくいパターンじゃない。

ITアライグマ
はい。スキップは失敗ではないので、CIの表示だけ見ていても分かりませんでした。

ついでに、PRごとの負荷試験もやめた

順序の見直しに続けて、起動条件のほうも整理しました。対象は負荷試験です。

DevPickの負荷試験には複数のプロファイルがあり、PRで動いていたのは一番軽い smoke だけでした。そして smoke の結果は、閾値を超えても警告が出るだけで、マージの可否には一切影響していませんでした。止める力を持たないチェックのために、1回あたり平均174秒ランナーを占有していたことになります。

性能の劣化を捕まえる役割は、もともとPRの負荷試験ではありません。mainへのpushで走るものと、週次で走るより重いプロファイルが担っていました。つまりPRでの実行は、待ち時間を作る以外にほとんど仕事をしていなかったわけです。

そこで、負荷試験に関わるファイルを触ったPRのときだけ動かすように、起動条件へパスの絞り込みを入れました。ここで確認したのは「絞ることで守りが薄くなっていないか」です。

負荷試験そのものが成り立たなくなる変更、たとえば閾値を持たない操作を増やしてしまうようなケースは、どのプロファイルで走らせても失敗します。そうした変更を作れるファイルは、計測シナリオを定義するファイルと、閾値を持つファイルの2つだけで、どちらも今回の絞り込み条件に含めました。加えて、判定ロジック自体が正しいかどうかは、全PRで走るバックエンドのテストが検査しています。守りを外したのではなく、同じ守りをもっと安い場所で担保できていることを確認したうえで、重い実行だけをやめたという整理です。

ついでに、古くなっていた「セルフホストランナーは1台」というコメントも実態に合わせて直しました。設定ファイルに残った古い説明は、後から読む人の判断を確実に狂わせます。

IT女子 アラ美
マージを止められないチェックに3分近く使ってたって、それはもう飾りよね。

ITアライグマ
そのとおりです。同じ検知が別の場所でできていたので、重い実行だけ外しました。

まとめ

今回の一連の変更を振り返ると、次のような整理になります。

  • 速くするより、走らせない:キャッシュやジョブの高速化に手を付ける前に、実行順序と起動条件を見直したほうが効いた。指摘が出て作り直すことが確定しているPRに、テストもビルドもE2Eも流す必要はなかった。
  • 止める力のないチェックは、待ち時間だけを生む:マージ可否に影響しない負荷試験がランナーを占有していた。同じ検知が別の安い場所でできているかを確かめてから、重い実行を外した。
  • 前段を足したら、後段の条件を明示する:スキップは直接の依存だけでなく祖先まで遡って伝わる。既定の判定に任せていたE2Eとデプロイが巻き込まれ、CIは成功なのに本番へ出ない状態になった。
  • 設計を変えたら説明も直す:古い前提を書いたコメントが残っていると、次に読んだ人が必要な条件を不要と判断して元に戻してしまう。

CIの待ち時間を減らそうとすると、つい「各ジョブをどう速くするか」から考えてしまいます。ただ、ランナーの台数が限られている個人開発の環境では、そもそも走らせるジョブを減らすほうが効果は大きいと感じました。一方で、順序を変える改修は、ジョブ単体の挙動ではなく依存関係全体の挙動を変えます。緑色のチェックマークが「意図したものが全部動いた」を意味しているとは限らないことを、身をもって確認した1日でした。

IT女子 アラ美
速くするんじゃなくて走らせない、って発想の転換が一番効いてるわね。

ITアライグマ
はい。ただ順序を変えると依存関係ごと変わるので、そこは慎重にやるべきでした。

作者が開発したサービス「DevPick」

この記事をシェアする
  • URLをコピーしました!
  • URLをコピーしました!

この記事を書いた人

ITアライグマのアバター ITアライグマ ITエンジニア / PM

都内で働くPM兼Webエンジニア(既婚・子持ち)です。
AIで作業時間を削って実務をラクにしつつ、市場価値を高めて「高年収・自由な働き方」を手に入れるキャリア戦略を発信しています。

目次