fresh digitable

めんどくさかったなってことを振り返ったり振り返らなかったりするための記録

テストの後片付けでLiveData.removeObserver()すると怒られることがある

Kotlin Android testing

はじめに

そんなことをやる必要はありません。あくまでもネタとしてお読みください。

今回の：

org.jetbrains.kotlinx:kotlinx-coroutines-core:1.4.3
org.jetbrains.kotlinx:kotlinx-coroutines-test:1.4.3
androidx.lifecycle:lifecycle-livedata-ktx:2.2.0

顛末

ViewModelのテストで、LiveDataやFlowの監視を開始したり終了したり流れてきた値をチェックする処理を簡単にできるようにしたいと考えて、そのようなクラスを作り現行の処理と置き換えたところ、次のようにコルーチンの処理が全部終わってないといって怒られた。

kotlinx.coroutines.test.UncompletedCoroutinesError: Unfinished coroutines during teardown. Ensure all coroutines are completed or cancelled by your test.
    at kotlinx.coroutines.test.TestCoroutineDispatcher.cleanupTestCoroutines(TestCoroutineDispatcher.kt:178)
        ...

試しにコルーチンを起動させている個所をコメントアウトするなどしてみたが状況は変わらず、特に変わったことしてないはずなのにおかしいなーなどと思いながらコードを見直したところ、良かれと思って後片付けの箇所に足した処理があったのを思い出した。

