この記事は「Develop fun!」を体現する Works Human Intelligence Advent Calendar 2020 21日目の記事です。

昨日の記事は@sparklingbabyさんのStream API がもっとわかる記事でした。

あらすじ

私は2019年にWorks Human Intelligence（正確には分社前の会社）に新卒入社し、
19年10月からプロダクト開発部門に配属され、SETエンジニアとしてとある製品のJava開発環境の改善に取り組んでいます。

ざっくりとプロダクト開発を紹介するとこんな感じです。

3万クラス程度ある大規模Java Webアプリケーション
開発環境はEclipseを使用
開発者のOSはWindowsのみ

Before

私が開発チームに参加した時点では
部門として新規開発に注力しており、足下の環境改善をやる担当者がおらず、
いろいろな汚れが少しずつ蓄積していた結果、
工数的にも、技術的にも誰も手がつけられない状況になっていました。

具体的にあげると

新人が開発環境のセットアップするのに２日かかる
セットアップを正しく行えていない開発者が多く、LinterやFormatterの設定がバラバラ
推奨設定でセットアップするとIDEのエラー表示タブが1000件を超えるLintエラーで溢れてコンパイルエラーも埋もれてしまう
ユニットテストは存在していたが、メンテが一切されておらず、コンパイルすら通らない。開発者はテストコードのビルドエラーを無視してローカルでテストを実行する必要がある
依存ライブラリの管理を手動で行なっているため、安全なライブラリ更新が実質不可能
プレマージの自動チェックは一切なく、レビュワーの目視のみが頼り。結果、ビルドが頻繁に落ちる。
大半の開発者はユニットテストを利用していない

と言った感じです。なかなかにワイルドな開発者体験だと思いませんか？

そこで、部署に配属された私は、~~この環境で開発やりたくない~~この状況をなんとかしたかったため、
上長と相談して専任で開発環境改善を担当することになりました。

After

私が１年間改善に取り組んだ結果、以下のようなモダンな開発環境になりました。

開発環境のセットアップは大半が自動化され、2時間以内で終わるように
セットアップ自動化により、全員が推奨設定で開発環境を利用するように
フォーマット違反、Lintエラーを0件に削減。
ユニットテストをメンテし、コンパイルエラーを修正し、動くテスト約2000件を抽出。（動かないテストは隔離）
依存ライブラリ管理をGradleに任せることで、安全なライブラリ更新が可能に。
プレマージでビルド、フォーマッタ、Linter、ユニットテストが自動チェックされるCIを導入。ビルドエラー、Lintエラー、ユニットテスト失敗の0件維持をCI導入から半年以上達成。
開発者がユニットテストをコンスタントに書くように

この記事ではどのように改善を進めていったのかを紹介します。

ステップ1: 開発環境セットアップ工数の削減

退屈なことはPowerShellにやらせよう

まず、手始めに開発環境セットアップ手順を見直しました。
当時の開発環境構築は十数ステップに渡る手順から構成されており、

新人がセットアップを完了するのに２日かかる
間違いなくセットアップを完了させられる人はほぼいない

と言う状況でした。

セットアップが複雑な理由

このような長い設定手順になっている原因は大きく２点ありました。
1. デフォルト設定のEclipseに設定を入れていた。
2. 開発に必要なソフトウェアを各自が手動インストールしていた。

解決策

１つ目の原因に対しては、ローカル環境に依存しない設定をプリセット¹したEclipseを作成し、配布することにしました。

２つ目に対しては、また、開発に必要なソフトウェア一式をサイレントインストールするPowerShellスクリプト²を作成し、自動化しました。

荒れ果てたソースコード

これで、開発環境のセットアップ工数が大幅に削減され、全員が推奨設定で開発できるようになりました。
しかし、前述の通り、「推奨設定でセットアップするとIDEのエラー表示タブが1000件を超えるLintエラーで溢れてコンパイルエラーも埋もれてしまう」という問題があったため、
新しい開発環境への移行前に既存のLintエラーを直す必要がありました。
（なお、ここでいうLintエラーとはEclipseのError/Warning設定で指定するUnused Imports等のエラーのことです）

