Android開発のコードレビューbotを乗り換えた話

モバイル開発で利用しているコードレビューbotを最近乗り換えた話をします。

コードレビューbotとは

コードレビューbotはPull Request(以下PR)に対して、静的解析した結果などをコメントする機能を持つプログラムの事を指します。 コードレビューbotを導入すると、些末な内容はbotが勝手に指摘してくれるため、レビューワーがより重要な内容のレビューに時間を使うことが期待できます。 有名なサービスにHoundSideCIなどがあります。

Android開発でのレビューbotの役割

CookpadのAndroid開発では、下記の項目をPR毎に実行しています。

今まではこれら全てをDokumiと呼ばれるツールで行っていました。(上記の通りDokumiではコードレビューだけではなくdeploygateへのアップロードなども受け持っていたため、 コードレビューツールと呼ぶのが適切かもしれません)

Dokumi時代

CookpadのiOS/Android開発では、自社製のDokumiというレビューbot(レビューツール)を長年愛用してきました。詳しくは下記のエントリに紹介されています。
Dokumi (日本語)

長い間お世話になってきたDokumiも、利用していく間にいくつか問題点が生まれました。

  • 依存関係が難しい
    • OSSであるdokumiと社内で利用しているdokumi-cookpad-customというmoduleの関係性が複雑だった
  • セットアップが難しい
  • デプロイ手順が煩雑(gemifyされていない)
  • Dokumi以外の仕組みで動かしてる機能もあり、それらを統合したい
    • Dokumiはプラグイン機構がなく機能追加が容易ではない
  • dokumiの設定がAndroidの開発レポジトリと別で扱いづらかった
    • dokumi-cookpad-customレポジトリ内に設定が管理されていた

このような理由で、iOSの開発チームが@giginetを中心にDangerへの乗り換えを進めていました。 それに続くようにAndroid開発の環境もDangerに移行する運びになりました。 Android版Cookpadのアプリの開発環境をDokumiからDangerに移行する作業は@_litmon_が対応してくれました。

Dangerの特徴

Danger - Stop Saying "You Forgot To…" in Code Review

  • Ruby製
  • 多くのCIシステムをサポートしている
  • 多くのコードホスティングサービスをサポートしている
  • 設定をDangerfileとしてレポジトリに含める

コアは小さく、様々な機能はプラグインによって提供されています。 Dangerのプラグインは充実しているとはいえませんが、iOS環境は比較的揃っている印象です。Android関連ではFindBugs,AndroidLint,JUnitのプラグインがあります。

pluginの作成

DokumiからDangerへ移行するに当たりfindbugsプラグインがDanger側に無かったため、findbugs-dangerを作成しました。 Dangerはpluginのテンプレートをコマンドラインで作成できるなどサポートが手厚く、下記の記事を読み進めていくとrubyやruby-gemsのエコシステムに詳しくなくてもハードルは高くない印象でした。

Creating your first Plugin

Dangerの導入

導入手順は下記のページにまとめられていますが、簡単に紹介します。

Getting Set Up

Dangerはgemとして公開されています。gem install danger としても利用できますが、bundlerを経由した利用が推奨されているのでGemfileをプロジェクト直下に用意しましょう。

# frozen_string_literal: true
source "https://rubygems.org"

gem 'danger'

danger init を実行すると初期セットアップが行われDangerfileが作成され、セットアップウィザードのようなものが表示されます。

bundle install
bundle exec danger init

ひたすら長文のメッセージが流れてくるので、enterで進めていきましょう。

  • Step 1はDangerFileを作成したというメッセージが表示されます。
  • Step 2はbot用のGithubアカウントを作成を促されます。クールなアイコン画像の設定をすることを忘れてはいけません。
  • Step 3でbotアカウントでアクセストークンを作るように言われます。Publicなレポジトリの場合 public_repo の権限だけで問題ないそうです。
  • Step 4でCI側の設定を求められますが、詳しい案内はないので、setting-up-danger-to-run-on-your-ciを見ると良さそうです。
  • 全てのセットアップが完了したら、CIからbundle exec dangerを実行する様にセットアップしましょう。

