nkns165 / geb-template Goto Github PK

もしかして、アプリケーションがうまくデプロイされていないのかもしれませんが、板垣さんにご指摘いただいたので、Filterでは名前を付け替えたのですがなんか難しい指定がされていました。
たぶん下のように変更できると思いますが、なにか行き違いかなんかでしょうか？

        filter { $("a", text: "Filter Me") }
        filterOpName { $("#filter\\2e op\\2e name") }
        filterName { $("#name") }
        filterOpTagsName { $("#filter\\2e op\\2e tags\\2e name") }
        filterTagsName { $("#tags\\2e name") }
        buttonActionFilter { $("#filterPaneForm > div > div.buttons > span:nth-child(3) > input") }

        filterMe {$("a", text:"Filter Me")}
        nameFilterOperation {$("select", id:"filter.op.name")}
        tagsFilterOperation {$("select", id:"filter.tags.name")}
        scenarioFilterOperation {$("select", id:"filter.op.scenario")}
        nameFilter {$("input", name:"filter.name")}
        tagsFilter {$("input", name:"filter.tags.name")}
        scenarioFilter {$("input", name:"filter.scenario")}
        applyFilter {$("input", value:"Apply")}

例えば、Filterというものに対してsearchと言い換えていますし、newというものに対してcreateやaddと言い換えています。
createやaddがシナリオとしてすばらしいのであれば、そうであるべきであるという主張をなんらかの形ですべきです。
いまの形では「テストコードを書いた人が言いやすかったから」というだけで、なんのメリットもありません。誤解のもとにしかならないです。

2つあって、openじゃないだろっていうのと、単語の並び方です。

まず、openMenuTestCaseだと、なにかドロップダウン的ななにかに見えます。実際には遷移をするための動作なので、clickとか、touchでしょうか。openであれば画面名をいれるくらいかと思いますが、openだったらto PageObjectにすると思いますが、どうでしょうかね。

次に、趣味の話だとは思いますが、前回お話ししたように、「目的語.動詞.目的語」という並びはあまりすきじゃないです。
主語が省略されているだけなら、わからなくもないのですが。

header#openMenuTestCase　よりは

header.testCase.click()　がよいかなぁとか。これを隠ぺいするほど意味が複雑な感じはしないです。DRYにしたいという要望で、header.testCaseではなくなる場合には、そのステップがまるごとなくなるとか、意味がまるで変わると思いました。

もし手を加えるなら、共通的なメソッドとして次のような感じに定義しておいて、

class UserFriendlyGebReportingSpec extends GebReportingSpec {
  click(Object){
   object.click()
  }
}

のようなものを定義しておき、テスト中でclick(header.testCase)とするほうが自然かなぁと思いました。

以下のコードはなにもしていないです。やめてください。

then:
true

システムレベルのテストをしているのに、「こういったケースで使いにくそう」など、改善を提案するテストがまるでありません。
また、シナリオとして実装されていないとおかしいと思われるケースもありません。
これでは、バグを見つけることもできないでしょうし、テスト戦略やテスト設計がなっていないと言わざるをえません。
まとめると、「緩い統合テスト」でしかありません。

これは好みの問題なので無視して大丈夫です。

おそらくSeleniumのPageObjectPatternとも関連があると思うのですが、コンテンツに(to:FooPage)とすることで、そのコンテンツをclickしたときにFooPageのtoを呼び出すのと同等の効果がえられるように書けます。いまだと、Module系でそれを実装している例が見れます。

これをすると、テストコード側ではおおよそat呼び出しがいりません。（toされているし、すでにpageにはFooPageがはいっているから）
ですが、僕はわざとat FooPage と書きたい場合があります。それはドキュメンテーションとして書きたい時です。

(to:FooPage)をつけるようなコンテンツにフォーカスしたシナリオではおそらく「ある画面に遷移した」ということを明示的に書きたいのですが、そうではないシナリオではそれほど重要ではないのでat FooPageは必要ないです。

おそらくこれはJavaのWebDriverでは、コンテンツ選択の結果であるpageを変数として受け取って、その変数の型が該当ページのクラス名になることによって、ドキュメンテーションの役割を担っていました。

Gebでそういった書き方をしないのであれば、atを付けたほうがよりよりシナリオと、そうでないシナリオがあるかなぁと思いました。

searchTestCasesで該当する条件にあるテストケースの集合を返していますが、比較が「空文字もしくは指定された文字に該当するもの」となっています。これでは指定したとしても、他のテストケースも返ってくるし、「正常に登録できていなかったとしてもテストがテストが成功する」という事態になります。

    public TestCaseRow[] searchTestCases(String name, String scenario, String[] tags) {
        return testCaseItems.findAll {
            (name == "" || it.name == name) && (scenario == "" || it.scenario == scenario) && (tags == "" || it.tags.split("\n").toList().containsAll(tags))
        }
    }

仮に「指定されていないときは空文字で指定する」とかにしたいのであればデフォルト引数を利用すべきです。

    public TestCaseRow[] searchTestCases(String name = "", String scenario = "", String[] tags = []) {
        return testCaseItems.findAll {
            (name == "" || it.name == name) && (scenario == "" || it.scenario == scenario) && (tags == "" || it.tags.split("\n").toList().containsAll(tags))
        }
    }

目立ったのが、コンテンツと簡単な英単語です。
buttonUpdateとなっていたり、editButtonsとなっていたりする。
Testcaseとなっていたり、TestCaseとなっていたりする。
テストメソッド名がバラバラなのはまだいいとして、簡単なコンテンツと英単語くらいの命名はそろえるべきです。
これは複数人でやっていたとしても、当然です。

