Resurrecting a Dead Library: Part Three - Rehabilitation

2018-08-20

死んだライブラリを蘇らせる：パート3 ─ リハビリテーション

私はリファクタリングが大好きです。スパゲッティコードを解きほぐし、その奥にあるロジックを明確で直感的な形で浮かび上がらせることほど、満足感を覚えることはありません。

リファクタリングには慎重さが求められることを学びました。若く無謀だった頃は、レガシーなコードベースに飛び込み、変更を制御することなど気にせずにコードを引き裂いていました。すると必ず、数日あるいは数週間後に、取るに足らないように見えて実は稀なケースで不可欠だった微妙な部分を取り除いてしまい、コードを壊していたことに気づくのです。

今回の記事では、慎重にリファクタリングする方法をお見せします。実際のレガシーなPythonライブラリをリファクタリングする際に私が用いたテクニックを解説します。ミスを最小限に抑えるために使った開発ツールチェーンや、既存の挙動を固定するためのユニットテストの追加プロセスも含めてご紹介します。

これは、機械学習を使って料理の材料表記（例：「2 cups milk」）を構造化データにパースするライブラリingredient-phrase-taggerを私がどのように蘇らせたかを綴った、3部構成のシリーズの最終回です。詳しい経緯はパート1をご覧ください。かいつまんで言えば、放置されていたライブラリを見つけ、自分のSaaSビジネスを支えるために復活させたという話です。

パート1：蘇生 ─ コードを手当てして、どんなモダンな環境でも動くようにした話
パート2：安定化 ─ コードを修復しながら機能の退行を防いだ話
パート3：リハビリテーション（今回の記事） ─ コードのリファクタリングを始めた話

現在地の確認

前回の2つの記事で、私はカスタムDockerイメージを作成してこのライブラリをどこでも使えるようにし、エンドツーエンドテストを追加して高レベルな挙動を保持しました。コードベースに変更を加えるたびに、Travis CIがすべての依存関係をビルドし、制御された環境でテストを実行するようにしました。

ここまでは、コード自体には手を付けていませんでした。既存のコードの上に、その挙動を検証するためのツールやスクリプトを付け加えただけでした。安全にコードを変更するための仕組みがすべて整ったので、ようやくリファクタリングを始められるようになりました。

空白の規約を徹底する

開発者は決して空白に頭を悩ませるべきではありません。私は新しいソフトウェアプロジェクトを始めるときは、できるだけ早い段階で空白のフォーマットを自動化するようにしています。

Pythonプロジェクトでは、これをYAPF（Yet Another Python Formatter）で実現しています。このプロジェクトで最初に行ったコード変更は、すべてのファイルを私の好みの標準であるGoogle Pythonスタイルガイドに合わせて再フォーマットすることでした。

yapf \
  --in-place \
  --recursive \
  --style google \
  ./ \
  --exclude="third_party/*" \
  --exclude="build/*"

これにより大幅なコードの入れ替えが発生しましたが、YAPFは成熟したツールであり、エンドツーエンドテストも依然としてパスしていたため、安全な変更であると確信できました。

ノイズの中に他の変更が埋もれてレビューしづらくなることを避けるため、プルリクエストは空白の変更だけに限定するように注意しました。

今後の変更でも同じスタイル規約が守られるように、ビルドスクリプトに新しいチェックを追加しました。

yapf \
  --diff \
  --recursive \
  --style google \
  ./ \
  --exclude="third_party/*" \
  --exclude="build/*"

先ほどのコマンドと同じですが、--in-placeフラグの代わりに--diffフラグを使っています。YAPFが空白の違反を検出すると、それを出力し、失敗を示す終了コードを返すため、ビルドスクリプトは失敗して終了します。

静的解析の導入

pyflakesも、私がPythonのツールチェーンに必ず加える便利なコンポーネントです。静的解析によって、未初期化の変数や未使用のimportといった不注意なミスを見つけてくれます。

これをingredient-phrase-taggerのビルドスクリプトに追加したところ、すぐに未使用のimportを検出してくれました。

$ pyflakes \
    bin/ \
    ingredient_phrase_tagger/
ingredient_phrase_tagger/training/utils.py:3: 'string' imported but unused

いよいよコードを読む

お気づきかもしれませんが、この過程を通じて私はコードを理解しようとすることを避けてきました。ライブラリの挙動について表面的な理解だけでやり過ごしてきたのです。

コードを読むための最良の方法は、リファクタリングとテストをしながら進めることだと私は考えています。著名なソフトウェアの専門家であるMartin Fowlerは、このプロセスを次のように的確に表現しています。

