LoginSignup
9
1

More than 3 years have passed since last update.

NJC Advent Calendar 2019Day 5
@sakamoto_t(貫 坂本)

リーダブル自動化テスト

Posted at

AdventCalendar前日の記事とリーダブルコードかぶっちまった＼(^o^)／

テスト自動化経験はまだ浅いですが、リーダブルコードを読んで色々思うところがあったのでそのまとめです！
なお僕のチームではテスト自動化にはSelenideを使ってます。

もともと新人向けに作ったのを再編したので、当たり前なことが含まれていますがご愛嬌ということでm(__)m
コーディングルールとかを決めてガッチリ作っている人は読まなくてもいいと思います・・・
手探りな人向け！

自動テスト作りは責任重大

※このセクションはポエムです。読み飛ばして何ら問題ありません。

テスト自動化のコードは、製品のコードより大切だと思ってます。
なぜって、製品の品質はテストで担保するけど、テストの品質は何も担保がないからです。

@Test
public void 至高のテスト() {
    ・・・
}

@Test
public void 完璧なテスト() {
    ・・・
}

@Disabled("システムエラー出るからやらない")
@Test
public void 奇怪なテスト() {
    ・・・
}

@Disabled("何故か毎回失敗するからやらない")
@Test
public void 悪夢のテスト() {
    ・・・
}

自動テストはいろいろな人が使います。
先人たちが作った秘伝のソースに自分のソースを書き加えながら、テストを実行していきます。

読みにくいソースコードは誰も触りたくありません。
何をしているかわからないコードは"おまじない"になります。
とりあえず動いてるし、壊したくないから触らない。

あれ、このテスト、何してんのかわからないけど、
なんかエラーになって動かないんだけど・・・

・・・

自動テストは、製品のテストが目的です。また、繰り返し実行します。
回帰テストに当たる部分を自動化することで、新規の機能強化やバグフィックスに集中することができます。
新しい機能を追加したら、その機能のテストを追加していきます。

似たような機能があれば、コピーしてテストを作ることもあるでしょう。
もちろん、おまじないは触りたくないのでそのままコピーします。
よしよし、実装できたぞ、OKついたぞ。

あれ、機能にバグが有ったのにNG判定してくれないんだけど・・・

・・・

キレイなテストと汚いテスト

テストコードが綺麗なら何のテストなのか一目瞭然！
汚いテストは・・・何をしているか読み取れるだろうか？

@Test
public void 至高のテスト() {
    売上を登録();
    assert在庫が減ったか();
}

@Test
public void 悪夢のテスト() {
    String str = "A001002"; //★★★
    //品目：800011
    //$x("//*[@id="item_cd"]").click.setValue("800011");
$(byId("item_cd")).click.setValue(/*"800011"*/"800010");
$(byId("tp_cd")).click.setValue(str);   //ハッコウフーズ
    gamen.move();   gamen.refrech();
    $x("//*[@id=\"sales-detail\"]/div[1]/span/unit[2]").setValue(100);
//  if(
        gamen.pressF12();
        //！！！エラー！！！
        if($x("//*[text()=\"閉じる\"]").exists())
        gamen.pressF2();
$(byId("item_cd")).click.setValue(/*"800011"*/"800010");
assertTrue($(byId("pkg_qty").getValue=100);
}

悪夢のテストに潜むバグを果たして見つけられるだろうか？

ソースは綺麗に書こう

自動テストは読みやすいことが何より大切！

基本的なルールは守ろう
・インデントを揃える
・意図不明なコメントを残さない
・統一感のある書き方を心がける
・常識的に書く
・というかリーダブルコード読んで

また、日本語でコーディングできるなら日本語でコーディングがおすすめ！
命名でGoogle先生に頼る必要もなくなる。
日本人が読むなら日本語のほうが読みやすいに決まってる。

テストと画面を分離しよう

テストクラスと画面クラスを適切に分離する、というのは常識、当たり前。
でも、その分離の目安は、割りと感覚的になっているのではないだろうか？

分離の目安は、人間が操作する時に意識する単位、が良い！

↓テストクラスには、人間が操作する時に意識する最小の単位でテストの操作を書く。

テストクラス
//売上を登録する。得意先と品目と数量を入れて登録を押せばいい。
画面.set得意先CD("A001002");
画面.set品目CD("800010");
画面.set売数(100);
画面.press登録ボタン();

↓画面クラスには、人間が無意識にやっていることも考慮に入れつつ、スクリプトを実装していく。

画面クラス
private SelenideElement cstCd = $(byId("CstCd"));
private SelenideElement itemCd = $(byId("itemCd"));
//（中略）//

public void set得意先CD(String cst_cd) 
    util.log("set得意先CD")
    cstCd.click();      //得意先CD入れろって言われたら、まず（無意識に）得意先CDクリックするよね？
    waitLoading画面();    //フォーカスが得意先CDに移ると画面ロードが走る、その間は（無意識に）待つ
    cstCd.setValue(cst_cd);
    pressTab();     //CD入れた後、隣の名称欄に名称出すために（なんとなく）タブキー押したりするよね？
    waitLoading画面();    //名称を取ってくるのにまたロードが走る
}
public void set品目CD(String item_cd) {
//（中略）//
}

