symfony/symfony#10983
に関するメモです。
単なる覚え書きなのでいつも以上に乱文ですがご容赦を。
前提
私とmockの出会い
http://phpmentors.jp/post/55127910549
私も昔(ほんの一年前)はmock使わずテストを書いてました。というのも私の主戦場がsymfony1で、テスト機構はlimeとsfTestFunctionalだったわけです。そこにmockという機能はありませんでした。
Pull Requestのきっかけ
仕事でGoutteを使ってみようと思ったのがそもそもの発端です。
https://github.com/fabpot/goutte
Goutteは内部でSymfony¥Component¥DomCrawlerを使っているので、必然的に色々な日本語HTMLをDomCrawlerに渡すことになるわけですが、Symfony(に限らず海外ライブラリを使う場合)に頻出の日本語(マルチバイト文字)対応はどうなっている?というのが気になりますよね。それで、色々なHTMLを作ってDomCrawlerに渡してみることにしました。
正直EUC-JPやSJISのHTMLは読み取り失敗てもしょうがないかなーと思っていたんですが、意外なことにEUCもSJISも大丈夫でした。(よく考えたらsfBrowserやsfTestFunctionalでも既にOpenPNE3の携帯版(SJIS)のテストができていたので意外でもないかも?)
しかし、不可解なケースも出てきました。
<meta http-equiv="Content-Type" content="text/html; charset=Shift_JIS" />
はOKなのに
<meta charset="Shift_JIS" />
がNGなのです。
DomCrawlerの中を見てみると、addContent()で後者のタイプのmetaタグから文字エンコーディングを抽出する正規表現が英数+ハイフンしか考慮していないことがわかりました。UTF-8やEUC-JPはそれでも良いですがShift_JISは「_」があるので正常に読み取れていなかったわけです。
そこを修正して<meta charset="Shift_JIS" />からも正しくShift_JISを読み取れるようにしたいと思いました。
Pull Requestを出すまで
正規表現の修正自体は「_」一文字足すだけでさっさと終わるのですが、テストをどう書くか悩みました。
というのも、従来のテストは、モックを全く使わずに、コンテンツをaddContent()に渡した後、実際にaddHtmlContent()やaddXmlContent()が呼ばれてDOMに追加された結果をテストしていたからです。
https://github.com/symfony/symfony/blob/cff410507f8485133c19019257f0ea4d3cfcd38f/src/Symfony/Component/DomCrawler/Tests/CrawlerTest.php#L208
$crawler = new Crawler();
$crawler->addContent('', 'text/html; charset=UTF-8');
$this->assertEquals('foo', $crawler->filterXPath('//div')->attr('class'), '->addContent() adds nodes from an HTML string');
DomCrawler::addContent()の処理内容は、まず渡されたコンテンツの種類がxmlかhtmlかを判別し、更にコンテンツのcharsetを判別した後、種類によってaddXmlContent()かaddHtmlContent()を呼び出して、コンテンツとcharsetを渡すというものです。
今回は私が変更した部分が正しいかどうか、つまりhtml5形式のmetaタグでShift_JISをcharsetとして判別できるようになったかどうかだけをテストしたいのにも関わらず、従来の形式を踏襲したテストを書くとすれば、addHtmlContent()やaddXmlContent()(ひいては、実際にaddHtmlContent()の内部でそのcharsetを渡されるmb_convert_encoding())がその$charsetを受け付けるかどうかも考慮しなければならないのです。もちろん、判別された$charsetはaddHtmlContent()なりaddXmlContent()で利用できるものでなければならないのですが、実際にある$charsetが受け入れられるかどうかはaddHtmlContent()なりaddXmlContent()の関心事であって、addContent()側で知っている必要があるとは思えませんでした。
ただ、Shift_JISに関しては<meta http-equiv="Content-Type" content="text/html; charset=Shift_JIS" />の形式で指定されているときにaddHtmlContent()に$charsetとして正常にShift_JISが渡され(←たぶん)、それを使ってaddHtmlContent()が正常にSJISエンコーディングされたHTMLを扱ってくれるのが既にわかっています。それなら、多少の不安はあっても妥協しても良いという決意ができたので、従来のテストと同じ形式でShift_JISで日本語を読み取るテストをつけて、Pull Requestを出しました。
追加修正の提案とテスト追加
Pull Requestを出すと、今まで想定外だった「_」を含むcharset名がでてきたことをきっかけに
htmlの仕様などを読んで、「_」だけでなく「:」と「.」もcharset名として使われる可能性がある、ついでに追加すべきでは?と提案してくれる人が出て来ました。
正規表現に更に文字を追加するのは別にかまわないのですが、問題はテストです。
SJISと違って、「:」や「.」を含む文字エンコーディングはhtmlの仕様に載っていたcharset名リストを眺めても全然馴染みがなく、それを使って判別できるとうれしい文字というのも皆目見当がつきません。"test"とか適当な文字列を入れてテストをして、addHtmlContent()側でエラーが出たら、よく知らないエンコーディングのためにmb_*関係を調べる手間をかけることになります。逆に、テストが通ったとしても本当に正しく文字エンコーディングが判別できた結果なのかfallbackのエンコーディングで処理された結果なのかの区別もつけられません。
そこで、私はCrawlerのインスタンスをパーシャルモックとして作成し、addHtmlContent()を呼ぶ時に第二引数として渡される$charsetをチェックするテストを書き、コミットを追加しました。
ところが、この後から追加したテストについて、テスト対象のクラス自体をパーシャルモック化したテストはよくないと言われ(:と.について提案してくれたのとは別の人から)、「:」や「.」を含む適切なテストデータが見付けられないと説明したところ、「テストするのが難しいなら、テストしなくていい」と返ってきました。最終的に「:」と「.」を追加したことについてのテストを削除した状態で、私のPull Requestはマージされました。
マージされて一週間近く、私はまだモヤモヤしています…。
ちなみに、テストを認めてもらおうと説明する過程で、twitterやfacebookで愚痴っていたところ、@t_wadaさんから、文字エンコーディングを抽出する処理を別メソッドにして、その新しいメソッドに対してテストを書くという方法ならどうか、と教えてもらいましたが、残念ながら「それいいね、それで行こう!」とはなりませんでした。
まあ、一回のPull Requestでモヤモヤしたからと言って、SymfonyにPull Requestを送ることや #symfony_ja へのコントリビュート(主にドキュメント翻訳)は止めませんし、Symfonyも使い続けます。
Symfonyは素晴らしいプロジェクトだけど、全員の考え方が一から十まで完全に一致できるわけではない、と思い知ったのでした。