Skip to content

Instantly share code, notes, and snippets.

@r-plus
Last active September 14, 2018 11:25
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save r-plus/d9d5dda454b576e9b041ca3f8bf81bb0 to your computer and use it in GitHub Desktop.
Save r-plus/d9d5dda454b576e9b041ca3f8bf81bb0 to your computer and use it in GitHub Desktop.

per_fileオプションのハンドリングをするのはモデル側(or AbstractReporter)の方が良いのではないかと考えています、理由としては

  • ファイルごとの実行時間を出すのを専用のReporterにもっていってしまうと、例えばJSONReporterでJSON形式でファイル毎の実行時間をだそうと思った場合 JSONReporter側でper_fileオプションのハンドリングをしなければならなくなってしまうと思っています。
  • またThresholdオプション+per_fileの場合はファイルの合計時間に対してフィルタリングがかかるべきで、 個別の実行時間をフィルタリングした後の合計値を表示するのはちょっと違うかなと思っています。 で、それを考えると各Reporter側にper_file処理を担当させると各Reporter側で filter_executions をoverrideして threshold処理の前にファイル毎にまとめる処理を入れないとならずそれも微妙かなと。

といったあたりで、まとめると専用のReporterにもっていってしまうと他のReporterで、実装次第でこのオプションの使える使えないが出てきてしまうのではないかなと。

モデル側にper_file用のモデル(method_name, line, columnが不要なExecution・・・Executionのスーパークラス?)があってもいいかもなとは思いますが。

@giginet
Copy link

giginet commented Sep 13, 2018

返事遅れました。考えてみたのですが、

1点目のJSONReporterについては考慮済みで、2点目については考慮外でした。ありがとうございます。
手間はかかりますが、AbstractExecutionを作って、行数とカラムを持つMethodExecutionとファイル名のみを持つFileExecutionに分けてしまう方法が良いかも知れません。
この方法で両者の問題を解決できそうにみえますが、若干変更量が多くなってしまうと思います。
よろしくお願いします。

@r-plus
Copy link
Author

r-plus commented Sep 14, 2018

ありごとうございます。その方向性で行ってみようと思います。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment