facebook/jestに2本目のOSSプルリク投げて勉強になった話とか

前回の記事の最後で投げたプルリクがマージされた!

いぇい!

 

kdnakt.hatenablog.com

 

 

[コードレビューは勉強になった]

github.com

 

1本目のプルリクは2、3行の修正だったので、特に指摘も入らず、すんなりとマージされたのだが、今回のプルリクは行数も多く、同じディレクトリに置かれたテストコードを参考にしていたのだが、もっと良い書き方があるということで多くの指摘をもらった。

 

  • requireじゃなくてimportを使う

テスト対象のクラスをコードの途中でimportしていたのだが、確かに一番初めに宣言しておいたほうが分かりやすそうだった。

 

  • 無駄にdescribeを使うな

何となく他のテストコードを参考に書いたのだが、特に内容のあるdescriptionを書かなかったためか、削除することに。

 

  • 重複コードは消せ

これは本当に自分が悪かった。テストのテストをするために、個々のテストに初期化処理を書いたり、beforeEachに移したりしていたのを十分に修正しきれていなかった。

 

  • テスト結果のオブジェクトはスナップショットを使え

最初にスナップショットテストを使ったのがReactでrenderした結果のDOMっぽいやつだったので、てっきりDOM専用かと思っていたが、そんなことはなかった。

 

  • テストコードとFlowは相性が悪い

Date.now()をモックした部分とかがエラーになるので、使えないということが分かった。便利だけど残念。

 

[まとめ]

次はJavaOSSとかにプルリク出してみたい。