開墾

既存のLintエラーですが、間違った直し方をしてしまうと、大量のデグレードを発生させてしまう恐れがありました。
デグレードは言うまでもなくダメですが、
特に私は新卒１年目で、社内での信頼度はまだ低く、最初のプロジェクトで悪評が立つことは避けなければなりません。

エラーの内容を精査し、

QuickFixで一意に直せるもの（Unused Imports, Missing Annotation等）
単純なコードの削除で直せるもの（Unused local variable等）
手動修正が必要なもの

に分類し、1は一括で自動修正し、2についてはコードレビューを実施して削除して問題ないコードかを確認して慎重に修正を行いました。
3については @SuppressWarningsアノテーションを付与することで、コードは修正せずにエラーを抑制する対応を取りました。

効果

このようにして新しい開発環境への移行を行いました。
結果として以下のような効果が出ました。

セットアップにかかる工数を２日から2時間に短縮
全員が同じ設定で開発できるように
EclipseのProblems Viewに表示されるLintエラーが０件になり、自分が新たに発生させたコンパイルエラー、Lintエラーに気づくことができるように

開発環境自動化を作成したことで現場からも「セットアップが楽になった」　「開発環境を壊してもすぐ直せるので便利」と言った評判をいただきました。

ステップ2: プレマージ導入

このビルドを落としたのは誰ですか？

開発環境セットアップ自動化により、全員が推奨設定でIDEを使えるようになったのですが、

以下のような場合にフォーマッタ違反、Lintエラーが混入する可能性がありました。

何かの弾みで設定を変えてしまった場合
推奨IDEを使わずにコードを修正している場合

このままでは、徐々にエラーが蓄積し、せっかく綺麗にしたソースがまた荒れてしまいます。
それを防ぐためにプレマージでの自動チェックを導入することにしました。

目標

プレマージを導入するにあたって、以下の３点に注意して要件を設定しました。

1. 待ち時間が妥当でありジョブの待ち行列が詰まらないこと

CIにかけられるマシンリソースはオンプレのメモリ16GBのマシン２台でした。
対象プロダクトのビルドは8GBのメモリが必要で、クリーンビルドすると3分程度かかります。

繁忙期にジョブキューが詰まると、MRの承認に待ち時間がかかり、
場合によってはプレマージを無視してマージするようになってしまいます。

そのようなことを避けるために一回のチェックは5分以内に終了することを目標にしました。

2. エラーがあったときには迅速かつわかりやすく通知され、エラーがないときにはウザくないこと。

プレマージがエラーを検出したとき、最も実装が簡単な通知方法は
「MRのコミットステータスを失敗させ、開発者にジョブのログを確認させる」
というものになるでしょう。

しかし、小規模開発やCIの仕様を開発者全員が知っている場合はこれでも良いですが、
この方法は大規模開発において迅速かつわかりやすくとは言えません。

MRの失敗通知はメールで送られてきますが、メールを常時チェックしている人は少ないでしょう。
また、開発者がジョブのログを確認するのも無視できない手間がかかります。
また、ログを読み慣れていない開発者はエラー原因が分からず、CI担当者（私）に問い合わせてくることでしょう。
そうなってしまうと、コミュニケーションコストがかかってくるため、双方が幸せになれません。

今回はエラーの通知方法として
エラーの箇所をMRのコメントで指摘し、同時にエラーがあったことをSlackに通知するようにしました。
また、多数のコミットをpushしたときにMRのコメント欄が読みにくくなるのを防ぐために、コメントをつけるのはエラーを検出したときのみとして、正常終了した場合はコミットステータスの更新のみを行うようにしました。

3. 誤検知が少ないこと

プレマージの信頼性は非常に重要です。なぜなら信頼性が低いプレマージはそのうちに無視されるようになるからです。

プレマージのエラー誤検知のパターンは２種類あります。

チェック時に実行環境に障害が生じてジョブが失敗する場合
元々存在していて、MR起因ではないエラーを検出した場合

