E2Eテストとの1年 ~もらったコードレビュー~

はじめに

こんにちは21卒のberryです。2021年6月にQA部に配属され、1年ほど基盤開発系のQAをした後、2022年7月からはfreee会計の開発チームでQAをやっています。趣味は競技プログラミングです。

QA業務ではもちろん手動でのテストもやってきましたが、ユーザーがブラウザに対して行うであろう操作をシミュレートして基本的な機能がきちんと使えるかどうかをテストするE2Eテスト自動化との関わりが深い1年強でした。

freeeには社内向けにカスタマイズされたE2Eテスト基盤が存在するため、多少コードを書き慣れていなくてもE2Eテストシナリオを書くことができます。

E2Eテストの構成については、E2Eテスト基盤そのものを開発・保守しているSEQチームの方が書いている記事もがありますのでそちらも見てみてください。

freeeの自動テストの全体構成 - freee Developers Hub（「E2Eテストの構成」の「WEB(メイン)」が該当します。）

この記事では約1年半の間でE2Eテストのリポジトリに出したプルリクエストを振り返り、新卒QAがテストコードを書くときにどんなことでつまづいたのかを共有したいと思います。

出したプルリクエスト

マージされたPRは合計で48件でした。内訳としてはテストシナリオ新規作成が27個、シナリオの安定化や保守対応が21個でした。

E2Eテストのコードを触る理由となった業務

E2Eテストに関する業務は主に下記4つでした。コードレビューの例を紹介する上ではあまり必要ない情報ですが、参考までに載せています。

2021年6月~10月課金基盤チームで課金動線のリグレッションテスト作成
- 課金系の動作確認を行うE2Eテストシナリオの洗い出し&実装
2021年12月認証基盤チームで新サインアップ動線の実装に伴い、リグレッションテスト実施
- 新サインアップをメソッドとして実装し、既存のテストシナリオで使われているサインアップメソッドと差し替えてリグレッションテストを実施
2022年6月 ERP基盤チームで組織情報の基盤システムのリグレッションテスト作成
- 直前にリグレッションテストを行っていたため、メジャーなテストシナリオをE2Eテストとして実装
2022年10~12月会計チームでプロダクトの開発に合わせて既存のE2Eテストの保守
- 画面要素の変更にテストシナリオを追従させる活動

PR上でもらったコメント

上記4つの業務で受けたレビューや議論内容をタイプ別にまとめてみます。今見返してみると恥ずかしい内容のものもありますが、あえて共有していきます。

コードの書き方の部分

コードの意味がわからない部分にコメントをつける

後から読む人のために「なぜこのコードになっているか」を書きます。何をやっているか（What）ではなく、なぜこう書くのか（Why）を意識すると良いです。特に下記を意識します。

リポジトリの中でも独特な書き方になっている部分
sleepを使っている部分（詳細は後述）
自明ではない数字が出てくる部分
- カラムのindexや、金額などがよく該当します。
- 仕様を理解している人は暗黙的にわかっていても、別チームの人が見るとわからないことが多いです。

コードを読むであろう関係者や未来の自分のためにもコメントは残していきましょう。

まとまった処理はメソッドに切り出す

何度も同じような処理を繰り返し書くのは大変で、処理の一部の仕様が変更された場合などには該当するテストシナリオ全てに修正を入れる必要があります。その修正作業も、エディタの置換機能を使って置き換えれば良いというものではなく、前後の流れを汲んで処理を書き換える必要が出てしまいます。目的の単位でまとまっている処理、例えばサインアップやフォームを伴うダイアログの入力などは、適宜引数を設定して切り出してあげると良いです。

使う分だけ実装する

YAGNI: “You ain't gonna need it”という原則があり、「今は使わないけど将来的に使うかもしれないからメソッドの引数に設定できるようにしておこう」のような設計を避けるということです。将来的に使いたくなった人がその時に追加しましょう。

freee会計のメモタグを新規作成できるかどうかを確認するE2Eテストを例として挙げてみます。

この新規作成画面では、文字列を渡すとメモタグの名前欄などを入力し、作成ボタンを押してくれるメソッドを作ると良さそうです。

厳密には下記のコードが動くわけではありませんがニュアンスが伝わればと思います。

def create_memotagu(name, shortcut_1: '', shortcut_2: '')
  name_input.set name
  shortcut_1_input.set shortcut_1 unless shortcut_1 == ''
  shortcut_2_input.set shortcut_2 unless shortcut_2 == ''
  submit_button.click