馴染みのないコードを見るときは、それが何をしているのかを理解しようとしなければなりません。数行を眺めて、ああ、この部分はこういうことをしているのかと自分に言い聞かせます。リファクタリングでは、頭の中でメモするだけでは終わりません。実際にコードを自分の理解がよりよく反映されるように変更し、そしてコードを再実行して正しく動くかどうかを確認することで、その理解をテストするのです。
-Martin Fowler、Refactoring: Improving the Design of Existing Code

まずいコード構成に対処する

ライブラリのコードの80%は、わずか2つのファイル、cli.py（コマンドラインインターフェース）とutils.py（ユーティリティ）に集中していました。言い換えれば、作者はコードを「ユーザーインターフェース」と「その他すべて」という2つの箱に分けただけだったのです。しかし、その分け方すらきれいではありませんでした。

cli.pyの中でも、コマンドラインからの読み書きに関係するコードはごくわずかでした。そこにはCliという単一のクラスがあり、次のようなメソッドが並んでいました。

run
generate_data
parseNumbers
matchUp
addPrefixes
bestTag
_parse_args

私がまず取り組んだのは、Cliクラスをスリム化し、コマンドラインインターフェースとしてより論理的な抽象化にすることでした。

`Cli`クラスを解剖する

Cliクラスを分割するために、まずは手がかりが必要でした。generate_dataはどう見てもユーザーインターフェースを担うクラスに属すべきものには思えませんでしたが、すぐには移動できませんでした。generate_dataはselfパラメータを介してCliの他のメソッドを呼び出しており、クラスの他の部分と状態を共有しているように見えたからです。

しかし、本当にそうでしょうか。cli.pyのすべての関数はCliクラスのメンバーでしたが、実際にインスタンス変数を共有していたのでしょうか。

Cliのコンストラクタを確認してみました。

def __init__(self, argv):
      self.opts = self._parse_args(argv)
      self._upstream_cursor = None

コンストラクタではself._upstream_cursorに値が代入されていましたが、この変数を参照している箇所はどこにもありませんでした。デッドコードだったので、簡単に削除できました。

もう一つのメンバー変数であるself.optsはデッドコードではありませんでしたが、これを参照していたのはrunとgenerate_dataの2つのメソッドだけでした。

共有される状態がなければ、Cliの他のpublicメソッドがメソッドである理由はまったくありません。すべてモジュールレベルの自由な関数として存在できるのです。さらに言えば、cliよりも目的を的確に表すまったく新しいモジュールに移動させることもできます。

きれいな抽象化を作る

Cliのほとんどのメソッドが別のモジュールに移せることが分かってからは、その新しいモジュールを設計する必要がありました。もちろん、すべての関数を移動してpublicにすることもできましたが、Cliクラスとこの新しいモジュールとの間のインターフェースを最小限にしたいと考えました。

Cliが他のすべての関数をgenerate_dataのループ本体の中で呼び出していることに気づきました。その部分を新しい関数として抽出すれば、Cliは新しい関数だけにアクセスすればよく、以前のメソッドには一切触れる必要がなくなります。

この変更により、Cliクラスはよりスリムで論理的にまとまりのあるものになりました。現在は2つのpublicメソッドと1つのprivateメソッドだけから構成されています。

run
generate_data
_parse_args

まだ完璧ではありませんでしたが、以前の肥大化したインターフェースよりはましになりました。もちろん、やりたい変更は他にも山ほどありましたが、それらは後回しにすることにしました。

ミスの可能性を最小限に抑えるため、リファクタリングでは各プルリクエストのスコープを厳しく絞りました。ファイル間でコードを移動する際は、移動自体で行単位の変更に気づきにくくなるため、変更を最小限に抑えることが特に重要です。

エンドツーエンドテストはパスしたので、移動によって重大な破損はなかったことが分かりましたが、作業はまだ終わりではありません。リファクタリングで新しい関数が生まれたので、それを検証するための新しいユニットテストが必要でした。

初めてのユニットテスト

ユニットテストの作成は簡単でした。translator.translate_rowの開始時と終了時に一時的にデバッグログを仕込み、入力と出力を出力させました。その値が、最初のユニットテストの入力と期待される出力になりました。

def test_translates_row_with_simple_phrase(self):
    row = {
        'index': 162,
        'input': '2 cups flour',
        'name': 'flour',
        'qty': 2.0,
        'range_end': 0.0,
        'unit': 'cup',
        'comment': '',
    }
     self.assertMultiLineEqual("""
2\tI1\tL4\tNoCAP\tNoPAREN\tB-QTY
cups\tI2\tL4\tNoCAP\tNoPAREN\tB-UNIT
flour\tI3\tL4\tNoCAP\tNoPAREN\tB-NAME
""".strip(),
                              translator.translate_row(row).strip())