人間が意識しない操作を画面クラスに追いやることで、テストクラスが格段に読みやすくなる。
雑多なxpathや要素idに惑わされることもなくなる。

ついでに画面クラス側にログ出力とかを仕込めば、後から操作の手順を追うのも楽ちん！

読みやすくメソッド化しよう

テストクラスと画面クラスを分離するだけで、十分何をしているのか読めるコードになる。
「コードを入れたら名称を取ってくるか？」くらいのテストならこれだけでも十分だ！

でも、テストが複雑になるに従って、テストの全容が見えにくくなってくる。
例えば、これでも短い例だけれども、もしテストメソッド名が無かったら、一瞬何をしているのかな？と考えてしまうのではなかろうか。

テストクラス
@Test
public void 売上を登録したら在庫が減ること() {
    画面.set得意先CD("A001002");
    画面.set品目CD("800010");       //品目CDを入れると在庫数が画面に表示される
    int i = 画面.get在庫数();
    画面.set数量(100);
    画面.press登録();
    画面.set品目CD("800010");
    int j = 画面.get在庫数();
    assertEqual(j, i - 100);
}

そこで、この”操作”の羅列を、テストをする時の手順単位にメソッド化する。

「売上を登録したら在庫が減るかテストしてくれ」と言われたら、多くの人は
1. まず事前に在庫を確認しよう
2. 次に売上を登録しよう
3. もう一度在庫を確認しよう
という操作のストーリーを考えるはず。（考えるよね？）

この操作のストーリー単位でメソッド化するのだ。

@Test
public void 売上を登録したら在庫が減ること() {
    // 最初に在庫数を確認する
    int i = 在庫チェック();
    // 売上を登録する（数量：100）
    売上(100);
    // 在庫が減ったか確認（アサーション）
    売上後在庫チェック(i, 100);
}

private int 在庫チェック() {
    画面.set品目CD("800010");
    return 画面.get在庫数();
} 
private void 売上(int i) {
    画面.set得意先CD("A001002");
    画面.set品目CD("800010");
    画面.set数量(i);
    画面.pressF12_登録();
}
private void 売上後在庫チェック(int i, int j) {
    int k = 在庫チェック();
    assertEqual(k, i - j);
}

テストメソッドの中には「在庫数を確認」「売上を登録」「アサーション」しかない。
これだけテストメソッドがスッキリすれば何のテストをしているのか一目瞭然だ！

また、メソッド化することで売上前と売上後の在庫数を取得する処理を共通化できたのもgood！

名前に意味を持たせよう

テストクラス
@Test
public void 最適ではないテスト() {
    // 最初に在庫数を確認する
    int i = check在庫();

    // 売上を登録する（数量：100）
    売上(100);

    // 在庫が減ったか確認
    売上後在庫チェック(i, 100);
}

これだけでもテストメソッドは十分わかりやすくなったのだが、コメントがうざったい。
変数名、メソッド名を適切にするだけで、コメントは不要になる。

テストクラス
@Test
public void Betterなテスト() {
    int 売上前在庫数 = get在庫数();

    int 売上数 = 100;
    売上を登録(売上数);

    assert在庫が減ったか(売上前在庫数, 売上数);
}