一つ目のパターンはなかなか難しい問題でCIインフラ担当者が頑張る必要がありますが、
大抵は再実行してもらうことでなんとかなります。

二つ目のパターンをなくすために
「topicブランチのHEADとMerge Baseコミットをそれぞれチェックしてエラーの増分を検出する」
と言う方法を取ることにしました。³⁴

※ここでMerge Baseコミットは以下の図のようなものです。

大規模プロジェクトにおける課題

上記要件を達成するにあたって幾つかの技術的な課題がありました。

ビルドスクリプトはAntで書かれており、長らく変更されておらずメンテナがいない状態だった。
フォーマッタおよびLinterはEclipseの付属のもの利用しており、コマンドラインから簡単に実行できない。
全クラス（3万程度）をコンパイルするのに3分程度かかる。一方でエラーは変更したソース起因のものだけ表示するためには２回ビルドしなければならず、5分に間に合わせるためには差分ビルドを行う必要がある。

解決策

フォーマッタやLinterがEclipseに強く依存していたため、切り出してビルドスクリプトに組み込むのは困難でした。

そこでEclipseをCLIから操作できるようにしようと考え、Eclipse RCP として以下の機能を実装しました。

ビルドしてビルドエラーとLintエラーをCSVに出力する
指定したファイルにフォーマッタを適用する

実はEclipse JDTは優秀なのでうまくキャッシュをすると⁵差分ビルドもしてくれるので結構⁶高速です。

効果

プレマージチェックを導入したことで以下のような効果がでました。

developブランチにコンパイルエラー、Lintエラーが混入することがほぼなくなった
以下のようなエラーをコードレビュー前に開発者が自主的に気づいて直せるようになった
- ファイルのコミット漏れ
- 不要になったメソッド、変数の削除漏れ
- サブクラスで既に定義されているメソッドと同名のメソッド、フィールドを親クラスに追加してしまうミス

また、プレマージチェックに失敗するとSlackのチャンネルに ~~レビュワーと開発者の名前が晒される~~ レビュワーと開発者に通知が飛ぶようにしたため、CIに怒られないようにしようという意識が開発者全体に浸透しました。

その結果、懸念であった、エラーが放置されたり、修正前にマージされたりということは起こりませんでした。

ステップ3: Gradle化

とあるライブラリの身元特定（バージョニング）

プレマージを導入した後、今後SpotBugs等のLinterやJUnitを整備していきたいと考えていました。
しかし、それらのツールをビルドツールの助けなしで整備するのは骨が折れるため、
先にビルドスクリプトをAntからGradleに移行することにしました。

また、外部ライブラリの管理を温かみのあるベタ書きシェルスクリプトで行っていたため、
ライブラリ更新時の変更箇所が多く、工数がかかると言う問題も解決することにしました。

目標

Antで書かれたビルドスクリプトをGradleに移行する
シェルスクリプトで管理されていた外部ライブラリをGradleのdependencyで管理する

課題

既存のAntのビルドスクリプトのメンテナはいない。あるのはAntのXMLのみ。
外部ライブラリは社内のmavenレポジトリに再ホストされており、それらバージョン情報や依存ライブラリの情報が失われているため、各ライブラリがMaven Centralのどのバージョンに対応するものか特定する必要がある

古代技術の再現

メンテナ不在のなか、絶対にデグレードを起こしたくなかったので、
ビルド結果に意図しない変更が起こらないようにテスト駆動開発でGradleスクリプトを作成しました。

つまり、Antでのビルド結果と比較して

生成されるクラスファイルがバイナリ一致すること
生成される各Jarに含まれるファイル名に意図しない変更がないこと
生成されるWarに含まれるファイル名に意図しない変更がないこと

をテストするスクリプトを書き、
テストに通るようにGradleスクリプトを書いていきました。

ご注文はライブラリですか？

外部ライブラリの依存関係解決をGradleに任せるためには、失われた各ライブラリの依存情報を再構築する必要がありました。
そのため、社内ホストされた外部ライブラリを一つ一つ展開し、バージョンの特定を行いました。

