Re: [PATCH v2 01/18] t7107, t7526: directly test parse_pathspec_file()
- From
- Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
- Date
- Dec 19, 2019, 17:19 UTC
- Message-ID
- <0cf9d6a8-f804-c828-28f5-e60089190167@syntevo.com>
- In-Reply-To
- <xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com>
On 18.12.2019 22:57, Junio C Hamano wrote:
Show 18 quoted lines
>> 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)