Andres Freund <andres@anarazel.de> writes:
> On 2017-03-20 10:35:15 -0400, Stephen Frost wrote:
>> I continue to be of the opinion that this entire discussion is quite
>> flipped from how we really should be running things- adding regression
>> tests to improve code coverage, particularly when they're simply adding
>> to the existing structure for those tests, should be strongly encouraged
>> both before and after feature-freeze.
> I don't think posting a preliminary patch, while continuing to polish,
> with a note that you're working on that and plan to commit soon, would
> slow you down that much. There's pretty obviously a difference between
> an added 10 line test, taking 30ms, and what you did here - and that
> doesn't mean what you added is wrong or shouldn't be added.
New tests are not zero-cost; they create a distributed burden on the
buildfarm and, by increasing the buildfarm cycle time, slow down feedback
to authors of subsequent patches. So I'm very much not on board with
any argument that "more tests are always better and don't even require
discussion".
I'd have liked to see this patch posted with some commentary along the
lines of "this improves LOC coverage in pg_dump by X%, and for me it
increases the time taken for 'make installcheck' in bin/pg_dump by Y%".
Assuming Y isn't totally out of line with X, I doubt anyone would have
objected or even bothered to review the patch in detail ... but it would
have been polite to proceed that way.
In short, I agree with Stephen's position that test additions can get
away with less review than other sorts of changes, but I also agree with
Robert's position that that doesn't mean there's no process to follow
at all.
regards, tom lane