特定は簡単なものもあれば難しいものもありました。難易度順で並べると以下のようでした。

Easy: ファイル名にバージョンが書いてあるもの
Medium: Jarの中にpomファイルがあるもの
Hard: Jarの中のMETA-INF/MANIFEST.MFにそれっぽい情報があるもの
Extreme: No hints (WEB上で存在する同名Jarを虱潰しで特定できることも）

バージョンがわかったからと言って安易に差し替えてはいけません。
もしかしたらホストする際に別のバージョンをアップロードしている可能性や、Jarの一部のファイルを差し替えている可能性もあります。

バージョンのずれによるデグレードを避けるために、
二つのJarが同じバージョンかどうかを以下の基準で判定するスクリプトを書き、
確認をしながら特定を進めていきました。

二つのJarのハッシュ値が等しい場合は同じバージョン
そうでない場合、二つのJarに含まれるファイルがMETA-INF以下をのぞいて全て等しい場合は同じバージョン
そうでない場合、差分のあるファイルを確認し、同じバージョンか人間が判断する。

このようにした結果、主要な外部ライブラリについては安全にバージョンを特定することができました。
一方、特定できなかったものについてはそのまま残すこととしました。

効果

Gradleに移行したことで外部ライブラリ周りの更新が非常に楽になりました。

これまで外部ライブラリに更新があったときに各開発者がローカルで更新スクリプトを手動実行する作業がなくなった
ライブラリのバージョンを安全に更新できるようになった。

ステップ4: JUnit復活

俺、このテストが動いたら退勤するんだ

ようやくGradleに移行できたので次はJUnitの整備です。
JUnitテストは昔に書かれたテストがそれなりに存在していたのですが、
開発者がローカルのEclipseで動かしてテストするという運用になっており、

全JUnitの定期的な実行
プレマージチェックでのJUnitの実行は行われていませんでした。

また、既存のJUnitテストを動かすにもライブラリが足りておらず、
必要なライブラリを各開発者がクラスパスに手動で追加する必要がありました。

目標

gradle testで全テストが実行できるようにする
Eclipse上で特に追加設定なしにJUnitを実行できるようにする
プレマージチェックで変更のあったクラスに対応するJUnitテストがあれば実行する
- 例： SomeService.javaを修正したら、SomeServiceTest.javaを実行。
- 実行結果とカバレッジをGitLab上から簡単に確認できるように
- テストが失敗したらビルドエラーと同様にパイプラインを失敗させ開発者に通知する
日次で全JUnitを実行し、結果サマリをSlackに投稿する

動かないテスト

まず、既存のJUnitを動かせるようにbuild.gradleにテスト用のライブラリを足して、gradle testを実行してみました。

> gradlew test
....
....
....

いくつもの問題のあるテストがあり、全てを動かすの一筋縄ではいきませんでした。
例をあげると

無限ループするテスト
System.exit(0)を呼び出すテスト（←マジでEvil）
開発DBに接続しにいくテスト

等がありました。テストがハングしている雰囲気を察して、
問題のあるテストを特定し、除外していくことを繰り返すと
ようやく全テスト実行が終了するようになりました。

遅すぎるテスト

しかし、全テストを実行するのに20分程度かかっていました。
テストケース数から鑑みても遅すぎるので
遅い原因を調査していくと以下の二つが主な原因でした。

同じテストが複数回実行されている
DBに接続しようとしてタイムアウトしている

テストの重複

テストの重複は、TestSuiteクラスが原因でした。すなわち、

HogeSuite.java
 - FugaTest.java
 - PiyoTest.java
 - FooSuite.java
   - BarTest.java
   - BazTest.java

のように階層化されたTestクラスとTestSuiteクラスがあった場合、
素朴に全クラスを実行すると、Suiteクラスからの実行とTestクラスの単体実行が重複してしまうのです。

- FugaTest
- PiyoTest
- BarTest
- BazTest
- FooSuite:BarTest
- FooSuite:BazTest
- HogeSuite:FugaTest
- HogeSuite:PiyoTest
- HogeSuite:FooSuite:BarTest
- HogeSuite:FooSuite:BazTest

今回の場合Suiteクラスである必要は特になかったのでSuiteクラスはテストの対象から除外することにしました。

DBにつなぐテスト

残るはDBに接続する必要のあるテストでした。
DBにつなぐ必要のあるテストは遅くて不安定であるため、
プレマージでの実行対象からは除外することにしました。

DBアクセスするテストの多くは古い時代に書かれたJUnit3のテストでした。
ご存知の通りJUnit3ではTestCaseクラスのサブクラスとしてテストを書きます。
そして、本プロダクトでは独自のTestCaseのサブクラスをテストの
基底クラスとしていたのですが、その基底クラスがDBに接続する仕様になっていました。

そのため、テスト自体はDBアクセスを必要としないが、独自の基底クラスを継承している故にDBアクセスをしているテストが相当数あることが予想されました。
そのようなテストまで除外してしまうのはもったいないので、以下のやり方で修正することにしました。

継承の親クラスをTestCaseクラスに変更する
そのクラスのビルドが通り、実行した際に１つ以上のテストケースが成功する場合、プレマージでの実行対象とする。
そうでない場合（コンパイルが通らない場合または全てのテストケースが失敗する場合）、修正を元に戻し、プレマージでの実行対象から外す。

これで動くテストを抽出することができ、20分かかっていたテストも2分程度で終わるようになりました。

失敗するテスト

最後に導入時点で失敗しているテストをIgnoreする修正を行いました。

JUnit4テストの場合、@Ignoreアノテーションをつける
JUnit3テストの場合、テストメソッド名に_をつける（testHoge -> _testHoge)