極力ハードコードはやめてください。一番多いのはsleepの時間です。
コンテンツ取得時にsleepするのであれば、コンテンツ(wait:"slow"){$("input", xxx)}のようにwaitをいれたり、
ロジカルにsleepを入れたいのであれば定数化するなり、sleepと取得をリトライするような汎用的な仕組みを用意すべきです。

ブラウザの言語設定によって（つまり、HTTPヘッダのlanguageによって）表示されるメッセージが日本語と英語など切り替わる部分があります。
これらに対応するようなテストが見当たりませんでした。

Userクラスでat呼び出しをすると、テストメソッドで呼び出した後にどの画面で次の操作やthenを行っているのかわからなくなって、Test as Documentの考え方からはあまり同意できません。

addTagでは登録し、createTagでは登録しログの検査をしている。明らかにメソッドの責務が名前と乖離している。

現在のUserオブジェクトはIDとパスワードによって、Administrator機能が有効かどうかになっています。
つまり、次のように継承的な感じにするのがいいかと。

class Administrator extends User {
    def addUser(User user) {
        browser.page.header.openMenuUser()
        browser.page.addUser(user.username, user.password, user.mailAddress)
    }
}

自動化対象のシステムテスト自体のテスト戦略が不在であるため、記述する。

また、システムテストの目的(テスト戦略にあるもの)をユースケースのパス網羅をするものとする予定のため(EndToEndの正常フローを最低限保証する、セキュリティー攻撃、基盤異常などによる業務停止リスクはいったん対象外とする)元となるユースケースを記述する。

まず、isXXXと書くのは、対象オブジェクトに問い合わせをしている場合だと思います。
いまのテストコードで対象オブジェクトにたいして問い合わせをするということは結構不自然かなぁと思いました。

例えば次のようなコードはたくさんあって、ちょっと違和感。

then:"Test Case詳細画面に正常登録のメッセージが表示される"
isTestCaseCreationSuccessful()
// -> 訳すと、テストケース作成は成功した？

thenのドキュメントからもわかるように宣言をしているわけですから、次のほうが自然かな。

then:"Test Case詳細画面に正常登録のメッセージが表示される"
TestCaseCreationIsSuccessful()
// -> 訳すと、テストケース作成は成功する

みたいな。

シナリオパッケージにあるなら、意味のあるシナリオにすべきです。
これをユーザーが意味のあるシナリオとして実行するなら、むしろなにか機能改善を促すようなシナリオかと思います。
シナリオではなく、例えばテスト環境を調整するためのメソッドであるならば、もっと別のパッケージしたり、コメントをつけるなど配慮すべきです。

        when:
        to TopPage
        admin.login()
        header.openMenuUser()
        for (int i = 0; i < 100; i++) {
            if (deleteButtuns.size() <= 5) {
                break
            }
            sleep(1500)
            deleteUser(5)
            sleep(500)
            driver.switchTo().alert().accept()
            sleep(1500)
        }

searchTestCases、updateTestcaseで配列を使っていますが、特に理由がなければListを使うべきです。

ページオブジェクトでモジュールを利用して、ユーザーの一覧やテストケースの一覧のテーブル部分をリストとして扱っていますが、各項目（例えばテストケース）の削除ボタンがモジュールではなく、ページオブジェクトのコンテンツでリストとして保持されています。これはTestCaseの例です。

    static content = {
        // 略
        testCaseItems { moduleList TestCaseRow, $("#testCaseRow") } // ここでテーブル部分の一行をリストとして保持している
        editButtons { $(".glyphicon-eye-open") } // テーブルの１行に含まれている項目なのにtestCaseItems経由ではないアクセス！
        deleteButtuns(required: false) { $(".glyphicon-remove") } // テーブルの１行に含まれている項目なのにtestCaseItems経由ではないアクセス！
    }

これは、あまりに責務分割ができていなさすぎると思います。TestCaseRowモジュールの中でeditButtonやdeleteButtonを実装すべきです。直してください。

以前のハンズオンで「登録することが重要な場面では、実際のインタラクションになる各項目を羅列すべきです。ですが、別のことにフォーカスしているシナリオにおいて、例えば前提条件の作成で登録が必要な場面では隠蔽して構わないと思います。」とお伝えした通りです。

例えば、Userオブジェクトをまるっとわたして登録なりログインなりしていますが、それでは「実環境のユーザーに最初にどのくらいの項目が必要とされているのか？」といったことが隠蔽されてしまいます。つまり、シナリオによって明確になる利用品質のテストを無視しています。
それでよい場合もありますが、それは概ね別のテストだと思います。
こういったテストケースを書いていて問題になるのは
「え！最初はTwitterアカウントだけでログインできると思ったのに、これとこれとこれもいるの？テストしていないの？」
「いえ、テストは書いています。ただし全ての情報をインプットしていました」みたいになります。（ちょっと大げさかな。

システムテストとしてシナリオの重要性がたかく、そこにおいて「インタラクションをテストする」という部分において、「ユーザー登録の確認」において「ユーザー登録の項目を隠ぺいする」とか「ユーザーログインの確認」において「ユーザーログインの項目を隠ぺいする」とかは、違うと思います。

searchTestCasesが以下のようになっている。対象のテストケースが表示されているページにない場合には空を返すが、その場合にページング処理がないので、失敗する。
このメソッドにページングを内包するか、別の仕組みを用意する必要がある。

    public TestCaseRow[] searchTestCases(String name, String scenario, String[] tags) {
        return testCaseItems.findAll {
            (name == "" || it.name == name) && (scenario == "" || it.scenario == scenario) && (tags == "" || it.tags.split("\n").toList().containsAll(tags))
        }
    }