danger init で作成されるDangerFileはとてもシンプルなので必要に応じてカスタマイズしましょう。導入に成功するとbotがPRに対してコメントを投げてくれるようになります。

※ FindBugsの結果からdangerが指摘した例

CookpadのDangerfile

2017/6末地点で、Android開発で利用しているDangerfileを公開します。導入の際はぜひ参考にしてください。

####
#
# github comment settings
#
####
github.dismiss_out_of_range_messages

####
#
# for PR
#
####
if github.pr_title.include? "[WIP]" || github.pr_labels.include?("WIP")
  warn("PR is classed as Work in Progress") 
end

# Warn when there is a big PR
warn("a large PR") if git.lines_of_code > 300

# Warn when PR has no milestone
warn("A pull request must have a milestone set") if github.pr_json["milestone"].nil?

# Warn when PR has no assignees
warn("A pull request must have some assignees") if github.pr_json["assignee"].nil?

####
#
# Findbugs
#
####
findbugs.report_file = "your_module/build/reports/findbugs/findbugs.xml"
findbugs.gradle_module = "your_module_name"
findbugs.report(true)

####
#
# Android Lint
#
####
android_lint.gradle_task = "your_module:lint"
android_lint.report_file = "your_module/build/reports/lint/lint-result.xml"
android_lint.filtering = true
android_lint.lint(inline_mode: true)

まとめ

Cookpadにおけるコードレビューbotの役割や、ツールを乗り換えた経緯、Dangerの特徴などを紹介などをしました。コードレビューbotをもし導入されていないのであればDangerはおすすめ出来ます。手軽にセットアップが可能なのでぜひお試しください。

Android TVアプリの自動化されたテストの小話

技術部の松尾(@Kazu_cocoa)です。

クックパッドでは、2年程前からAndroid TVに対してアプリをリリースしています。以前、Cookpad Android TV Appのデザインで考えたことにて触れられたこともあります。

f:id:kazucocoa:20170622175302p:plain

みなさんがGoogle Play Storeからダウンロードするクックパッドアプリには、1つのバイナリ(または apk)にスマートフォン/タブレット向けの実装とAndroid TV向けの実装が含まれています。そのため、スマートフォン/タブレット向けのクックパッドアプリのリリースサイクルと同じ周期でAndroid TV向けのアプリも更新されています。

1つのパッケージに全てのプラットフォームの実装を含めることで、どのプラットフォームにおいてもユーザーはただひとつのクックパッドアプリを探してインストールすれば良くなります。開発者側としても、パッケージ管理が煩雑にならずに済むという利点があります。一方で、例えばスマートフォン/タブレット向けの対応だと思っていたものや、共通して利用しているライブラリの更新などによりTV向けの機能が意図せず壊れる可能性があります。(ここはトレードオフになりますね)

クックパッドでは、そのような破壊が含まれないように、スマートフォン/タブレット同様、自動化されたいくつかのテストを実施しています。Android TV向けに関しては、日頃行うリリースフローの中では人間の手による確認は不要となっています。

この記事では、そのようなAndroid TV向けの、少しニッチな世界のテストコードのお話をします。

アプリの変更頻度

Android TV向けのUI変更を含んだ機能開発は、ここ1年以上の間、スマートフォン/タブレットに比べてほとんどありません。また、アプリの画面遷移数や機能も、スマートフォン/タブレット版と比べるとはるかに少ないです。そのため、リリース頻度はそれなりにあるが、リリース毎の変更はない状態を保っていました。

不具合の発見

ここ半年程度を振り返ると、2回ほど、スマートフォン/タブレット側の修正に影響され、Android TVでクックパッドアプリを操作した時にクラッシュを引き起こす不具合が混入していることがありました。1つは画像ライブラリに関係するもの、もう1つは不要な画像リソースを減らす時の対応漏れです。それらは、あらかじめ用意していた自動化されたテストだけで検出されています。