これでめでたくgradle testで全テストが実行でき、かつ失敗件数を0件にすることができました。

CIの整備

最後にプレマージでのテスト実行を整備しました。
gradle testでは全てのテストを実行するので、
現時点では2分とはいえ、今後テスト件数が増えていくことを考えると、プレマージでの全件実行はできません。
したがって、修正したファイルに対応するテスト（末尾にTestがつくもの）のみを実行する仕様にしました。

また、gradleタスクでテストを実行する場合、普通にやるとgradleでのビルドが実行され、
5分ほど時間がかかってしまいます。
Lintエラーを出力する関係で、ビルドはgradleではなくEclipseを用いたいため、
Eclipseがコンパイルした結果を用いてテストを実行するgradleタスクを作成しました。

結果として、プレマージでのテストを30秒程度で実行することができるようになりました。

効果

JUnitのCIを整備したことで、テストを定期的に実行し、テストの失敗がメンテされる仕組みを作ることができました。

一方でテストを新規に書く人は残念ながらほとんど増えませんでした。

ステップ5: JUnit道場

やってみせ、言って聞かせて、させてみせ、ほめてやらねば、人は動かじ

上記の通り、プレマージでJUnitを動かすようにしたのですが、ユニットテストを書く開発者はほとんど増えませんでした。

ユニットテストには以下のようなメリットがあります。

サーバを起動しなくて良いため確認が速く、繰り返し実行できる
プレマージで自動実行されるため、評価時の手動テスト実行が一部⁷不要に
日次実行されるため、多くのテストを整備することで長期的にデグレードの早期発見が可能になる

目標

JUnitをコンスタントに書く文化を開発者に根付かせることを目標にしました。

現状の可視化

しかし、上の目標はまだ曖昧です。

まず、具体的な目標数値を設定するために現状の可視化を行いました。
テストの日次実行の結果を集計用DBに突っ込み、
Metabaseを使ってダッシュボードを作成し、どの部署がどのくらいテストを書いているのかを可視化しました。⁸

ダッシュボードを観察していてわかったことは

テストの追加件数は10件/月程度である
テストを追加しているのは特定のごく少数の開発者に限られる

ということでした。したがってテストを増やしていくためにはテストを書く開発者を増やす必要がありました。

目標数値

テストの追加件数を10件/月から100件/月にする。

テストを書かない理由

