From: Alexandr Miloslavskiy Date: Thu, 19 Dec 2019 17:19:21 GMT Subject: Re: [PATCH v2 01/18] t7107, t7526: directly test parse_pathspec_file() Message-ID: <0cf9d6a8-f804-c828-28f5-e60089190167@syntevo.com> In-Reply-To: On 18.12.2019 22:57, Junio C Hamano wrote: >> In my previous patches, `parse_pathspec_file()` was tested indirectly by >> invoking `git reset` and `git commit` with properly crafted inputs. This >> has some disadvantages: >> 1) A number of tests are copy&pasted for every command where >> `--pathspec-from-file` is supported. With just two commands, it >> wasn't too bad, but I'm going to extend support to many more >> commands, which would make a handful of low-value tests. >> 2) Tests are located in suboptimal test packages >> 3) Tests are indirect > > That cuts both ways. For a developer who is too narrowly focused > (because s/he spent enough time staring at the code), testing the > underlying machinery in a more direct way does feel attractive, but > at the same time, what matters to the end users is how well the > feature, when integrated into the commands they use (not the test > scaffolding like the "test-parse-pathspec-file" command), works. > > So "indirect" is not necessarily a bad thing. I agree that it cuts both ways. Just recently I had an (unrelated) discussion with Johannes Schindelin who forced me to drop 2 of 3 tests (where #3 also by chance covered #1 #2) in some other PR, because too many tests is also evil. To verify: I see 13 git commands that could benefit from --pathspec-from-file. There are 6 tests that in fact test underlying machinery, which can't be easily influenced by bugs in command's code. That makes 12*6 = 72 tests that are copy&pasted and doesn't test anything new. Do you suggest to return _all_ tests back into every command? (but also keep the new direct tests, I assume)