asya81 / ruby-practices

フィヨルドブートキャンプのRubyプラクティスの提出物をまとめるリポジトリ
0 stars 0 forks source link

refs #32 lsコマンド3を実装 #33

Closed asya81 closed 1 year ago

asya81 commented 1 year ago

refs #32 lsコマンド3を実装しました。

asya81 commented 1 year ago

@kfukai23 さん、コメントありがとうございます。 いただいたコメントについて、理解できていない部分があるので質問させてください。

前回のlsコマンド2のレビューで、get_objectsに対するテストは内部構造に関するテストになっており、テスト不要とのことでした。 今回、rオプションが指定された場合のテストを追加しようと試みたのですが、その場合、 get_objectsに対するテストをすることになり、前回のlsコマンド2で指摘されたのと同様、 内部構造に関するテストになると考え、テストを追加しませんでした。

ご質問したいこと

  1. rオプションのテストを追加する場合、内部構造に対するテストには該当しないということでしょうか?
  2. テスト内容としては、get_objectsにrオプションを渡した場合、逆順でオブジェクト一覧が返ってくること、で問題ないでしょうか?

以上、よろしくお願いします。

kaito0046 commented 1 year ago
  1. rオプションのテストを追加する場合、内部構造に対するテストには該当しないということでしょうか? いえ、そういうことではありません。単に新しい機能(仕様)をプログラムに追加したので、そのテストも必要だということです。 前回は、get_objects に対するテストと、aオプションを指定した際に、隠しファイル・ディレクトリも含めて表示されること のテストを追加され、僕は「後者のみでよい」とお伝えしました。前者は内部構造に対するテストであり他の処理から利用される(=他のテストが通っていれば壊れていないと言える)、から不要で、後者は必要、という話をしたと思います。 rオプションのテストは、前回で言うところの後者のほうに該当すると思いませんか?:eyes:

  2. テスト内容としては、get_objectsにrオプションを渡した場合、逆順でオブジェクト一覧が返ってくること、で問題ないでしょうか?

そうですね、結論から言うといったんそれでよいと思います( ちなみに glob_objects ですかね?)。 理想を言えば、テスト対象のメソッドを main にしてしまいたいところかな、と思います。というのも、今回のプログラム全体で見ると、glob_objectsmain から利用される処理であって、どちらかというと内部構造っぽいものだからです。 結局、今回の課題では「プログラム全体が、特定の入力やオプションの場合に、それに応じた出力がされること」をテストしたいので、それには glob_objects や現状テスト対象になっている list_objects だけをテストの対象とするのではなく、処理全体を司る main をテスト対象にしたほうがよいのではないかな、と思った次第です。 ...が、現状 list_objects に条件に応じたカレントディレクトリの内容を引数で渡しているので、これをmainに変えるには、引数で渡す以外の方法でカレントディレクトリの内容を条件ごとに切り替える必要があります。これって結構大変だと思うんですよね...(もし、ruby ls.rb ~/foo のように表示対象をコマンドライン引数で渡すようになっていればそれを利用してうまくやれるかもしれませんが、これは歓迎要件なので、テストのためにそこまでするのも...という感じです:sweat:) なので、いったんは冒頭に書いたとおりでいいかな、と思います。

asya81 commented 1 year ago

@kfukai23 ご回答いただき、ありがとうございます!

  1. のテスト追加が必要な理由について 基本的なお話として、新しく追加した機能に対するテストを書きましょう、ということと理解しました。 前回は、新しく追加した機能について2パターンテストを追加していたため、内部構造へのテストになってしまっているテストは不要だったということですね 🙆

  2. のテスト内容について (すみません、完全に間違えてました。glob_objectsでした。。) 現状は、glob_objectsに対するテストでも一旦OKとのこと、承知しました。

プログラムの構造(設計)を変えれば、内部構造へのテストではないテストの書き方(main自体のテスト)ができるが、 歓迎要件の範囲になるため、そこまで対応しなければならないというものではない、と理解しました。

今回は、glob_objectsに対するテストとして追加して、この後進めていく中で理解を深めて、機会があればテストや設計を見直してみたいと思います。ありがとうございます!

asya81 commented 1 year ago

@kfukai23 さん、テストを追加しました。 再度レビューをお願いいたします。

asya81 commented 1 year ago

PRレビューOKだったため、クローズします。