テストを書いていない理由を考察してみたところ以下のような要因がありそうでした。

そもそもJUnitの使い方を知らない
（E2Eテストではなく）ユニットテストを書くメリットを理解していない
プロダクトのコードのテスト容易性が低くテストを書きたくてもかけない

JUnit道場

テストを書く文化を根付かせるために、JUnitの書き方を教える研修（JUnit道場)を企画しました。

現場の開発者のテスト技術にもレベルがあることを考慮して以下のようなコンテンツを作成しました。

基礎編
- 対象者： JUnitの書き方を知らない人向け
- 参加者: 開発者40名程度
- 形式: developer dojo (30 min x 4)
- 内容: JUnit4 + hamcrest + Mockitoの基礎的な書き方をハンズオンでやってみる
マインド編
- 対象者: ユニットテストを書くメリットを知らない人向け
- 参加者: 開発者４０名程度
- 形式: オンラインYouTube鑑賞会
- 内容: t_wadaさんのTDDライブコーディング動画を鑑賞
実践編
- 対象者: 基本的なJUnitは使えるがプロダクト開発でどうテストを書いたら良いかわからない人向け
- 参加者: 各開発チームから2-3人(計十数名）
- 形式: ライブコーディング(60min) + ハンズオン（60 min x 3)
- 内容: レガシーコード改善ガイドからいくつかのテクニックを抜粋して練習用のレガシーコード上でリファクタリングしながらテストを書いてみる。

効果

JUnit道場を実施した結果

これまでユニットテストを書いていたチームは「息をするように」テストを書くように
これまでユニットテストを書いていなかったチームも所々テストを書くように

なりました。

定量的にも、JUnit道場前後でテスト追加件数が10件/月から100件/月に増えました。

今後もユニットテストをかける開発者が啓蒙活動を行なっていくことででテスト追加件数はさらに加速していくことが期待できます。

まとめ

まだまだ道半ばなところはありますが、レガシーコードであっても粘り強く改善していった結果、
モダンな開発環境、開発手法を取り入れることができました。

１年間改善を続けて来て以下のような教訓を得ました。

改善は片手間ではできない。専任者を建て、十分な裁量を与えるべし。
環境をきれいにすることに集中する。コードによくないところがあっても直そうとすると泥沼。
施策を導入する前に、何を改善・削減するのかを明確にしておく。
訳のわからない仕様であっても必ずそうなった原因がある。可能な限りその仕様になった理由を探るべし。
使われていないコード、動かないテストは潔く切り捨てる。
文化を根付かせるためには技術や仕組みだけでは不十分。地道な啓蒙活動あるのみ。

Works Human Intelligenceではレガシーコードに立ち向かうエンジニアを募集しています！

長い記事に最後までお付き合いいただきありがとうございました。
明日の記事は@_53aさんのテレワークをより快適にするソフトウェア n 選です。

Eclipse Pleiadesプラグインを使うとそのようなことができます。 ↩
Chocolatey等のパッケージマネージャを利用することも検討しましたが、一部の開発者はインターネットアクセスに制限があることから断念しました ↩
ちなみに「修正されたソースコード上のエラーを検出する」という方法もありそうですが、検出すべきエラーを見逃すケースがあるため採用しませんでした。 ↩
「エラーは０件維持されているんだからMerge Baseコミットにエラーはないんじゃないの」と思った方もいるかもしれませんが、開発イテレーション末で0件になっていますが、開発イテレーション中にエラーが混入することは想定する必要があります。また0件維持されていないWarningも同じ仕組みで検出しているためこのようになっています。 ↩
なかなかキャッシュが言うことを聞いてくれないのでJDTの判定ロジックをHackする必要がありましたが ↩
のちにGradleでの差分ビルドも試しましたが、いまのところEclipseの方が高速です ↩
もちろんE2Eテストじゃないと評価できないものもあります。 ↩
ダッシュボードは毎日眺めているだけで仕事した気になるのでとても良いですね。 ↩

新卒１年目が荒れ果てた開発環境に１年間でCIを導入し単体テストを布教した話