この関数が何をしているのか、まだ完全には理解できていませんでしたが、ユニットテストによって理解が一歩進みました。この関数は、次のようなCSVファイルに格納されたライブラリの学習データを処理していることが分かりました。

index	input	name	qty	range_end	unit	comment
162	2 cups flour	flour	2.0	0.0	cup

そして、ライブラリの機械学習エンジンが理解できるタブ区切りの値を返していました。

さらにいくつかのユニットテストを追加して、分数を含む材料（"1 1/2 teaspoons salt"）やコメントが付いた材料（"Half a vanilla bean, split lengthwise, seeds scraped"）など、さまざまなタイプの材料をカバーしました。

ユニットテストをビルドに組み込む

ユニットテストはビルドプロセスに組み込まれてこそ意味があります。そこで、ビルドスクリプトを更新してテストを含めるようにしました。

ビルドスクリプトにユニットテストコマンドを追加するdiffのスクリーンショット — ビルドスクリプトにユニットテストの実行を追加

Travis CIがすでにコード変更のたびにビルドスクリプトを実行していたため、次のTravisビルドでユニットテストの出力を見ることができました。

ユニットテストのログ出力 — Travisのビルド出力におけるユニットテストのログ

コードカバレッジの追加

リファクタリング中に、より多くのコードをテストの対象にしてカバレッジの数値が上がっていくのを見るのが大好きです。Pythonプロジェクトでは、カバレッジ情報の収集にcoverageモジュールを、結果をウェブダッシュボードで確認するためにCoverallsを使っています。

Python標準のユニットテストランナーからcoverageへの切り替えは、ビルドスクリプトへのごく小さな変更で済みました。

-python -m unittest discover
+coverage run -m unittest discover

次に、Travisがコードカバレッジ情報をCoverallsにアップロードするように、Travis設定にafter_successキーを追加しました。

after_success:
  - pip install pyyaml coveralls
  - coveralls

Coverallsを確認し、期待に胸を膨らませてカバレッジの統計を見てみると……

結果が表示されないCoverallsのスクリーンショット — Coverallsにコードカバレッジ情報が何も表示されない

何も表示されませんでした。

コードカバレッジはどこへ行ったのか

これまで何十ものプロジェクトでCoverallsを使ってきたので、なぜ何も表示されないのか理解できませんでした。単純なPythonプロジェクトのはずです。coverageコマンドは.coverageというファイルにカバレッジ情報を作成し、coverallsコマンドがそれをCoverallsのダッシュボードにアップロードするはずでした。

あっ、そういうことか。coverageコマンドはDockerコンテナの中で実行されていましたが、coverallsバイナリは通常のTravis環境で実行されていたため、.coverageファイルが見つからなかったのです。Dockerコンテナから外側のTravis環境へコピーしていませんでした。

修正は簡単でした。Dockerコンテナから.coverageファイルを抽出するコマンドを追加すればよかったのです。

after_success:
  - pip install pyyaml coveralls
  - docker cp ingredient-phrase-tagger-container:/app/.coverage ./
  - coveralls

それでも、Coverallsのダッシュボードには何も表示されませんでした。

しかし、Travisのビルドでは、これまでのビルドにはなかった出力が出ていました。

$ coveralls
Submitting coverage to coveralls.io...
No source for /app/ingredient_phrase_tagger/__init__.py
No source for /app/ingredient_phrase_tagger/training/__init__.py
No source for /app/ingredient_phrase_tagger/training/cli.py
No source for /app/ingredient_phrase_tagger/training/translator.py
No source for /app/ingredient_phrase_tagger/training/utils.py
Coverage submitted!
Job #177.1
https://coveralls.io/jobs/39259674

そこで、別の問題があることに気づきました。

TravisとDockerでは、ファイルシステムの見え方が食い違っていました。例えば、cli.pyファイルはそれぞれ次のように見えていました。

環境	ファイルパス
Dockerコンテナ	`/app/ingredient_phrase_tagger/training/cli.py`
Travis	`/home/travis/ingredient_phrase_tagger/training/cli.py`

そう考えると、Travisでcoverallsが出力したエラーメッセージにも納得がいきました。

No source for /app/ingredient_phrase_tagger/training/cli.py