社内のエンジニアの多くは、通常はスマートフォン/タブレットを使い開発していますし、全ての開発においてほとんど影響のないAndroid TVの確認も要求することは効率的ではありません。(Android TV向けの多くのコードは分離されているため)そのため、あらかじめ想定していた戦略に沿って、期待したタイミングでちゃんと不具合を見つけることができました。

なお、そのテストコードのメンテナンスコストを考える方もいるかと思いますが、Android TV向けのテストシナリオだけを必要に迫られて修正した回数は2年間で1回です。他にはEspresso全体で共通して使ってるメソッドの置換などです。(例えば以下を見ると、fixと修正しているのは1年程度前のものだけ)

f:id:kazucocoa:20170622175321p:plain

このように、シェアが低い・対応優先度が低いものに対してテスト実行する量を減らすではなく、自動化に倒してリグレッションテストとして不具合をリリース前に検出できる形にしていました。

自動化されたテストの種類

ここからは、少し具体例を混ぜながらどのようなテストコードを書いているのかを載せていきます。以下ではテストサイズ の区分を元に言葉を使っています。合わせて軽く補足を足しますが、もう少し区分を把握したい方は先ほどのリンク先を参照ください。

Sサイズ/Mサイズのテスト

広く単体テストと呼ばれるような粒度の自動テストです。これらは、スマートフォン/タブレット側と共通して使っているもの以外はほとんど書かれていません。これは、Android TV自体がログイン機能を持たないなど、最小限の機能だけを持っているため、複雑な内部ロジックを持たないビューアになっていたためです。そのため、このサイズではあまり多くをカバーせず、後述する範囲で必要なぶんだけの領域をカバーしています。

Lサイズの単体・シナリオテスト

UIの単体テストとして、もしくは一連の短い画面遷移ベースのシナリオをもとに各画面の確認をLサイズのテストとして実施しています。ここではEspressoベースのテストコードに書き上げています。

キーマッピング定義

Android TVではリモコンの上下・左右などに対して以下のようにKeyEventがあり当てられていました。そのため、Android TVの文脈における用語の対応を用意し、Espressoのシナリオを記述するときに表現が実際のTVの操作に近づくようにしました。

これは、シナリオコードの可読性をあげるためのちょっとした工夫ですね。

public enum Keys {
    TV_UP(KeyEvent.KEYCODE_DPAD_UP),
    TV_LEFT(KeyEvent.KEYCODE_DPAD_LEFT),
    TV_RIGHT(KeyEvent.KEYCODE_DPAD_RIGHT),
    TV_DOWN(KeyEvent.KEYCODE_DPAD_DOWN),
    TV_ENTER(KeyEvent.KEYCODE_DPAD_CENTER),
    TV_BACK(KeyEvent.KEYCODE_BACK),
    TV_HOME(KeyEvent.KEYCODE_HOME),
    DEVICE_BACK(KeyEvent.KEYCODE_BACK);

    private final int keyCode;

    Keys(int pKeyCode) {
        this.keyCode = pKeyCode;
    }

    public ViewAction press() {
        return pressKey(keyCode);
    }
}

また、Android TVではその特性上、特定の要素に対してスマートフォンなどでいう タップ 操作がありません。基本的にはKeyEventを繰り返し入力することでカーソルを移動させたり、決定したりする必要があります。そのため、例えば以下のようにViewを特定するための一連の操作を1つのメソッドにまとめて記述し、確認したいシナリオに対してノイズになるような表現を減らしたりもしました。

    private ViewInteraction onRelatedRecipeCardOnMainActivity() {
        return onView(isRoot())
                .perform(Keys.TV_DOWN.press())
                .perform(Keys.TV_DOWN.press())
                .perform(Keys.TV_DOWN.press())
                .perform(Keys.TV_DOWN.press())
                .perform(Keys.TV_DOWN.press());
    }

Eテストとアプリの更新テスト

