やったこと

参考にしました: https://github.com/gin-gonic/examples/blob/master/basic/main_test.go
- github.com/stretchr/testify を入れるか迷ったが、入れない場合結局assert関数を自作することになるなぁ、と思い、じゃあ入れちゃうか、と思って入れた。
テストしやすくするためにrouterの処理をリファクタリング
対象のrouterにおける"/"へのテストを記述(=> main_test.go)
テストしてるときにmodule名が古いままだったので更新
テストしてるときにデバッグログがウザかったので、出さないように変更
- 環境変数から読み込めるようにしたいが、既存の作業とバッティングしそうなので、このPRでは行わない
説明資料としてwikiにtipsを追加した: https://github.com/sakaba-project/sakaba/wiki/%E7%B5%90%E5%90%88%E3%83%86%E3%82%B9%E3%83%88(routing%E3%81%AE%E3%83%86%E3%82%B9%E3%83%88)%E3%82%92%E6%9B%B8%E3%81%84%E3%81%9F%E3%81%A8%E3%81%8D%E3%81%AE%E6%89%8B%E6%B3%95
- なんかわかりにくい気がする。。すみません；；

review観点

[ ] 今後のテストの書き方のイメージが湧くか。自分でも今後書けそうか。
- イメージのスコープとしてはroutingの部分だけに限定させてください
[ ] テスト観点として、抜けがなさそうか
[ ] make testでテストが通るか

その他気になったこと気軽にコメント下さい！

naoi217 commented 4 years ago

指摘じゃなくてコメント

今後書くってなった時目線で考えた時に"Hello"の部分をテストデータみたいな感じで変数化してもいいかもね。可読性上がりそう。
あと、ドキュメント系とか今後初めてgoのテストを書く人向けにアサーションのドキュメントページのリンクとかをどっかに残しとくととっつきやすくなったりすると思います。(普段の業務で残してないから戒めを込めてのコメント)

shamisonn commented 4 years ago

@naoi217 コメントありがとうー！

今後書くってなった時目線で考えた時に"Hello"の部分をテストデータみたいな感じで変数化してもいいかもね。可読性上がりそう。

確かにパッと見だとexpected, actualがどっちかとかわからなそうな感じかも。やってみます！

ドキュメント系とか今後初めてgoのテストを書く人向けにアサーションのドキュメントページのリンクとか

ここらへんはどう残すか難しい感じしたけど、詳細に書いてみるね。具体的にはwikiにテスト時の手法のナレッジを書いて、リンクをREADMEから追えるようにしたいと思います！

todo

[x] テストコードを理解しやすいようにコメント加筆&変数化する
[x] テストコードをどう書いたのか、調査手法から含めて全部wikiにドキュメントとして残す

shamisonn commented 4 years ago

テストコードをどう書いたのか、調査手法から含めて全部wikiにドキュメントとして残す

を行いました :bow: 結構わかりにく感じになっちゃったかも；

https://github.com/sakaba-project/sakaba/wiki/%E7%B5%90%E5%90%88%E3%83%86%E3%82%B9%E3%83%88(routing%E3%81%AE%E3%83%86%E3%82%B9%E3%83%88)%E3%82%92%E6%9B%B8%E3%81%84%E3%81%9F%E3%81%A8%E3%81%8D%E3%81%AE%E6%89%8B%E6%B3%95

shamisonn commented 4 years ago

@naoi217

今後書くってなった時目線で考えた時に"Hello"の部分をテストデータみたいな感じで変数化してもいいかもね。可読性上がりそう。

ここの部分って https://github.com/sakaba-project/sakaba/pull/24/commits/d45618fe10c8b9fb4ef0585ddfad44fa4d95b1fc でイメージあってるかな？ちょっと違ってたら意見ほしいかも。。

naoi217 commented 4 years ago

修正ありがとう！ごめん、コメント見返して言葉足らずかんが半端なかった。例えば配列がexpectedになるときとかを想定してのコメントでexpectedBodySubStringだけ変数化するイメージだった！けど修正でわかりやすくはなったと思う。修正してくれたのを見て思ったこととしてはコメントだけでも理解できるかもと思った！

wikiとてもGodだと思いました。見ただけだけどわかった気になれたから調査の際の思考とかをログとして残すって大事だなと思った。

shamisonn commented 4 years ago

@naoi217 返信遅くてごめん；；見てくれてありがとう〜！

例えば配列がexpectedになるときとかを想定してのコメントでexpectedBodySubStringだけ変数化するイメージだった！

あー構造が変わるという意味合いだったか。ちょっと盲点だったかも！あざす！

修正してくれたのを見て思ったこととしてはコメントだけでも理解できるかもと思った！

OKまる:ok_hand: そしたらちょっとコード自体はスッキリさせて、コメントのほうで表現する形にmodifyしようと思います。

wikiとてもGodだと思いました。

あざます！！

shamisonn commented 4 years ago

@naoi217 せっかくだからPR自体のレビューもお願いしていいかな？？他のメンバーでわからんってなったらまたissue切る感じにしようと思う。

shamisonn commented 4 years ago

@naoi217 再レビューたのんだ(bow)

shamisonn commented 4 years ago

https://github.com/sakaba-project/sakaba/pull/24/commits/0b2cf61a517b05e6a0174c04d54b1fe143749858 で https://github.com/sakaba-project/sakaba/issues/11 の部分もちょっとやっちゃったけどこれスルーしちゃってください;;

あくまで最初に書いた"review観点"だけ見ていただく感じでオネシャス；；

sakaba-project / sakaba

add; routing test for main.go #24

やったこと

review観点