Re: [PATCH v2 01/18] t7107, t7526: directly test parse_pathspec_file()
From: Alexandr Miloslavskiy <hidden>
Date: 2019-12-19 17:19:28
On 18.12.2019 22:57, Junio C Hamano wrote:
quoted
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 indirectThat 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)