テストコード by yuji91 · Pull Request #1 · runteq/ExamPostApp_Sorcery

yuji91 · 2021-04-16T14:47:27Z

No description provided.

DaichiSaito · 2021-04-19T06:17:48Z

+
+      # 処理結果の確認
+      expect(page).to have_content('User was successfully created.'), 'ユーザー作成のメッセージが表示されていません'
+      expect(current_path).to eq(login_path), 'ユーザー作成後にログイン画面に遷移できていません'


login_pathではなく/login（もしかしたらloginかも？）にしたほうが `visit 'users/new'と平仄があって良いんじゃないかな？

9e037d5 で対応しました！

DaichiSaito · 2021-04-19T06:18:48Z

+      click_on '登録'
+
+      # 処理結果の確認
+      expect(page).to have_content('User was successfully created.'), 'ユーザー作成のメッセージが表示されていません'


「ユーザー作成後に『User was successfully created.』というメッセージが表示されていません」みたいなほうが良さげ？

36e7fea でメッセージ確認部分の全般を修正しました！

DaichiSaito · 2021-04-19T06:34:26Z

+      click_on '登録'
+
+      # 処理結果の確認
+      expect(page).not_to have_content('User was successfully created.'), 'ユーザー作成のメッセージが表示されています'


これっているかな？
もし受講生が「ユーザーが作成されました。」みたいな日本語でフラッシュメッセージを設定してた場合ここ普通にすり抜けるのでいらないような気が。

メッセージを日本語で記載させても良いのか、scaffoldのデフォルトの英語のまま変えさせないのかは方針を決めておいた方が良さそうですね！

現状では全部のエラーメッセージの確認部分でscaffoldのデフォの英語のままであることを前提としてテストコードを作っています。

僕はメッセージの確認を行うならテストの都合上「scaffoldで作成されるメッセージは変えずに、英語のままにしてください」と言う注意書きを１つ加えておいて運用するでも良いのかな、と思いました。

今回の確認範囲のメインはログイン、ユーザー登録とPostとの関連がメインで日本語化は主題ではないので、あまり意識させずにメッセージの確認は残しておくことでちゃんとコピペじゃなくてscaffoldなどのrails generateを使いこなすことを求めても良いのかな、と思っています。

あれ、これってscaffold使わせないよね？使わせないのに「User was successfully created.」みたいなscaffoldデフォのメッセージを実装させるのって結構不毛じゃない？（この指摘部分じゃなくて全体に関わる話になっちゃうけど）

確かに、今回だとscaffold使うところはなくやるとしてもrails g controllerでControllerとViewファイル作るだけなので、メッセージの実装は強制しない形にしようと思います😅

その場合どうやってユーザー作成に成功したことを確認する？メッセージを実装していた目的はたぶんユーザーがちゃんと作成されてることを確認するためだったと思ってて、となると単純にそれを削除しちゃっていいのかが気になりました！

よくやるのはこういうやつなんだけど、変更する負荷高いかな？

expect { fill_in ~~~ click_on ~~~ }.to change(User.size).by(1)

3563faf のコミットでUser.countを以下の様に確認する形で対応しました！🙆
}.to change { User.count }.by(1)

あざっす〜。sizeじゃなくてcountのほうが適切ぽいね。ありがとう。

DaichiSaito · 2021-04-19T06:37:13Z

+      expect(page).to have_css("label[for='email']"), 'Email というラベルをクリックすると対応するフィールドにフォーカスすることを確認してください'
+      expect(page).to have_css("label[for='password']"), 'Password というラベルをクリックすると対応するフィールドにフォーカスすることを確認してください'


ここむずいっすねー。
form_with scope: :session とかで書かれるとテストが通らなくなる。そしてscope: :sessionって書くのは間違いではない。

最終的には/loginでuser_sessions#newに遷移するようにパスを指定してlogin_pathで視認性を上げて開発するのが理想かと思うので、login_pathを使うことを要件として要求しても良いのかな、と思いました。

そうすれば<%= form_with url: login_pathと書く部分を<%= form_with scope: :sessionと記載されてテストが通らない事例が発生しても、
質問とかで「scopeで記載しても動作は問題ないですが、今回は要件に指定があるので慣例通りlogin_pathを使って実装してみましょう！」みたいな対応記録を残しておけば良いのかな、と思いました！

いや、scope: :sessionを書くと

name=session[email]

のようになるので

expect(page).to have_css("label[for='email']"),

が通らなくなるよって話です！

scope: :sessionを記載した形だと以下の２点が発生する認識ですかね、

Controller側ではparams[:session][:email]などの形でパラメータを受け取る必要がある

labelについてsession_emailの様な値になり、テストコードが通らない

上記の形が望ましいとも思えないので、課題要件でscope: :sessionと書くと満たせない様な要件を設定してしまうのはどうでしょうか？
（「ログイン画面のform_withではurl: login_pathの形で引数を指定してください」など）

「ログイン画面のform_withではurl: login_pathの形で引数を指定してください」など

urlオプションとscopeオプションは共存できるのでその案内はちょい違和感あるかな〜。

当初伊藤くんが提案してくれた通り

そうすれば<%= form_with url: login_pathと書く部分を<%= form_with scope: :sessionと記載されてテストが通らない事例が発生しても、
質問とかで「scopeで記載しても動作は問題ないですが、今回は要件に指定があるので慣例通りlogin_pathを使って実装してみましょう！」みたいな対応記録を残しておけば良いのかな、と思いました！

↑ でいきますかー。

ちなみに個人的にはsessionというキーでグルーピングするほうがむしろ望ましいかなぁとは思ってて、基礎編の実装ミスったなぁと感じてる笑

おけです、では以下の対応で良いですかね？

ログイン用のパスはlogin_pathを使って実装する指示を課題要件に記載する

scope: :sessionで実装した形でテストが通らない場合は、質問してもらってそこで対応して記録を残す

おｋっす〜

改めて考えたのですが、scopeはリクエストパラメータに渡す情報をparams[:sessions]の様に階層で分けたい場合に使うことがある、ということですよね。
https://apidock.com/rails/ActionView/Helpers/FormHelper/form_with

そこでscopeを設定されてしまうとlabel[for='email']の値がずれてしまう。

これまでの流れを整理しました。

「labelとフィールドの対応付け確認」をテストコードから省略しない場合は、scopeを設定されると自動テストが通らず、生徒のローカルでは問題なくログインできるという現象が発生する。

「labelとフィールドの対応付け確認」をテストコードから省略すれば、生徒側でscopeを自由に設定しても自動テストは成功する。

観点：「labelとフィールドの対応付け確認」はテストコードから省略すると問題になるか？
--> 省略した場合、課題要件で「ラベルをクリックして対応する入力フォームがアクティブになることを確認してください」という形になる。見逃されたら「ローカルでログインできるのにテストが通らない」とか言われるので、ここは自動テストで指摘できる形で残しておいた方が良さそう。

--> 残すのであれば「scopeを設定すると自動テストが失敗する」ことをどこかで伝えなくてはならない
--> 課題要件ではなく、質問で投稿してもらってそれを他の生徒が見られるように記録する。（今までの話の流れ）

--> 最初から課題要件で「自動テストの都合上、ログインフォームのform_withのscopeは設定しないで実装してください」と指示しておいた方が親切？

--> 質問として記録するのは良いとして、あらかじめ課題要件で記載してあげた方が良い気がしました！

めちゃわかりやすいまとめ笑

--> 質問として記録するのは良いとして、あらかじめ課題要件で記載してあげた方が良い気がしました！

そうすね〜。それでいきましょか！

DaichiSaito · 2021-04-19T06:37:40Z

+
+      # 処理結果の確認
+      expect(page).to have_content('Login successful.'), 'ログイン成功のメッセージが表示されていません'
+      expect(current_path).to eq(posts_path), 'ログイン後に投稿一覧画面に遷移できていません'


ここも/postsにすべきかな？

9e037d5 で該当箇所について修正しました！

DaichiSaito · 2021-04-19T06:41:29Z

+      click_on 'Edit'
+
+      # labelの存在確認
+      expect(page).to have_content('Title'), 'Title というラベルが表示されていることを確認してください'


expect(page).to have_selector 'label', text: 'Title'

の方がいいんかね

bdd9a53 で全体的に修正しました！

DaichiSaito · 2021-04-19T06:49:50Z

+RSpec.describe "Posts", type: :system do
+  describe '確認観点4：投稿機能' do
+    let!(:user){ create(:user) }
+    let(:another_user){ create(:user, email: 'another_user@example.com') }


emailの指定いる？

Fakerで毎回ランダム作成で被ることがなかったので不要でした！

7ff5cae で削除しました。

yuji91

@DaichiSaito
指摘事項について対応しました！

２点対応を保留しているものがありますが、自分でもどうすべきか考えてみたので、以下相談させてくださいm(_ _)m
#1 (comment)

#1 (comment)

yuji91 · 2021-04-22T05:28:02Z

+      click_button 'ログイン'
+
+      # 処理結果の確認
+      expect(page).to have_content('Login successful.'), 'ログインの成功時に『Login successful.』のメッセージが表示されていません'


ログイン直後の画面は以下の様になるのですが、ここでフラッシュメッセージ以外の確認方法としては

current_pathが/postsであること

Posts一覧画面の内容がpageとして表示されること

右上のアイコンからログアウトへのリンクが表示されること
上記の３パターンがあるかと思っています。
current_pathの確認は直下のexpectで実施しているため、他の観点を確認項目としてわざわざ足すことはしなくても良いかな、と思っているのですがどうでしょうか🤔

3563faf でフラッシュメッセージ以外の確認方法をそれぞれのテストケースに追加しました！
これで問題なければ、フラッシュメッセージ関連のexpectは削除しようと思います。

ログイン直後の画面は以下の様になるのですが、ここでフラッシュメッセージ以外の確認方法としては

current_pathが/postsであること

Posts一覧画面の内容がpageとして表示されること

右上のアイコンからログアウトへのリンクが表示されること
上記の３パターンがあるかと思っています。
current_pathの確認は直下のexpectで実施しているため、他の観点を確認項目としてわざわざ足すことはしなくても良いかな、と思っているのですがどうでしょうか🤔

1だけで良さそうすね！

yuji91

@DaichiSaito
以下２点、対応方針に問題ないかコメントお願いしますm(_ _)m
#1 (comment)
#1 (comment)

yuji91 · 2021-04-22T07:34:56Z

@DaichiSaito
２点コメントしたので確認をお願いします！
#1 (comment)
#1 (comment)

DaichiSaito · 2021-04-23T06:15:05Z


      # 処理結果の確認
-      expect(page).to have_content('Login successful.'), 'ログインの成功時に『Login successful.』のメッセージが表示されていません'
+      expect(current_path).not_to eq('/login'), 'ログイン処理が正しく行えるかを確認してください'


一行下のexpectがあればこれは不要？

ちょっと迷ったんですが、他の観点でも

ログインが正しくできるか or 正しく失敗するか
-->「ログイン成功の場合/loginから変わったか」「ログイン失敗の場合/postsに移動していないか」で判断

「ログイン成功時に/postsに正しく画面遷移出来ているか」「ログイン失敗時に/loginから遷移されていないか」を確認

と言う段階を分けて確認するつもりで書いていました。
これはまとめちゃっても良いんじゃないか、と言うことですかね？

なるほどー。
じゃぁ今のままで良いか！

Test convert to english

Add spec

ab2203d

DaichiSaito reviewed Apr 19, 2021

View reviewed changes

yuji91 added 4 commits April 19, 2021 17:53

Fix: 不要なemailの指定を削除

7ff5cae

Fix: Label確認用マッチャを修正

bdd9a53

Fix: pathの確認は文字列で行う

9e037d5

Fix: メッセージ確認失敗時の表示文言を修正

36e7fea

yuji91 commented Apr 19, 2021

View reviewed changes

Fix: SyntaxError解消

4cd483e

yuji91 commented Apr 22, 2021

View reviewed changes

Fix: フラッシュメッセージ以外の確認観点を追加

3563faf

yuji91 commented Apr 22, 2021

View reviewed changes

yuji91 added 5 commits April 22, 2021 19:28

Fix: フラッシュメッセージを確認観点から除外

4671d01

Fix: タイポ修正

bc63c69

Merge branch 'master' into test

b13f633

Merge branch 'master' into test

c882dcb

Merge branch 'master' into test

e5de7b1

DaichiSaito reviewed Apr 23, 2021

View reviewed changes

yuji91 added 6 commits April 23, 2021 15:44

Merge branch 'master' into test

a5efd01

Merge branch 'master' into test

659c9c9

Apply headless_chrome

ebeec70

Add: 複数ユーザー登録時に挙動を確認するテストを追加

8463e3a

Modify: Login, Logoutボタンを英語表記にした

232b74c

Modify: Create Userボタンを英語表記にした

d7f653b

yuji91 added 3 commits March 22, 2022 19:35

Modify: Logoutボタンを英語表記にした

7cea1cd

Modify: Logoutボタンを英語表記にした

78f6ae4

Merge pull request #8 from runteq/test_convert_to_english

962cb33

Test convert to english

		expect(page).to have_css("label[for='email']"), 'Email というラベルをクリックすると対応するフィールドにフォーカスすることを確認してください'
		expect(page).to have_css("label[for='password']"), 'Password というラベルをクリックすると対応するフィールドにフォーカスすることを確認してください'

Conversation

yuji91 commented Apr 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuji91 Apr 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DaichiSaito Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuji91 Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuji91 Apr 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuji91 Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuji91 Apr 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuji91 left a comment

Choose a reason for hiding this comment

Uh oh!

yuji91 Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuji91 left a comment

Choose a reason for hiding this comment

yuji91 Apr 19, 2021 •

edited

Loading

DaichiSaito Apr 22, 2021 •

edited

Loading

yuji91 Apr 22, 2021 •

edited

Loading

yuji91 Apr 19, 2021 •

edited

Loading

yuji91 Apr 22, 2021 •

edited

Loading

yuji91 Apr 19, 2021 •

edited

Loading

yuji91 Apr 22, 2021 •

edited

Loading

yuji91 Apr 23, 2021 •

edited

Loading