On Mon, Aug 10, 2015 at 2:02 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> 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.
There are actually two ways to do this.
One model is the dummy_seclabel module. The build system arranges for
that to be available when running the core regression tests, so they
can use it. And the dummy_seclabel test does. There are actually a
couple of other loadable modules that are used by the core regression
tests in this kind of way (refint and autoinc, from contrib/spi).
The other model is what I'll call the test_decoding model. Since the
core code can't be tested without a module, we have a module. But
then the tests are in the module's directory
(contrib/test_decoding/{sql,expected}) not the core regression tests.
In general, I have a mild preference for the second model. It seems
more scalable, and keeps the core tests quick to run, which is
appropriate for more obscure tests that are unlikely to break very
often. But the first model can also be done, as show by the fact that
we have in fact done it several times.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
