geb-template's People
geb-template's Issues
Filter部分のcontentが複雑すぎる
もしかして、アプリケーションがうまくデプロイされていないのかもしれませんが、板垣さんにご指摘いただいたので、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がシナリオとしてすばらしいのであれば、そうであるべきであるという主張をなんらかの形ですべきです。
いまの形では「テストコードを書いた人が言いやすかったから」というだけで、なんのメリットもありません。誤解のもとにしかならないです。
header#openMenuTestCaseはあまりすきじゃないですー。
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)とするほうが自然かなぁと思いました。
UserRegistration#ユーザー削除練習用のthenが意味をなしていない。
以下のコードはなにもしていないです。やめてください。
then:
true
「緩い統合テスト」しかない。
システムレベルのテストをしているのに、「こういったケースで使いにくそう」など、改善を提案するテストがまるでありません。
また、シナリオとして実装されていないとおかしいと思われるケースもありません。
これでは、バグを見つけることもできないでしょうし、テスト戦略やテスト設計がなっていないと言わざるをえません。
まとめると、「緩い統合テスト」でしかありません。
好みの問題ですが、コンテンツにtoを寄せていても特定シナリオではatを呼び出したいときについて
これは好みの問題なので無視して大丈夫です。
おそらくSeleniumのPageObjectPatternとも関連があると思うのですが、コンテンツに(to:FooPage)とすることで、そのコンテンツをclickしたときにFooPageのtoを呼び出すのと同等の効果がえられるように書けます。いまだと、Module系でそれを実装している例が見れます。
これをすると、テストコード側ではおおよそat呼び出しがいりません。(toされているし、すでにpageにはFooPageがはいっているから)
ですが、僕はわざとat FooPage と書きたい場合があります。それはドキュメンテーションとして書きたい時です。
(to:FooPage)をつけるようなコンテンツにフォーカスしたシナリオではおそらく「ある画面に遷移した」ということを明示的に書きたいのですが、そうではないシナリオではそれほど重要ではないのでat FooPageは必要ないです。
おそらくこれはJavaのWebDriverでは、コンテンツ選択の結果であるpageを変数として受け取って、その変数の型が該当ページのクラス名になることによって、ドキュメンテーションの役割を担っていました。
Gebでそういった書き方をしないのであれば、atを付けたほうがよりよりシナリオと、そうでないシナリオがあるかなぁと思いました。
TestCasePage#searchTestCasesで空文字と比較しているのはバグだと思われる。
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の呼び出しがきもい
Userクラスでat呼び出しをすると、テストメソッドで呼び出した後にどの画面で次の操作やthenを行っているのかわからなくなって、Test as Documentの考え方からはあまり同意できません。
TagPage#addTag と TagPage#createTag があって命名がわるい
addTagでは登録し、createTagでは登録しログの検査をしている。明らかにメソッドの責務が名前と乖離している。
Userクラスに管理者の機能が存在しないほうがいい
現在の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の正常フローを最低限保証する、セキュリティー攻撃、基盤異常などによる業務停止リスクはいったん対象外とする)元となるユースケースを記述する。
isTestCaseCreationSuccessfulのようなisXXXの使われ方が微妙かなぁ。
まず、isXXXと書くのは、対象オブジェクトに問い合わせをしている場合だと思います。
いまのテストコードで対象オブジェクトにたいして問い合わせをするということは結構不自然かなぁと思いました。
例えば次のようなコードはたくさんあって、ちょっと違和感。
then:"Test Case詳細画面に正常登録のメッセージが表示される"
isTestCaseCreationSuccessful()
// -> 訳すと、テストケース作成は成功した?
thenのドキュメントからもわかるように宣言をしているわけですから、次のほうが自然かな。
then:"Test Case詳細画面に正常登録のメッセージが表示される"
TestCaseCreationIsSuccessful()
// -> 訳すと、テストケース作成は成功する
みたいな。
UserRegistration#ユーザー削除練習用 のwhenがシナリオとして意味がない
シナリオパッケージにあるなら、意味のあるシナリオにすべきです。
これをユーザーが意味のあるシナリオとして実行するなら、むしろなにか機能改善を促すようなシナリオかと思います。
シナリオではなく、例えばテスト環境を調整するためのメソッドであるならば、もっと別のパッケージしたり、コメントをつけるなど配慮すべきです。
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)
}
TestCasePageで配列を積極的に使う理由がわからない
searchTestCases、updateTestcaseで配列を使っていますが、特に理由がなければListを使うべきです。
ページオブエジェクトで削除ボタンがモジュールとは別に定義されている
ページオブジェクトでモジュールを利用して、ユーザーの一覧やテストケースの一覧のテーブル部分をリストとして扱っていますが、各項目(例えばテストケース)の削除ボタンがモジュールではなく、ページオブジェクトのコンテンツでリストとして保持されています。これはTestCaseの例です。
static content = {
// 略
testCaseItems { moduleList TestCaseRow, $("#testCaseRow") } // ここでテーブル部分の一行をリストとして保持している
editButtons { $(".glyphicon-eye-open") } // テーブルの1行に含まれている項目なのにtestCaseItems経由ではないアクセス!
deleteButtuns(required: false) { $(".glyphicon-remove") } // テーブルの1行に含まれている項目なのにtestCaseItems経由ではないアクセス!
}
これは、あまりに責務分割ができていなさすぎると思います。TestCaseRowモジュールの中でeditButtonやdeleteButtonを実装すべきです。直してください。
登録系とログインの抽象化があまり適切ではないと思われる。
以前のハンズオンで「登録することが重要な場面では、実際のインタラクションになる各項目を羅列すべきです。ですが、別のことにフォーカスしているシナリオにおいて、例えば前提条件の作成で登録が必要な場面では隠蔽して構わないと思います。」とお伝えした通りです。
例えば、Userオブジェクトをまるっとわたして登録なりログインなりしていますが、それでは「実環境のユーザーに最初にどのくらいの項目が必要とされているのか?」といったことが隠蔽されてしまいます。つまり、シナリオによって明確になる利用品質のテストを無視しています。
それでよい場合もありますが、それは概ね別のテストだと思います。
こういったテストケースを書いていて問題になるのは
「え!最初はTwitterアカウントだけでログインできると思ったのに、これとこれとこれもいるの?テストしていないの?」
「いえ、テストは書いています。ただし全ての情報をインプットしていました」みたいになります。(ちょっと大げさかな。
システムテストとしてシナリオの重要性がたかく、そこにおいて「インタラクションをテストする」という部分において、「ユーザー登録の確認」において「ユーザー登録の項目を隠ぺいする」とか「ユーザーログインの確認」において「ユーザーログインの項目を隠ぺいする」とかは、違うと思います。
TestCasePage#searchTestCases がページングに対応していなく、利用しているコードでおそらく失敗する
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))
}
}
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.