アプリ更新時に何らかのマイグレーションなどの処理が実行されたりする場合や、ローカルデータの不整合などに出くわすとアプリが更新直後にクラッシュしたり、次回以降の起動に失敗するなど発生することがあります。 そのため、Android TVに対しても、すでにスマートフォン/タブレットに対して行なっている自動化されたUIテストの中から1つ前のアプリバージョンからのアプリ更新の確認を自動化し、検証されるようにしています。Android TVはネットワークに接続されていればほぼ強制的にアプリの更新が実行されるため、スマートフォン/タブレットでは過去7バージョンに対する更新確認を行っている一方で、1つ前の公開されているバージョンのみの確認にとどめています。

ここで行っているバージョンアップの確認は非常に簡単で、以下のコマンドのように1つ前のバージョンがローカルに保存したデータを保持したまま、新しいバージョンに更新するというものです。マイグレーション処理などがうまくいかないなどあれば、この更新した後のアプリ起動の時に処理がおかしくなります。

# 1つ古いバージョンのアプリをインストールする
$ adb shell install com.example.app
$ adb shell am start -n com.example.app/.Main

# 新しい、テストしたいアプリをインストールする
$ adb shell install -r com.example.app
$ adb shell am start -n com.example.app/.Main

まとめ

少しニッチな話題として、Android TVにおける自動化されたテスト環境、それらがどの程度のメンテナンスコストで行われているのかを書きました。このように、大きく機能追加はないが継続してリリースする必要のある機能に対しては自動化されたテストは非常に効率的に機能します。そんな状態になっているアプリの一例でした。

TVに関するとちょっとしたよもやま話

最後に、ちょっとしたAndroid TVに関わる知見を共有しておきます。UI_MODE_TYPE_TELEVISIONの値に関してです。UI_MODE_TYPE_TELEVISION でAndroid TVかどうか判定する場合、実はAndroid TV ではない 4.x 系のセットトップボックス端末とかが引っかかることがあります。そのため、UI_MODE_TYPE_TELEVISIONをもつがAndroid TVではない環境下で、Android TV向けのAPIを呼んでしまった場合、アプリがクラッシュしてしまいます。国内ではいくつかこの状態になる端末が存在するらしく、私たちも再現するまで気づかなかったのですがこのような状態になるようです。(ただ、 UI_MODE_TYPE_TELEVISION による判定はGoogle公式にも書かれている方法ではあるのですが)

Kuroko2の近況

技術部開発基盤グループの大石です。

先日、弊社主催のイベント CookpadTechKitchen#8 〜舞台裏を支える黒衣たち〜 にて、「Kuroko2の近況とクックパッドのバッチ周りの概況」というテーマで発表させて頂きました。今回はこの発表内容の中でも Kuroko2 についてピックアップして紹介したいと思います (今回の記事ではクックパッドのバッチの概況については特に触れませんが下記資料を参照ください)。

Kuroko2 とは

Kuroko2とは、Ruby製のWebベースのジョブスケジューラーです。2014年にクックパッド社内で開発され、2016年の秋にオープンソースとして公開しました

詳細については、当ブログの クックパッドのジョブ管理システム Kuroko2 の紹介Kuroko2 リポジトリのドキュメント をご覧ください。

また、Kuroko2 のオリジナル作者である弊社高井の社内用のLT資料 The Design Philosophy of Kuroko2 が公開されており、kuroko1 の事例を元に Kuroko2 が採用しているアーキテクチャの背景を知ることができます。 Kuroko2 の内部についても説明されているため、Kuroko2 へコントリビュートする際にとても参考になる良い資料だと思います。

今回はこれらの資料にある背景に加えて Kuroko2 をOSS化するにあたって意識していた Kuroko2 の方針、そして実際に Kuroko2 をカスタマイズする方法を紹介したいと思います。

Kuroko2の方針

ジョブ管理、ワークフロー管理への要求はトレンドや新たな技術の登場によって細かな要求が今後も変わってくることが予想されます。 そのため、Kuroko2 はメンテナンスや拡張が行いやすいシンプルな設計を保つこと、様々な現場にあわせた拡張性を担保するために本体はシンプルに保っていくべきだと考えています。

そのために以下を方針として意識しました。

1. 現実的な運用を見据えた安定した設計を保つ

