20 万行超のコードベースをテストせずにリファクタリングリリースした話

こんにちは、鈴木です。

 

20 万行を超えるアプリケーションのほとんど全てのソースコードを変更し、テストを行わずに本番リリースしました。

 

「それってテストいるんですか?」問題

いきなりですが質問です。ソースコードを 1 バイトでも変更したら再テストする必要はあるでしょうか。「絶対に再テストすべき」という方もいれば、「状況によるしケースバイケースかな・・」という方もいらっしゃると思います。

ケースバイケースと考える方は、どのような場合にテストを行わなくて良いと考えるでしょうか。例えば、コメント内の誤字を修正した場合はどうでしょうか。ローカル変数の名前を typo していたので修正した場合、デッドコードを削除した場合はどうでしょうか。

こんなことがありました

ある日、Python のソースコードを眺めていると、「# $Id」のような CVS 時代のコメントがありました。いまやソースコードは Git で管理しているため、残す必要の無いコメントです。

別の場所を見ると「import xxx.yyy # ○○機能をインポート」のようなコメントがありました。そのモジュールに慣れる前に書いたコメントが残っているのだと思いますが、いまやコードを見れば分かる内容であるため、残す必要の無いコメントです。

今のコーディング規約では、import 文はアルファベット順に並べることになっていますが、手元のソースコードはコーディング規約ができる前に書かれたものなので import 文はアルファベット順に並んでいません。

こうしました

全部のソースコードを開いて、CVS 時代のコメントを削除し、import 文に書かれている不要なコメントを削除し、import 文をコーディング規約に沿うように並び替えました。そして「テストはいらないと考えます」と書いてプルリクエストを出しました。

こうなりました

レビュアーから「テストしないの!?」というツッコミが入りました。おおぅ!

物議を醸すその変更

なぜツッコミが入ったかというと、単体テストを行わずにプルリクエストを出したからです。修正内容は軽微ですが、コメント以外も変更しています。レビュアーとしては「テストしなくて大丈夫?」と心配になるのは当然の反応ですよね(^^;

レビュアーと議論して見つけた落とし所 

この時に出したプルリクエストは、結果として非常に価値のあるものになりました。

というのも、プルリクエストに対して「テストは必要」「いや不要だ」という主観のぶつけ合いにはならず、「どのような場合にテストが必要なのか」というディスカッションに発展したからです。 

AST が変化しない変更ならばテストは不要だ!

レビュアーとディスカッションして辿り着いた結論は「抽象構文木 (AST: Abstract Syntax Tree) が変化しない変更であればテスト不要」でした。

抽象構文木とはプログラムの本質的な構造を表現したものです。例えば以下のコードをご覧ください。

def add(a, b):
return a + b

これを少し書き換えます。

def add(a, b):
return a + b # a と b を足した値を返す

不要なスペースとコメントを追加しました。両者のソースコードはバイトレベルでは異なりますが、プログラムの本質である AST は等しいです。

抽象構文木 (AST: Abstract Syntax Tree) とは

f:id:monotaro-tech-blog:20180924135514p:plain

一般的なコンパイラは、ソースコードを読み取り、字句解析、意味解析、最適化、コード生成、という順番で処理を行います *1

抽象構文木とはその過程で構築されるデータ構造であり、プログラムの本質的な構造を木構造で表現したものです。抽象構文木には「コメント」や「不要なスペース」は含まれないため、「a + b」と「a  +  b # a と b を足す」の抽象構文木は等しくなります。

Python で AST を操作する方法

「AST が変化しない変更はテスト不要」という基準ができたので、次は AST が変化しないことを確認する手段が必要です。Python には ast という標準モジュールがあり、それを使うことで AST を扱うことができます。

また、astor という ast をラップしたライブラリもあります。

今回は astor を使って小さなツールを作成します。

AST のハッシュ値を求めるツールを作る

やりたいことは変更前後で AST が変化していないことの確認です。ソースファイルを引数として AST を出力しても良いのですが、ここでは AST のハッシュ値を出力することにします。

以下のコードは引数で与えられた Python プログラムファイルを読み取り、AST のハッシュ値を出力します。

# ast_analyzer.py
import sys
import astor
import hashlib
for file in sys.argv[1:]:
ast = astor.parse_file(file)
dumped_ast = astor.dump_tree(ast)
hashed_ast = hashlib.md5(dumped_ast).hexdigest()
print("{} {}".format(file, hashed_ast))

実行すると次のように「<ファイル名> <AST のハッシュ値>」というフォーマットで出力します。

$ python ast_analyzer.py a.py b.py
a.py 3f120028bd271b98ded2e65fca91fc44
b.py 47d0920d0fb39a688eb2acb06f1e4461

これで必要な道具が揃いました。あとは以下の手順で利用するだけです。

  1. 変更前の全ソースコードについて AST のハッシュ値を求める。
  2. ソースコードを変更する。
  3. 変更後の全ソースコードについて AST のハッシュ値を求める。
  4. 1. と 3. の差分が無ければ、テストせずに本番リリースする。

最終的にこうなりました

テスト不要で本番リリースしても良い基準(AST が変化しない場合はテスト不要という基準)と、それを確認するための道具が揃いました。

やるべきことは明確になったので、最初に出したプルリクエストは一旦取り下げ、AST が変化しない変更だけを含むプルリクエストとして出し直しました。具体的に行った修正と、取り下げた修正は以下の通りです。

行なった修正(AST が変化しない修正):

  • 不要なコメントの削除
  • 不要な改行やスペースの削除

取り下げた修正(AST が変化する修正):

  • import 文の並び替え
  • コメントの docstring 化
  • 変数名の typo 修正
  • etc..

AST が変化しない修正は限られていますが、これらを行うだけで全体の 3-5% 程度の行数を減らすことができました。また、納得できる基準を見つけることができたため、自信を持ってプルリクエストを出し、レビュアーに OK をいただき、本番リリースすることができました。

まとめ

20 万行を超えるアプリケーションのほとんど全てのソースコードを変更し、テストを行わずに本番リリースしました。

それは勢いで行ったのではなく、「AST が変化しない場合はテスト不要」という基準を作り、それを確認するための小さなツールを作成し、それを用いて技術的に安全であるという自信を持って行いました。

長く続いているプロダクトであれば、初期に書かれたコードにテストが無い場合も珍しくありません。それが大規模なシステムであるなら、とても「気軽にリファクタリング」とは言いづらいものがあります。

しかし、「なんとなく不安だから」「なんとなく大変そうだから」と諦めるのではなく、技術的に妥当な落とし所を見つけて突破することができれば素敵だと思います。

 

Enjoy! 大規模システム

 

*1:インタプリタの場合は「コード生成」の代わりに直接実行するか、バイトコードのような中間表現を生成します