.coverage内のパスはDockerコンテナ側のファイルシステムに基づいており、Travisのファイルシステムには/appというパスが存在しないため、Coverallsはファイルを見つけられなかったのです。

同じファイルに対する見え方が異なる、この2つの環境の隔たりをどう埋めればよいのでしょうか。解決策は見つかりましたが、少々回りくどいものでした。

回り道をしてパスを変換する

coverageのドキュメントを見ていると、複数のファイルシステムからのパスを統合することについて説明したpathsオプションがあることに気づきました。

pathsドキュメントのスクリーンショット — `paths`オプションのドキュメント

これらのオプションを使うために、次のような.coveragercファイルを作成しました。

[run]
source = ingredient_phrase_tagger

; Run in parallel mode so that coverage can canonicalize the source paths
; regardless of whether it runs locally or within a Docker container.
parallel = True

[paths]
; the first path is the path on the local filesystem
; the second path is the path as it appears within the Docker container
source =
  ingredient_phrase_tagger/
  /app/ingredient_phrase_tagger/

新しい解決策では、Dockerコンテナ内でcoverageコマンドを実行し、その後Travis環境でcoverage combine機能を実行することで、すべてのパスをTravisのファイルシステムに正規化しました。

この解決策を適用した後、Travis設定のafter_successセクションは次のようになりました。

after_success:
  - pip install pyyaml coveralls
  # Copy the .coverage.* file from the Docker container to the local filesystem.
  - docker cp ingredient-phrase-tagger-container:/app/$(docker exec -it ingredient-phrase-tagger-container bash -c "ls -a .coverage.*" | tr -d '\r') ./
  # Use coverage combine to canonicalize the source paths.
  - coverage combine
  # Upload coverage information to Coveralls.
  - coveralls

ようやくコードカバレッジを取得

完全な解決策を試してみました。ついにCoverallsが結果を受信し、コードカバレッジの数値を表示してくれました。

コードカバレッジの統計を表示するCoverallsのスクリーンショット — Coverallsにようやくコードカバレッジ情報が表示された。

ここに、このライブラリの復活を宣言します

コードカバレッジのトラッキングを統合した後、このライブラリは再び息を吹き返したと感じました。品質で賞を取るような代物ではありませんでしたが、私や他の開発者が高い自信を持ってコードの改善を続けられるだけのインフラは整いました。

この一連のブログ記事を通じて、私はライブラリを小さく区切ったステップでどのように改善してきたかを説明してきました。これによりバグの可能性は最小限に抑えられましたが、全体像は見えにくくなったかもしれません。少し視点を変えて、復活させる過程で行ったハイレベルな改善を振り返ってみます。

改善前	改善後
OS Xでしかビルドできない	Dockerが使える環境ならどこでもビルドできる
エンドツーエンドテストなし	徹底したエンドツーエンドテストを完備
ユニットテストなし	少数のユニットテストと、簡単に追加できる仕組みを整備
コードカバレッジ情報なし	コミットごとにコードカバレッジを計測し、経時的な履歴を保持
自動ビルドなし	コミットごとにコードを自動的にビルド・テスト
一貫性のないコードスタイル	自動ツールによりスタイル規約を徹底
開発者が未使用のimportや未初期化の変数を手作業で見つける必要がある	静的解析を適用して不注意なミスを自動で検出

捨てるつもりでリファクタリングする

これらの変更を誇りに思っていたと聞くと驚かれるかもしれませんが、さらに数週間コードを改善した後、私はそれを捨てて完全に書き直すことにしました。

……一つは捨てるつもりで計画せよ。いずれにせよ捨てることになるのだから。
-Fred Brooks、人月の神話 — ソフトウェア工学エッセイ

コードをリファクタリングすればするほど、根本的なアーキテクチャの問題に気づくようになりました。だからといって、コード改善の努力が無駄だったわけではありません。深い理解を得るためには、実際に手を動かす必要があったのです。すべてを理解したとき、保守性とパフォーマンスを高めるために一から書き直すことに安心して踏み切ることができました。

その結果生まれたのが、Zestfulというサービスでした。ingredient-phrase-taggerと似た機能を提供しますが、ホストされたAPIとして提供されます。私がオリジナルのライブラリを動くようにするためにくぐり抜けたような面倒な手順を踏むことなく、クライアントはすぐに材料のパースを利用できます。

Zestfulが実際に動くところを見てみたい方は、ライブデモをご覧ください。

カバーイラスト：Loraine Yow。ingredient-phrase-taggerライブラリの私のフォークはGitHubで公開しています。このライブラリをベースにしたマネージドサービスとしてZestfulを提供しています。