ポイントとしては

  • 誤解される命名でないか考える
    　→check在庫だと何をチェックしているのかわからない。
    　　売上登録したらマイナス在庫にならないか？のメソッドと誤認するかもしれない。

  • getとかexistsとか、おなじみのフレーズはそのまま使う
    　→在庫数を取得する()なんて無理に日本語にする必要はない。
    　　プログラミングでおなじみの表現はそのまま使おう。わかりやすさ最優先。

  • 意味のない文字列の名前を使わない（i, str, temp, ...）
    　→変数名は、その変数に何が格納されるかを示すようにする。型が何かは大した情報にならない・・・
    　　メソッド名も、その振る舞いを示す名前にしよう。基本動詞を含むものになるはず。

  • あえて変数に入れて名前をつける
    　→今回の例では単なるマジックナンバー回避みたいになっているが、これがメソッドの返り値についても同じ理屈が適用できる。
    　　スマートに「.」でメソッドを繋げたくなるが（Javaの場合）、一旦変数に入れて名前をつけると読みやすくなることがある。

  • アサーションしていることを明示する
    　→テストコードにおいて、アサーションをしているかどうかは重要だ。
    　　アサーション書き忘れていたらテストがOKになっちゃうし、アサーション結果NGになったらそこでテストが終わってしまう。
    　　見た人が「ここでアサーションしているんだな」と気付ける名前にしよう。

なお、
「良い命名が思いつかないからコメントしとこ♪」は避けたい。
コメントは往々にしてメンテされない。
実装と乖離したコメントほど地雷なものはない！

考えたことをコメントに残そう

上に書いたように、ソースの読みにくさをカバーするためのコメントは不毛なので一切排除するべきだ！
一方で、コードには現れない、そのコードに至るまでに自分が考えた（悩んだ）ことは、積極的にコメントに残そう。

例えば、一見不可解なコードになる場所はコメントをしよう。

画面クラス
public void 明細追加() {
    //まず明細にフォーカスが無いと明細追加が効かないので、品目をクリック
    品目CD(1).click();
    press明細追加ボタン();
    waitLoading画面();
}

コメントが無いと、なんで品目CDをクリックしてるんだ？明細追加に関係ないだろ、となる。
実はここは、最初press明細追加ボタン()だけで組んだら明細追加できなくて、悩んだところだったのだ・・・。
その悩みを乗り越えてこのコードに至ったことは、あとから見た人は気づけないのでコメントに残そう。
　

また、ここのコードは未完成だよ、あるいは、書き方が汚いけどあとで直すから勘弁してね、のようなコメントも残しておこう。

画面クラス
    // 列固定されている範囲はテーブルが別に組んである。〜frozen〜とかのidで。
    // カラム名のxpathが存在しなければtable[contains(@id,"frozen")]からデータを取得する、みたいに条件分岐すれば列固定も対応できるが、
    // 面倒なので今はやらない。
    public String getグリッドデータ(int row, String column) {
    //（中略）//
    }

このコメントがあれば、このメソッドを後から見る人が心の準備ができるし、
あわよくば自分の遺志を受け継いでより素晴らしいコードに仕上げてくれるかもしれない。
　

あるいは使い方が難しいメソッドの使い方を書いても良い。

テストクラス
    //+++++ 条件がTrueの間、タイムアウトするまでwaitする関数 ++++++++++++++++++++
    //+ ラムダ式なのでこんな感じで使う
    //+ waitUntil(() -> メッセージ.exists());　//メッセージが存在する間waitする
    //+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
    public void waitUntil(BooleanSupplier cond) {
        //（中略）//
    }

まとめ

改めてBeforeAfterを比べてみるとどうだろうか。
テストメソッドをここまで綺麗にかければ、日本語なのも相まって、"プログラム"ではなく"テスト"として読めるはず。
（というかBeforeは劣悪すぎるけどね）

至高のテスト
@Test
public void 売上を登録したら在庫が減ること() {
    int 売上前在庫数 = get在庫数();

    int 売上数 = 100;
    売上を登録(売上数);

    assert在庫が減ったか(売上前在庫数, 売上数);
}
悪夢のテスト
@Test
public void 売上を登録したら在庫が減ること() {
    String str = "A001002"; //★★★
    //品目：800011
    //$x("//*[@id="item_cd"]").click.setValue("800011");
$(byId("item_cd")).click.setValue(/*"800011"*/"800010");
$(byId("tp_cd")).click.setValue(str);   //ハッコウフーズ
    gamen.move();   gamen.refrech();
    $x("//*[@id=\"sales-detail\"]/div[1]/span/unit[2]").setValue(100);
//  if(
        gamen.pressF12();
        //！！！エラー！！！
        if($x("//*[text()=\"閉じる\"]").exists())
        gamen.pressF2();
$(byId("item_cd")).click.setValue(/*"800011"*/"800010");
assertTrue($(byId("pkg_qty").getValue=100);
}

最後に、リーダブルコードは読んだほうが良いよ！
プログラミング能力上がった気分になれるよ！

9
1
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
9
1