override fun finished(description: Description?) {  // TestWatcher の関数
    super.finished(description)
    observers.forEach { (l, o -> l.removeObserver(o) }   // これ
    scope.cancel()
}

該当の処理をコメントアウトしたところ、怒られが解消した。

解説

androidx.lifecycle.liveData()やandroidx.lifecycle.asLiveData()で作ったLiveData *1 は、activeになった時に内部のコルーチンスコープを使ってFlow.collect()を開始する。その後、inactiveな状態になると一定時間（デフォルトでは5秒）待ったあとでFlow.collect()のJobをキャンセルするようになっている。この一定時間待つという処理にdelay()が使われている。ということは当然コルーチンが起動しているので、これが完了するのを待たなければならない。テスト冒頭でDispatchers.setMain()をしつつ、kotlinx.coroutines.test.runBlockingTest()やそれに相当する関数を使うなどして、例えば、

    runBlockingTest {
        observers.forEach { (l, o -> l.removeObserver(o) }
    }

としてやることでdelay()を含んだJobを消化できる。

ちなみに

androidx.lifecycle.asFlow()の中ではJobのキャンセル時にGlobalScopeなコルーチンが起動するのでこれも終わるまで待てないとだめ（一敗）。

*1:asLiveData()は内部でliveData()を呼んでいる

fluxとかMVIみたいな構造のアプリを作ってみたかった　その5

Android Kotlin

前回： akihito104.hatenablog.com

前回からの進捗としては、NavigationDelegateをActivity, Fragmentに持たせることにしたり機能追加をやっていた。その中でViewModel以下のクラスをリファクタリングしたところしっくりくるような形になったのでまとめてみる。

クラスの構成について

Androidのデータバインディングのスタイルをなるべく守りたいので、ViewModelをDataBindingに渡してデータのプロパティやイベントリスナの関数とバインドしたいという大前提がある。また、RepositoryのインタフェースはFlowを返すかsuspend関数にして値を返すことにしているので、FlowからLiveDataに変換する必要がある *1 。そのためにはCoroutineContextが必要なので、ViewModel.viewModelScope.coroutineContextを使うことにする。そのような前提と、これまでの実装を踏まえつつクラスごとの役割分担を見直した結果、次のような構成になった。

この構成の基本的なアイディアは、ViewModelをイベントリスナの部分と状態遷移の部分とに分割して、それぞれの処理を別のクラスに委譲するということ。ViewModelの主な役割は状態のFlowをLiveDataに変換するのみになる。また、ViewModelはviewModelScopeを持っていたり、LiveDataを持たせる都合上、Androidの世界とビジネスモデルとの境界に立って両者の橋渡しを行う役割も暗に持つことになった。イベントリスナや状態を表すためのデータはViewModel側が別途定義する。

イベントリスナはActionsクラスに委譲する。当初、このクラスにはMVIのIntentのような役割を持たせたいと考えて、イベントバスに流れてくるイベントを捕まえて個別のFlowを作ることだけをやっていたのだが、いまいち役割が軽く見えていて不要かもしれないと思い始めていた。しかし、今回の見直しで、Actionsにイベントリスナを実装させることでUIイベントの生成から個別のFlowに流すまでを担当することになり、よりMVIのIntentに近づいたように思う。また、こうすることでイベントを流す具体的な処理や具体的なイベントクラスを隠蔽することができるようになった。

状態遷移はViewModelSourceクラスに委譲する。このクラスはもともとViewStatesという名前で、Viewの状態遷移を担当していたのでやること自体はあまり変わらない。ただ、このクラスにActionsを注入する都合から、このクラスにもイベントリスナを実装させActionsに委譲する形にしておけば、ViewModelはこのクラスだけに依存すればよいことになる。もしそうなったとき、ViewStatesとViewModelとの違いは状態をFlowで流すかLiveDataで持つかだけになるので、実質的にViewStatesはほぼViewModelと同じものになるんだなと考えて名前をViewModelの方に寄せる（ViewModelSource）ことにした。

状態遷移の処理もこれを機に見直した。当初はViewのプロパティをそれぞれ個別のFlowやLiveDataにわけて更新していたが、複数のFlowやLiveDataが関係しあうとコードが複雑になってよくなかったので、ViewModelから提供される状態データのインタフェースを実装したデータクラスを使って一元的に管理することにした。このあたりのことはまた別の記事に書こうと思うが、ざっくりいうとRxやFlowにあるscan()のような仕組みを使って、イベントが流れてくるごとに新しい状態のデータを生成して流すというような感じの実装になっている。

流れてきたイベントや状態遷移の結果、画面遷移したりSnackbarで表示するようなちょっとしたメッセージフィードバックをしたいときはそれ用のFlowを用意してActivityやFramgent向けにイベントを流す。

ここまでやってきて

当初はfluxやMVIのようなものを目指して作ってきたが、自分の理解力が足りなかったり実装力が無かったりしてバグが多く、ちょっと足したり削ったり移したりするとすぐ壊れてしまって難しかった。しかし、今の形に至り、ある程度わかりやすくまとまったのではないかと感じている。今、これは何かと尋ねられたら、大きくなってしまったViewModelを分割するための一つの方法、と答えると思う。今の実装をもっと洗練させていくとfluxやMVIといったものに近づいていくのかもしれないが、もうあまり意識していない。ただ、ある程度の達成感は得られたものの、この試みがここで終わるのかと聞かれるとそれも違う気がしていて、同じタイトルでこのまま続けていくかもしれない。

次回: akihito104.hatenablog.com

*1:いずれFlowのままDataBindingに渡せるようになるだろうから必要なくなるかも

テスト対象を初期化する際にはlazyを使う

Android Kotlin testing

前置き

JavaでJUnit4のテストを書いていたころは、@Beforeを付けた（setup()のような名前の）メソッドの中にテスト対象やその依存関係の初期化処理を書き、テストケースの中で使うもの（テスト対象など）はフィールドに定義してsetup()メソッドの中でフィールドにセットしていた。

private Foo sut = null;

@Before
public void setup() {
  final FooChild child = new FooChild();
  sut = new Foo(child);
}

個人的には、このテストクラスの文脈として共有したいことをsetup()に書いて整理するというスタイルが気に入っていたのだが、Kotlinでテストを書くようになってからはそれをやめて、テスト対象やその依存関係をテストクラスのプロパティとして書くようになった。

private val sut: Foo = Foo(
  FooChild()
)

多くの場合はこれで何の問題もないと思うが、気を付けておきたいのはTestRuleを使ってテストのセットアップを行う場合で、特にAndroidアプリの開発においては、テスト対象がLiveDataのプロパティを持っているようなときに使うInstantTaskExecutorRuleや、Kotlin coroutineのテストをするときにTestCoroutineDispatcherを使うようなときである。

初期化の順序に気を付けよう

ポイントは初期化処理の実行順で、テストではざっくり言って次のようになる。

  1. （テストクラスの@BeforeClass関数）
  2. テストクラスのコンストラクタ（プロパティの初期化処理、TestRuleのコンストラクタの処理もここで行われる）
  3. TestRule.apply()でStatement.evaluate()より前に実行される処理（TestWatcherを実装しているならstarting()）
    • InstantTaskExecutorRuleではここでLiveData内部で使うExecutorをテスト用のモックに差し替えている
    • TestCoroutineDispatcherを使ったセットアップではこのタイミングでDispacherをテスト用のモックに差し替える
  4. テストクラスの@Before関数
  5. テストケース

先の例ではテスト対象の初期化はリストの2番目で行われるが、テスト対象の初期化の中でLiveDataがアクティブになったり、coroutineのDispacher.Mainなどにアクセスするようなことをやっているとその時点でモックしなさい例外*1が出てクラッシュする（androidx.lifecycle.liveData()など）。なので、テスト対象の初期化はリストの3番目の後に行われるようにしなければならないが、さりとてsetup()の中で生成してlateinit varなプロパティにセットするというのも忍びない。手っ取り早く解決するにはlazyを使うのがいいだろうか。

private val sut: Foo by lazy {
  val child = FooChild()
  Foo(child)
}

こうしておけばsetup()やテストケースの中で初めてsutにアクセスしたときに初期化されるので、モックしなさい例外を回避できる。ただし、lateinit varの方がすっきり書けるという場合にはそっちの方がいいと思います。テストクラス用のTestRuleを自前で用意してその中でテスト対象を初期化するようなケースでは、lateinit varの方をバッキングフィールドにして隠すという手も使えるでしょうし。

*1:個人的にそう呼んでいる