Robert, * Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Jan 26, 2018 at 11:56 AM, Stephen Frost <sfr...@snowman.net> wrote: > >> I think you've chosen a terrible design and ought to throw the whole > >> thing away and start over. > > > > I'll all for throwing away the existing test once we've got something > > that covers at least what it does (ideally more, of course). > > I'm for throwing this away now. It's a nuisance for other people to > maintain, and as Tom's reply makes clear (and it matches my > suspicions), they are maintaining it without really knowing whether > the updates are making are *correct*, just knowing that they *make the > tests pass*. It's nice to make things turn green on the code coverage > report, but if we're not really verifying that the results are > correct, we're just kidding ourselves. We'd get the same amount of > green on the code coverage report by running the pg_dump commands and > sending the output to /dev/null, and it would be a lot less work to > keep up to date.
There's not much to argue about if committers are simply hacking away at it without actually considering if the test are correct or not. I suspect it's not quite as bad as being outright wrong- if individuals are at least testing their changes thoroughly first and then making sure that the tests pass then it's at least more likely that what's recorded in the test suite is actually correct, but that's not great. Further, having contributors include updates to the test suite which are then willfully thrown away is outright bad. If that's worse than not having this test coverage for pg_dump, I'm not sure. What strikes me as likely is that removing this test from pg_dump will just result in bugs being introduced because, for as much as updating these tests are painful, actually testing all of the different variations of pg_dump by hand for even a simple change is quite a bit of work too. I don't have any doubt that the reason bugs were found with this is because there were permutations of pg_dump that weren't being tested, either by individuals making the change or through the other regression tests we have. Indeed, the tests included were specifically to get code coverage of cases which weren't already being tested. > I'm glad this helped you find some bugs. It is only worth keeping if > it prevents other hackers from introducing bugs in the future. I > doubt that it will have that effect. I'm not convinced that it won't, but I agree that we want something that everyone will feel comfortable in maintaining and improving moving forward. I'll work on it and either submit a rework to make it easier to maintain or a patch to remove it before the next CF. Thanks! Stephen
signature.asc
Description: PGP signature