The Design Philosophy of Kuroko2 p.19 にある通り、Kuroko2 は実際にクックパッドのバッチ運用を通して現実的な現場を意識して作られたものです。 闇雲に最先端の技術を採用するのではなく、あえて管理のしやすさを優先して枯れたアーキテクチャを採用し、安定した運用ができるような設計を保っていくべきだと考えています。

2. 煩雑になりがちなバッチジョブ管理の問題をUIで解決する

Kuroko2 の特徴として、煩雑になりがちなバッチ管理の方法をUIがフレームワーク的にアシストしていることが挙げられます。 具体的には、ジョブ定義の際にジョブの説明を書くためのテンプレートが用意されていたり、ジョブのお気に入り機能、自分の管理するジョブが一覧で見れるダッシュボードなどクックパッドでの運用を通して必要とされたものが実装されています。

この点は Kuroko2 の良い点であり今後も重点的に改善を進めていくべき点だと思っています。ただし、当初よりフロントエンドのアーキテクチャが古くなっており、その改良は課題であると思っています。

3. スケジューラーと汎用性の高いワークフロー管理に徹する

Kuroko2 のコアになる役割は、スケジューラーとワークフロー管理に限定し、新しい役割を定義はしないようにします。 また本体にあるタスク(docs/task.md) は汎用性の高いものだけを定義し、Kuroko2 が利用されているドメインに特化したものはカスタムタスクを利用することで柔軟性を確保するようにします。

4. うまく外の機構と連携する

ワーカーが実行する処理については、現在 command-executor からシェル経由でコマンドが実行されるのみです。この部分についても特定の機構に依存するものを本体にいれるべきではなく、うまく外の機構と連携できるように目指すべきだと考えています。 弊社が採用している例として、DWHに関連する処理についてはSQLバッチフレームワークである Bricolage を利用し、AWS ECS タスクの実行は Hako を利用することでそれぞれのドメインに合わせた連携を行っています。

ただしこの点において、シェル経由は起動コストが高いという問題があり、高頻度のジョブなどで問題が出ているため command-executor と kuroko2 console との間の連携の抽象化をしてもう少し効率的な連携が行えるような改善の必要性があると認識しています。

5. 必要な部分だけに開かれた拡張性

Kuroko2 は Rails の Mountable Engine になっており、それを gem として配布しています。 この方法を採用した理由は、Kuroko2本体の機能は常にアップデート可能なように管理できて、かつ必要に応じてカスタマイズしたかったためです。 具体的には、先述したカスタムタスクの定義や、後述する Kuroko2::ApplicationController を拡張という機能を想定しています。

Kuroko2 はバッチを管理する上で生じるドメインに特化した細かなカスタマイズを行えるようにして、様々な現場に合わせるための拡張性を適切に提供できるようにしたいと考えています。

Kuroko2 を拡張してみる

それでは、先述したカスタムタスクと Kuroko2::ApplicationController への拡張の具体的な方法のサンプルを紹介したいと思います。

カスタムタスク

まず kuroko2 gem をマウントしているRailsアプリケーション内にカスタムタスクを置く場所を作ります (後述する kuroko2.yml の設定でnamespaceは任意に分けることはできますが、今回は簡単のため Kuroko2::Workflow::Task 以下にしています)。

$ cd your_kurko2_rails_apps/
$ mkdir -p lib/kuroko2/workflow/task/

次にカスタムタスク本体のコードを上記のディレクトリ以下に置きます。

module Kuroko2
  module Workflow
    module Task
      class MyProjectRunner < Execute
        def chdir
          '/home/alice/my_project'
        end

        def shell
          "./bin/rails runner -e production #{Shellwords.escape(option)}"
        end
      end
    end
  end
end

この例では、ワーカーに設置された任意のRailsアプリケーションの中で option で渡された任意のコードを実行できます。

このカスタムタスクを kuroko2.yml で設定して、利用可能な状態にします。

....
  custom_tasks:
    my_project_runner: MyProjectRunner
....

以上の設定で、kuroko2 script 内で

 env: VAL1=A
 env: VAL2=B
 my_project_runner: MyProject::Batch.run

