Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

メンター権限を持ったテストユーザーyamadaをメンターだと分かる名前に変更 #4165

Conversation

eatplaynap
Copy link
Contributor

@eatplaynap eatplaynap commented Feb 7, 2022

issue

#4040

概要

メンター権限を持ったユーザーyamadaをメンターだと分かる名前に変更しました。

  • yamadaという名のメンター権限を持つテストユーザーを、こちらをメンターだと分かるmentormentaroという名前に変更しました。
  • 既存のテストに存在するyamadaを全てmentormentaroに変更しました。
  • test/models/card_test.rbtest/cassettes/customer/list.ymlにもyamadaという文字列が存在していますが、こちらのyamada"yamada@example.com"という文字列としてだけ使われているため、修正しませんでした。修正の必要がありそうでしたらご指摘ください🙏

変更確認方法

  1. ブランチ feature/rename-mentor-user-yamada-to-be-more-explicit-name-on-test-dataをローカルに取り込む
  2. $ bin/setup$ rails db:seedを実行するとDBの内容が開発環境に反映されます
  3. $ rails s でローカル環境を立ち上げます
  4. テスト用ユーザー('komagata'で確認しました)でログインします
  5. 検索ワードでmentormentaroと入力し、プロフィールページへのリンクが表示されmentormentaroというユーザーが確認できます

変更前

image

変更後

image

@eatplaynap eatplaynap self-assigned this Feb 7, 2022
@eatplaynap eatplaynap changed the title Feature/rename mentor user yamada to be more explicit name on test data メンター権限を持ったテストユーザーyamadaをメンターだと分かる名前に変更 Feb 7, 2022
@eatplaynap eatplaynap requested a review from Aseiide February 7, 2022 06:04
@eatplaynap eatplaynap marked this pull request as ready for review February 7, 2022 06:04
@Aseiide
Copy link
Contributor

Aseiide commented Feb 7, 2022

@eatplaynap
Reviewersを指定しただけだと、確信が持てないのでPRにコメントを付けて、その人にメンションを付けて「レビューお願いします」と言ってあげると親切でわかりやすいですね。
スクショのような感じで、PRを書き上げた後にもう1件コメントをつけるイメージですね。
スクリーンショット 2022-02-07 15 13 13

今日明日にはレビューします!

@eatplaynap
Copy link
Contributor Author

@Aseiide
ああ〜コメント!!すみません忘れていました🙏
ご指摘ありがとうございます!お手すきのときにレビューよろしくおねがいします〜!!

Copy link
Contributor

@Aseiide Aseiide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

動作確認とトミーさんが書き換えたテストが全てパスすることを確認しました。

test/models/card_test.rbとtest/cassettes/customer/list.ymlにもyamadaという文字列が存在しています

こちらも、mentormentaroへの変更を対応したほうがいいかなと思います。
なぜなら、ユーザーyamada自体が存在しなくなり、yamadaという文字列が残っていると、のちのちコードを読む人が「yamadaなんてなくない?」と混乱するかもしれないと思ったからです。
よろしくお願いいたします!

@Aseiide
Copy link
Contributor

Aseiide commented Feb 9, 2022

@eatplaynap レビューしました!

@eatplaynap eatplaynap force-pushed the feature/rename-mentor-user-yamada-to-be-more-explicit-name-on-test-data branch from 392f966 to 515d8dd Compare February 12, 2022 02:22
@eatplaynap
Copy link
Contributor Author

eatplaynap commented Feb 12, 2022

@Aseiide
レビューありがとうございます〜!下記2点に対応しましたので再確認お願いします🙏

  • test/models/card_test.rbとtest/cassettes/customer/list.ymlのyamadamentormentaroに変更
  • アバター画像名にもyamadaが残っていることに気づいたので、そちらもmentormentaroに変更

※テストが落ちていますが、落ちているのは下記issueで問題になっているテストで、本PRに起因するテストは通過していると言う認識です。

@eatplaynap eatplaynap requested a review from Aseiide February 12, 2022 04:41
Copy link
Contributor

@Aseiide Aseiide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eatplaynap レビュー遅くなりました。オッケーです!
mainのCIは通ってるっぽいんですよね。
今、CI回してみたんですが、落ちてますね〜。CIがPASSするまではkomagataさんのレビューに回せないので、CIで落ちている rails test test/system/comments_test.rb:236 がローカルでどうなるか確認お願いできますか??

@eatplaynap
Copy link
Contributor Author

@Aseiide
レビューありがとうございます😄
4時間前に#4203 がマージされたからmainで通るようになったみたいですね。
git pull origin mainして手元で実行してみたら通りました!CIでも通るまで実行し続けるしかないんでしょうか?:sob:

image

@Aseiide
Copy link
Contributor

Aseiide commented Feb 15, 2022

@eatplaynap
なるほど〜。ローカルでは通るんですね。
CIが若干flakyなので、他の作業しつつテスト回してもらうしかないかな〜と思います。
system/comments_test.rbmentormentaroでgrepしても引っかからないので、このPRの変更でCIが落ちていないというのは同意です。

自分も時間を置いたらPASSしてた、みたいなこともあるので、お手数なんですが、今日の間はPASSするまで通してもらうしかないですかね。

@Aseiide
Copy link
Contributor

Aseiide commented Feb 15, 2022

@eatplaynap CIパスしたので、komagataさんにレビューお願いして大丈夫です。

@eatplaynap
Copy link
Contributor Author

@Aseiide
よかったです:sob: レビューありがとうございました!

@komagata
お疲れさまです!チームメンバーのレビューOKが出たので、お手すきの際にレビューをお願いします:pray:

@eatplaynap eatplaynap requested a review from komagata February 15, 2022 23:01
Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認しました、OKですー🙆‍♂️

@komagata komagata merged commit cee864a into main Feb 16, 2022
@komagata komagata deleted the feature/rename-mentor-user-yamada-to-be-more-explicit-name-on-test-data branch February 16, 2022 03:53
@github-actions github-actions bot mentioned this pull request Feb 16, 2022
54 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants