From: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
---|---|
To: | Noah Misch <noah(at)leadboat(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Test code is worth the space |
Date: | 2015-08-10 06:02:17 |
Message-ID: | CANP8+jKxeHGR_+rHqwT1daLFeGqbqCpAAXoGSxd5+J2QzNW-gg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 8 August 2015 at 17:47, Noah Misch <noah(at)leadboat(dot)com> wrote:
> We've too often criticized patches for carrying many lines/bytes of test
> case
> additions. Let's continue to demand debuggable, secure tests that fail
> only
> when something is wrong, but let's stop pushing for test minimalism. Such
> objections may improve the individual patch, but that doesn't make up for
> the
> chilling effect on test contributions. I remember clearly the first time I
> submitted thorough test coverage with a feature. Multiple committers
> attacked
> it in the name of brevity. PostgreSQL would be better off with 10x its
> current test bulk, even if the average line of test code were considerably
> less important than we expect today.
>
Almost every patch I review has either zero or insufficient tests.
If we care about robustness, then we must discuss tests. Here are my two
recent experiences:
I agree we could do with x10 as many tests, but that doesn't mean all tests
make sense, so there needs to be some limiting principles to avoid adding
pointless test cases. I recently objected to adding a test case to
Isolation tester for the latest ALTER TABLE patch because in that case
there is no concurrent behavior to test. There is already a regression test
that tests lock levels for those statements, so in my view it is more
sensible to modify the existing test than to add a whole new isolation test
that does nothing more than demonstrate the lock levels work as described,
which we already knew.
On another review I suggested we add a function to core to allow it to be
used in regression tests. A long debate ensued, deciding that we must be
consistent and put diagnostic functions in contrib. My understanding is
that we are not allowed to write regression tests that use contrib modules,
yet the consistent place to put diagnostic functions is contrib -
therefore, we are never allowed to write tests utilizing diagnostic
functions. We are allowed to put modules for testing in another directory,
but these are not distributed to users so cannot be used for production
diagnosis. Systemic fail, advice needed.
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2015-08-10 06:03:02 | Summary of plans to avoid the annoyance of Freezing |
Previous Message | Amit Kapila | 2015-08-10 05:09:38 | Re: Priority table or Cache table |