Re: [COMMITTERS] pgsql: Add TAP tests for pg_dump

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Sun, May 8, 2016 at 6:44 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > I do think that now is a good time for people to be reviewing what's
> > been committed, which includes writing tests (either automated ones,
> > which can be included in our test suite, or one-off's for testing
> > specific things but which don't make sense to include).
>
> And what if the tests are buggy? New test suites should go through a
> CF first I think for proper review. And this is clearly one.

They still won't result in data loss, corruption, or other direct impact
on our users, even if they are buggy.  They also won't destablize the
code base or introduce new bugs into the code.  Those are the things we
need to be concerned about when it comes to introducing new features
into the system and why we take a break from doing so before a release.

> > Regarding when we should stop adding new tests, I can't agree with the
> > notion that they should be treated as new features.  New tests may break
> > the buildfarm, but they do not impact the stability of committed code,
> > nor do they introduce new bugs into the code which has been committed
> > (if anything, they expose committed bugs, as the set of tests we're
> > discussing did, which should then be fixed).
>
> Yes, new tests do not impact the core code, but they could hide
> existing bugs, which is what I think Peter is arguing about here, and
> why it is not a good idea to pushed in a complete new test suite just
> before a beta release. The buildfarm is made so as a run stops once of
> the tests turns red, not after all of them have run.

I disagree that new tests are going to hide existing bugs.  The case
which I believe you're making for that happening is where the buildfarm
is red for an extended period of time, but I've agreed that we don't
want the buildfarm to be red and have said as much, and that's different
from the question about adding new tests and simply means the same thing
it's always meant- anything which makes the buildfarm red should be
addressed in short order.

> > As such, I'd certainly encourage you to propose new tests, even now, but
> > not changes to the PG code, except for bug fixes, or changes agreed to
> > by the RMT.  How you prioritise that with the release preparation work
> > is up to you, of course, though if I were in your shoes, I'd take care
> > of the release prep first, as we're quite close to doing a set of
> > releases.  I'm happy to try and help with that effort myself, though
> > I've not been involved very much in release prep and am not entirely
> > sure what I can do to help.
>
> Adding new tests that are part of an existing bug fix is fine because
> the failure of such a test would mean that the bug fix is not working
> as intended. Based on my reading of the code this adds test coverage
> that is larger than the basic test for a bug fix.

Yes, which is an all-together good thing, I believe.  We need more and
better test coverage of a *lot* of the system and this is just the start
of adding that coverage for pg_dump.

There is pretty clearly a lack of consensus on the question about adding
new tests.  It seems like it'd be a good topic for the dev meeting, but
I don't think everyone involved in the discussion so far is going to be
there, unfortunately.  I'm open to still proposing it as a topic, though
I dislike having to leave people out of it.

I'm not quite sure what the best way to put this is, but I can't help
but feel the need to comment on it in at least some way:

What *should* we be doing right now?  Reviewing code and testing the
system is what I had thought but perhaps I'm wrong.  To the extent that
we're testing the system, isn't it better to write repeatable tests,
particularly ones which add code coverage, than one-off tests that only
check that the current snapshot of the code is operating correctly?  I
feel like that's what we should be encouraging, but this position is
suggesting that we should be doing that independently and holding all of
those new tests until the next commitfest, which is the same as we're
asked to do for new development at this point.  Perhaps it's not ideal,
but I feel it's necessary to point out that between writing code
coverage tests and doing new development, other things being equal, you
can guess which would be preferred.  I'd rather we encourage more of the
former rather than acting as if they should be treated the same during
this phase of the development cycle.

Thanks!

Stephen