のようにKuroko2とは別のRailsアプリケーション内のバッチを実行するための専用のカスタムタスクを設定することができます。

クックパッドでもこのようなアプリケーション毎に設定などをまとめた専用のカスタムタスクを定義していたり、Hako を利用した Docker アプリケーションへのオプション定義のショートカットとして利用しています。 また、実験的なタスクを試したりすることもできるので、Kuroko2 本体にコントリビュートする際にも事前に検証などが行なえます。

Kuroko2::ApplicationController の拡張

次に Kuroko2::ApplicationController を拡張する方法を紹介します。

ここでなぜこのような拡張が必要なのかという理由を先に述べておきます。 クックパッドではアプリケーションのエラートラッキングに Sentry を採用しており、kuroko2 gem をマウントしたアプリケーションも他のアプリケーションと同様にエラーが発生した場合に Sentry で管理したいということがあったためです。

それでは、実際に拡張する例を書いてみます。

カスタムタスクと同様に以下の拡張コードを your_kurko2_rails_apps/lib 以下に置きます。(アプリケーションがロードできる場所であればどこでもよいです)

module ControllerExtention
  extend ActiveSupport::Concern
  included do
    before_action :additional_before_action
  end

  private

  def additional_before_action
    if signed_in?
      # do something
    end
  end
end

次に、こちらもカスタムタスクと同様に kuroko2.yml に設定を行います。

...
extentions:
  controller:
    - ControllerExtention
...

今回の例はユーザーがログインしている場合になにかを行うような例にしました。 先述したクックパッドで行っている Sentry を用いたエラートラッキングでは、before_action でログインユーザーやエラーが発生したときに必要なコンテキストを設定するようなことを行っています。

Mountable Engine を採用した利点として、 kuroko2 gem がマウントされたRailsアプリケーションからある程度のカスタマイズが可能になる点です。 いまは ApplicationController だけですが、必要があればこのような拡張性はある程度確保したいと考えています。

Kuroko2 のこれから

方針の中でもすこし触れましたが、具体的には、

  • UIをモダンに改善する
  • command-executor と kuroko2 console 間の連携の抽象化
  • command-executor 自体を Rails に依存しないようにして、軽量化する
  • ドキュメントの充実

など、Kuroko2 の方針に追いつけていない部分がまだまだあると認識しています。 ひとまずはこれらの課題に対して取り組んでいく必要があると考えています。

イベントで出た一部の質問とその回答

権限管理は実装しないのか

実装する予定はいまのところありません。

ジョブの実行前は確認のモーダルウィンドウが出るので誤って実行されにくいUIをしています。 さらにクックパッドではジョブを管理しているチーム以外にもたとえば障害が起きたときなど SRE や開発基盤がジョブの description をみて適宜リトライを行うようなオペレーションを行っています。DWHの領域では部署をまたがったジョブの連携が行われておりお互いにアクセスできる必要があります。またジョブに対する操作が行われた場合、いつ誰が実行・リトライしたかをログとして記録しています。 このため厳しく権限管理をするよりも性善説にたって誰でもアクセスできるようにしておいたほうがメリットが大きいと考えています。

Kuroko2::ApplicationController の拡張を使ってカスタマイズという形で実装することは可能だと思うので、必要があればこちらの方法をおすすめします。

補足: 現在認証については Google の G Suite のみがサポートされていますが、この点については将来的に他の認証方法などの対応は必要になってくるのかなとは感じています。

command-executor がメモリを食う

Railsに依存しすぎている部分がたしかにあるので対応したいです。 必要以上にRailsに依存しないようにすることと、方針の中で触れた command-executor の抽象化も含めて今後の課題だと認識しています。

最後に

Kuroko2 の設計方針とこれからの課題、拡張方法について紹介しました。 この記事で興味を持たれたり、導入してみたいという方は是非 Kuroko2 までコントリビュートお待ちしています。 また、実際に導入したなどの事例やフィードバックなどもご連絡頂けると幸いです。

f:id:eisuke-oishi:20170614172135p:plain