[HACKERS] Re: new tests post-feature freeze (was pgsql: Add TAP tests for pg_dump)
On Tue, May 24, 2016 at 08:19:20PM -0400, Robert Haas wrote: > On Sun, May 22, 2016 at 11:22 PM, Noah Misch wrote: > > Some or even most of the other tests would qualify under "closely related to > > ... a feature that is new in 9.6". Your 9.6 pg_dump changes affected object > > selection and catalog extraction for most object types, so I think > > validating > > those paths is in scope under Robert's suggestion. Testing "pg_dump > > --encoding" or "pg_dump --jobs" probably wouldn't fall in scope, because > > those > > features operate at arm's length from the 9.6 pg_dump changes. Expanding, > > for > > example, tests of postgres_fdw query deparse would certainly fall out of > > scope. That would have no apparent chance of catching a regression caused > > by > > the 9.6 pg_dump changes. > > ...although it might catch bugs in the deparsing logic, which was > heavily revised in 9.6. True. I cancel my last two sentences above; that was a weak choice of example, and the surviving sentences convey the message adequately. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: new tests post-feature freeze (was pgsql: Add TAP tests for pg_dump)
On Sun, May 08, 2016 at 12:29:27PM -0400, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: > > My suggestion is that, from this point forward, we add new tests to > > 9.6 only if they are closely related to a bug that is getting fixed or > > a feature that is new in 9.6. I think that's a reasonable compromise, > > but what do others think? +1. This is a natural extension of the well-established default that we (back-)patch tests for a bug into all releases getting a fix for the bug. > I'm willing to accept that compromise, but I'm not thrilled with it due > to what it will mean for the process I'm currently going through. The > approach I've been using has been to add tests to gain more code > coverage of the code in pg_dump. That has turned up multiple > pre-existing bugs in pg_dump but the vast majority of the tests come > back clean. This compromise would mean that I'd continue to work > through the code coverage tests, but would have to segregate out and > commit only those tests which actually reveal bugs, once those bugs have > been fixed (as to avoid turning the buildfarm red). The rest of the > tests would still get written, but since they currently don't reveal > bugs, they would be shelved until development is opened for 9.7. Some or even most of the other tests would qualify under "closely related to ... a feature that is new in 9.6". Your 9.6 pg_dump changes affected object selection and catalog extraction for most object types, so I think validating those paths is in scope under Robert's suggestion. Testing "pg_dump --encoding" or "pg_dump --jobs" probably wouldn't fall in scope, because those features operate at arm's length from the 9.6 pg_dump changes. Expanding, for example, tests of postgres_fdw query deparse would certainly fall out of scope. That would have no apparent chance of catching a regression caused by the 9.6 pg_dump changes. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: new tests post-feature freeze (was pgsql: Add TAP tests for pg_dump)
On 5/8/16 11:29 AM, Stephen Frost wrote: My suggestion is that, from this point forward, we add new tests to > 9.6 only if they are closely related to a bug that is getting fixed or > a feature that is new in 9.6. I think that's a reasonable compromise, > but what do others think? I'm willing to accept that compromise, but I'm not thrilled with it due to what it will mean for the process I'm currently going through. The approach I've been using has been to add tests to gain more code coverage of the code in pg_dump. That has turned up multiple pre-existing bugs in pg_dump but the vast majority of the tests come back clean. This compromise would mean that I'd continue to work through the code coverage tests, but would have to segregate out and commit only those tests which actually reveal bugs, once those bugs have been fixed (as to avoid turning the buildfarm red). The rest of the tests would still get written, but since they currently don't reveal bugs, they would be shelved until development is opened for 9.7. Having done extensive database unit testing in the past, I've experienced what Stephen's talking about and agree it's a real pain. With tap-style tests (or really anything that's not dependent on something as fragile as diffing output), there's pretty low risk to adding more tests that are passing. As for tests distracting people from reviewing patches, robust tests significantly reduce the need for manual review. I think it's a much better approach to take a methodical approach of writing tests to help verify a feature works than just randomly banging on it. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.com 855-TREBLE2 (855-873-2532) mobile: 512-569-9461 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: new tests post-feature freeze (was pgsql: Add TAP tests for pg_dump)
* Robert Haas (robertmh...@gmail.com) wrote: > Also, if we say that new tests are not features, that would mean that > they could be back-patched even after the release is out the door, and > generally I'm not in favor of that policy, except when we're adding a > test to a back-branch that is closely tied to a bug we are fixing in > that branch. I do not want to see the pg_dump test suite back-patched > to all active branches, for example, even though I approve of having > test coverage for pg_dump. I am confident that minimizing churn in > the back-branches is a more important goal than ensuring test coverage > for those branches, and that we will regret it if we reverse those > priorities. I agree that it doesn't make sense to back-patch large test suites which are primairly intended to provide code-coverage testing. In many cases, that would simply be duplicative without much gain as the code hasn't changed and tests would have to be removed due to features which don't exist in older branches, and there is certainly a risk there that it could complicate things for organizations which run the test suites and for packagers unnecessairly. > My suggestion is that, from this point forward, we add new tests to > 9.6 only if they are closely related to a bug that is getting fixed or > a feature that is new in 9.6. I think that's a reasonable compromise, > but what do others think? I'm willing to accept that compromise, but I'm not thrilled with it due to what it will mean for the process I'm currently going through. The approach I've been using has been to add tests to gain more code coverage of the code in pg_dump. That has turned up multiple pre-existing bugs in pg_dump but the vast majority of the tests come back clean. This compromise would mean that I'd continue to work through the code coverage tests, but would have to segregate out and commit only those tests which actually reveal bugs, once those bugs have been fixed (as to avoid turning the buildfarm red). The rest of the tests would still get written, but since they currently don't reveal bugs, they would be shelved until development is opened for 9.7. Thinking further on this, I'd probably end up creating a buildfarm animal which only reports to me which runs the full set of tests, so that any regression which occurs that the tests catch during the beta period is caught. Unfortunately, I'm not able to only work on and write tests only for bugs, as I don't know what the bugs are. Writing new tests using the test suite isn't really any more work than doing one-off testing, but it has lasting value that one-off testing doesn't. I don't mean to say "I accept the compromise but will do my own thing anyway" but rather to point out that this is an efficient and worthwhile way to find bugs and that's what I'm planning to work on to help ensure that we're ready to release. I'm happy to look at reviewing other committed patches also, but I've had my head in pg_dump for the past few months and have a pretty good handle on it, for the moment anyway, and there have certainly been lots of complaints over the years about our lack of test coverage for it. Were I to look at reviewing and testing other patches, well, at the moment I'd be pretty inclined to write test cases for those too, as it seems to be working to find issues. Thanks! Stephen signature.asc Description: Digital signature
