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)