end

このメソッドは一見良さそうですが、E2Eテストシナリオとしてはメモタグの名前だけが必要なため、ショートカット1, 2は使われません。

使われないコードが残って困る例としては、

後々仕様変更等でショートカット2が消えるという時に、上記メソッドの引数としてショートカット2が定義されているため消して良い要素かどうかの判断が難しくなる。
ショートカット2フォームの実装が変わった時に、画面要素の取り方を変える必要が生じる。

などがあります。こういった困りごとを将来発生させないためにも使う分だけ実装することを心がけていきましょう。

共通メソッドにexpect式を書かない

基本的にE2Eテストはパターンを網羅しすぎないようにしていくものであり、共通メソッド化が必要なほどに同じような確認をしている場合はテストのパターンが網羅的になってしまっている可能性があります。テストケースをもっと減らせることに後から気づくこともあります。

また、テストシナリオ中で期待値を確認する部分が外部に切り出されてしまうことになるため、シナリオで確認している内容がテストシナリオファイル内で完結しなくなってしまう欠点もあります。

何個もの要素を確認するからどうしても期待値の確認をメソッドに切り出したいという場合は、期待値を配列にして返すような各ページ用のメソッドを作る、テストシナリオファイル内のみで使えるメソッドとして切り出す、という解決策もあります。イメージとしては下記のようなget_elements_methodをテストシナリオファイル側から呼び出す形です。

expect(get_elements_method).to eq ['hoge', 'fuga', 'piyo']

expect式を共通メソッドに切り出したくなった場合は上記であげたような状態に陥ってしまっていないかを考えてみましょう。

Seleniumの部分

sleepは最終手段

sleep 2 と書くと2秒間処理を待ってくれます。

処理の待機に使えて便利ですが、テスト環境によるスペックの違いや、リクエストの混み具合などの影響で2秒の待ちでは足りず、テストがfailedになってしまうことがあります。そのため、sleepは他の待ち方がないときの最終手段として考えましょう。大抵の場合はsleepを使わずとも画面に何らかの変化が起きたり、画面リロードを繰り返すことで反映を確認できることが多いです。

RSpecの部分

やむを得ない場合を除き、別のシナリオの処理やテスト結果に依存しないテストを書く

例えばメモタグの作成と削除を確認したいとき、

feature 'メモタグの存在確認' do
    scenario 'メモタグの作成' do
        # サインアップ
        # メモタグ作成
        # メモタグが作成されていることを確認
    end
    scenario 'メモタグの削除' do
        # 作成した事業所にログイン
        # メモタグ削除
        # メモタグが削除されていることを確認
    end
end

と書きたくなりますが、これでは一つ目のシナリオでサインアップに失敗した場合や、たまたま1回目のメモタグ作成に失敗した時にメモタグの削除シナリオがfailしてしまいます。そういったことを起こさないためにもシナリオ同士で依存しない作りにする必要があります。このパターンであれば下記のようになるとよさそうです。

feature 'メモタグの存在確認' do
    scenario 'メモタグの作成' do
        # サインアップ
        # メモタグ作成
        # メモタグが作成されていることを確認
    end
    scenario 'メモタグの削除' do
        # サインアップ
        # メモタグ作成
        # メモタグ削除
        # メモタグが削除されていることを確認
    end
end

確認したい部分以外の処理はbackgroundに括り出す

例えば事業所を作成し、仕訳（取引）を入力して財務諸表の数字が計算通りに出力されることを確認するテストを考えたとき、サインアップや仕訳の入力が行えることを確認したいわけではないので、そういった処理はbackgroundの領域に括り出すとより確認したい機能が明確なテストになります。

テストシナリオファイル内で画面要素の定義を行わない

ドメイン内の各ページごとに画面要素をPage Objectとして定義してテストシナリオファイルもしくは共通メソッドから要素を操作するように責務を分離しましょう。これを徹底すると画面の実装が変わった画面要素を使っているテストシナリオファイルを一つひとつ探して置き換える必要がなくなります。

完全にユーザーの操作を模倣する必要はないこともある

ユーザーは基本的にグローバルナビゲーションなどを利用してページを移動しますが、ページの遷移は多ければ多いほど時間がかかります。また、E2Eテストでは些細な実装の変更によってページの遷移のための動線がうまく操作できなることもあります。テストシナリオでは移動したいページが決まっているため、特定のURLに遷移してしまえば上記のリスクを避けることができます。