Обсуждение: [HACKERS] Thoughts on unit testing?

Hi hackers,

The current regression tests, isolation tests and TAP tests are very
good (though I admit my experience with TAP is limited), but IMHO we
are lacking support for C-level unit testing.  Complicated, fiddly
things with many states, interactions, edge cases etc can be hard to
get full test coverage on from the outside.  Consider
src/backend/utils/mmgr/freepage.c as a case in point.

I guess we could consider the various xUnit systems for C[1] and have
an entirely new kind of test that runs independently of PostgreSQL.  I
guess that'd be difficult politically (choosing external project,
cognitive load) and technically (global state problems as soon as you
get away from completely stand-alone components).

One idea that keeps coming back to me is that we could probably extend
our existing regression tests to cover C tests with automatic
discovery/minimal boilerplate.  Imagine if you just had to create a
file bitmapset.test.c that sits beside bitmapset.c (and perhaps add it
to TEST_OBJS), and in it write tests using a tiny set of macros a bit
like Google Test's[2].  It could get automagically sucked into a test
driver shlib module, perhaps one per source directory/subsystem, that
is somehow discovered, loaded and run inside PostgreSQL as part of the
regression suite, or perhaps it's just explicitly listed in the
regression schedule with a .sql file that loads the module and runs an
entry point function.

One problem is that if this was happening inside an FMGR function it'd
be always in a transaction, which has implications.  There are
probably better ways to do it.

Thoughts, better ideas?

[1] https://en.wikipedia.org/wiki/List_of_unit_testing_frameworks#C
[2] https://github.com/google/googletest/blob/master/googletest/samples/sample1_unittest.cc

-- 
Thomas Munro
http://www.enterprisedb.com

Hi,

On 2017-08-11 09:53:23 +1200, Thomas Munro wrote:
> The current regression tests, isolation tests and TAP tests are very
> good(though I admit my experience with TAP is limited), but IMHO we
> are lacking support for C-level unit testing.  Complicated, fiddly
> things with many states, interactions, edge cases etc can be hard to
> get full test coverage on from the outside.  Consider
> src/backend/utils/mmgr/freepage.c as a case in point.
> 
> I guess we could consider the various xUnit systems for C[1] and have
> an entirely new kind of test that runs independently of PostgreSQL.  I
> guess that'd be difficult politically (choosing external project,
> cognitive load) and technically (global state problems as soon as you
> get away from completely stand-alone components).

Yea, I think there's little chance for something like that. The amount
of infrastructure you need to link in and initialize is prohibitive imo.


> One idea that keeps coming back to me is that we could probably extend
> our existing regression tests to cover C tests with automatic
> discovery/minimal boilerplate.

What's your definition of boilerplate here? All the "expected" state
tests in C unit tests is plenty boilerplate...


> Imagine if you just had to create a
> file bitmapset.test.c that sits beside bitmapset.c (and perhaps add it
> to TEST_OBJS), and in it write tests using a tiny set of macros a bit
> like Google Test's[2].  It could get automagically sucked into a test
> driver shlib module, perhaps one per source directory/subsystem, that
> is somehow discovered, loaded and run inside PostgreSQL as part of the
> regression suite, or perhaps it's just explicitly listed in the
> regression schedule with a .sql file that loads the module and runs an
> entry point function.
> 
> One problem is that if this was happening inside an FMGR function it'd
> be always in a transaction, which has implications.  There are
> probably better ways to do it.

You can already kinda avoid that in various ways, some more some less
hacky. I think it depends a bit on which scenarios we want to test.  How
much infrastructure do you want around? Do you want to be able to start
transactions? Write to the WAL, etc?  One relatively easy way would be
to simply have a 'postgres' commandline option (like we already kinda
have for EXEC_BACKEND style subprocesses), that loads a shared library
and invokes an entry point in it. That'd then be invoked at some defined
point in time. Alternatively you could just have a bgworker that does
its thing at startup and then shuts down the cluster...


Greetings,

Andres Freund

On Fri, Aug 11, 2017 at 10:35 AM, Andres Freund <andres@anarazel.de> wrote:
> On 2017-08-11 09:53:23 +1200, Thomas Munro wrote:
>> One idea that keeps coming back to me is that we could probably extend
>> our existing regression tests to cover C tests with automatic
>> discovery/minimal boilerplate.
>
> What's your definition of boilerplate here? All the "expected" state
> tests in C unit tests is plenty boilerplate...

I mean close to zero effort required to create and run new tests for
primitive C code.  Just create a .test.c file and type "TEST(my_math,
factorial) { EXPECT_EQ(6, factorial(3)); }" and it'll run when you
"make check" and print out nicely tabulated results and every build
farm member will run it.

>> Imagine if you just had to create a
>> file bitmapset.test.c that sits beside bitmapset.c (and perhaps add it
>> to TEST_OBJS), and in it write tests using a tiny set of macros a bit
>> like Google Test's[2].  It could get automagically sucked into a test
>> driver shlib module, perhaps one per source directory/subsystem, that
>> is somehow discovered, loaded and run inside PostgreSQL as part of the
>> regression suite, or perhaps it's just explicitly listed in the
>> regression schedule with a .sql file that loads the module and runs an
>> entry point function.
>>
>> One problem is that if this was happening inside an FMGR function it'd
>> be always in a transaction, which has implications.  There are
>> probably better ways to do it.
>
> You can already kinda avoid that in various ways, some more some less
> hacky. I think it depends a bit on which scenarios we want to test.  How
> much infrastructure do you want around? Do you want to be able to start
> transactions? Write to the WAL, etc?

I've mostly wanted to do this when working on code that doesn't need
too much of that stuff (dsa.c, freepage.c, parallel hash, ...) but am
certainly very interested in testing transactional stuff related to
skunkworks new storage projects and SSI.  I haven't thought too much
about WAL interaction, that's a good question.

> One relatively easy way would be
> to simply have a 'postgres' commandline option (like we already kinda
> have for EXEC_BACKEND style subprocesses), that loads a shared library
> and invokes an entry point in it. That'd then be invoked at some defined
> point in time. Alternatively you could just have a bgworker that does
> its thing at startup and then shuts down the cluster...

Hmm.  Interesting idea.

I guess you could also also just use _PG_init() as your entrypoint, or
register some callback from there, and LOAD?  Keeping it inside the
existing regression framework seems nice because it already knows how
to paralellise stuff and it'd be small incremental change to start
adding stuff like this.  I suppose we could run most modules that way,
but some extra ones by postgres --run-test-module foo if it turns out
to be necessary.

-- 
Thomas Munro
http://www.enterprisedb.com

On Fri, Aug 11, 2017 at 11:13 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> Just create a .test.c file and type "TEST(my_math,
> factorial) { EXPECT_EQ(6, factorial(3)); }" ...

Of course that would really need to #include "something/test_macros.h"
and "something/factorial.h", and EXPECT_EQ probably doesn't make much
sense in C, but you get the point: very low barrier to use it.

-- 
Thomas Munro
http://www.enterprisedb.com

On 8/10/17 17:53, Thomas Munro wrote:
> The current regression tests, isolation tests and TAP tests are very
> good (though I admit my experience with TAP is limited), but IMHO we
> are lacking support for C-level unit testing.  Complicated, fiddly
> things with many states, interactions, edge cases etc can be hard to
> get full test coverage on from the outside.  Consider
> src/backend/utils/mmgr/freepage.c as a case in point.

I don't have a good idea how to address this, but I agree that something
in this area would be widely useful.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> On 8/10/17 17:53, Thomas Munro wrote:
>> The current regression tests, isolation tests and TAP tests are very
>> good (though I admit my experience with TAP is limited), but IMHO we
>> are lacking support for C-level unit testing.  Complicated, fiddly
>> things with many states, interactions, edge cases etc can be hard to
>> get full test coverage on from the outside.  Consider
>> src/backend/utils/mmgr/freepage.c as a case in point.

> I don't have a good idea how to address this, but I agree that something
> in this area would be widely useful.

I don't think anyone doubts it would be useful.  The question is can we
get to something useful with an acceptable level of effort and overhead.
So far I've not seen anything promising in that sense.
        regards, tom lane

On Thu, Aug 10, 2017 at 2:53 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> The current regression tests, isolation tests and TAP tests are very
> good (though I admit my experience with TAP is limited), but IMHO we
> are lacking support for C-level unit testing.  Complicated, fiddly
> things with many states, interactions, edge cases etc can be hard to
> get full test coverage on from the outside.  Consider
> src/backend/utils/mmgr/freepage.c as a case in point.

It is my understanding that much of the benefit of unit testing comes
from maintainability. It's something that goes well with design by
contract. Writing unit tests is a forcing function. It requires
testable units, making the units more composable. The programmer must
be very deliberate about state, and must delineate code as testable
units. Unit tests are often supposed to be minimal, to serve as a kind
of design document.

In order to unit test something like an index access method, or
VACUUM, or the transaction manager, it would be necessary to mock the
buffer manager, and have a large amount of scaffolding code. This
would be an awful lot of work to do as an afterthought. I wouldn't
know where to draw the line between one unit and another, because the
code just wasn't written with that in mind to begin with. Is snapshot
acquisition a unit that includes heapam.c? If so, does that mean that
heapam.c has to have its own unit for other functionality, or does
that get lumped in with the snapshot acquisition unit?

As I've said a couple of times already, one thought behind amcheck was
that it would allow us to formalize the design of access methods in a
way that somewhat resembles unit testing. The code that we already
have for nbtree is extremely well commented, and is intended to
document how nbtree really works in terms of invariants. Although the
tests are very comprehensive, there really isn't all that much code,
because it's supposed to minimally describe correct states for a
B-Tree. It manages to do this well, perhaps because the Lehman & Yao
design formalizes everything as a small number of per-page atomic
operations, and is very clear about legal and illegal states.

It would be nice if amcheck eventually *guided* the development of the
heap access method. If comprehensive testing of invariants for heapam
becomes very complicated, it could be instructive to consider how to
refactor heapam to make the required verification code simpler but no
less comprehensive. Mostly, this is desirable because it will lead to
an overall simpler, more robust design, and because it lowers the
barrier to entry to assess the correctness of the code. Of course,
this requires buy-in from patch authors to work.

To put it another way, a patch author whose patch touches storage
implicitly asserts that their patch is correct. It would be useful if
they provided a precise falsifiable statement about its correctness up
front, in the form of verification code.

-- 
Peter Geoghegan

On Mon, Aug 14, 2017 at 2:31 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 14 August 2017 at 03:19, Peter Geoghegan <pg@bowt.ie> wrote:
>> It is my understanding that much of the benefit of unit testing comes
>> from maintainability. It's something that goes well with design by
>> contract. Writing unit tests is a forcing function. It requires
>> testable units, making the units more composable. The programmer must
>> be very deliberate about state, and must delineate code as testable
>> units. Unit tests are often supposed to be minimal, to serve as a kind
>> of design document.
>
>
> This is why I personally only find true unit tests useful as part of large
> software packages like Pg when they cover functional units that can be
> fairly well isolated.
>
> However, I'm not sure that Thomas meant unit tests in the formal sense
> isolated and mocked, but rather in the sense of test procedures written in C
> to excercise portions of Pg's code that are not easily/conveniently tested
> from SQL.

I'm interested in both.  The former would be marvellous but difficult
to get to from here, and the latter seems quite achievable and very
useful.  I'm hoping we can start figuring out a good way to do the
latter and make use of it initially for isolated new code, and then
see where that leads.  It's notoriously difficult to retrofit unit
tests to large old code bases and I have a few scars from doing that
(or trying) with big proprietary C and C++ systems.  But I'm
interested in gradual changes that cutting down on global state that
makes it difficult.  Here you can see a tiny example of that:

https://www.postgresql.org/message-id/CAEepm%3D1vUNNBUvTfP%2BJ7wgSqtEbb5NAg01VoZ2hPVyKG2qo8Qw%40mail.gmail.com

Things like this, even after the massive effort of moving enough
things into it to make it useful, wouldn't necessarily allow for
so-called mock session creation (ie a different implementation --
which I guess would involve interacting with the session via a vtable
of function pointers, or some compile time trickery?), but it's a
small step, and might eventually help you set up and tear down test
environments.

> Pg lacks the strict module delineation needed to make formal unit testing
> practical, as you have noted. That doesn't mean test frameworks couldn't be
> useful. I routinely write tests for reasonably isolated functionality in my
> code by writing throwaway SQL-callable functions to exercise it and
> pg_regress tests to run them. It's not strict unit testing as the rest of
> Pg's APIs aren't mocked away, but it's very practical small-unit integration
> testing that helps catch issues.
>
> I wouldn't mind having an easier and nicer way to do that built in to Pg,
> but don't have many ideas about practical, low-maintenance ways to achieve
> it.

I see now that the idea I mentioned earlier amounts to doing what
you've been doing, except with more tooling and conventions to lower
the barrier to use it.  A set of standard test macros, automatic
discovery of your tests, collocation of tests alongside the module
under test in the source tree, good test result reporting etc.  I'm
planning to come back to this in a while and try out some ideas for
the minimum useful scaffolding, but I'd love to hear any suggestions.

-- 
Thomas Munro
http://www.enterprisedb.com

On 13.08.2017 21:19, Peter Geoghegan wrote:
> On Thu, Aug 10, 2017 at 2:53 PM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
>> The current regression tests, isolation tests and TAP tests are very
>> good (though I admit my experience with TAP is limited), but IMHO we
>> are lacking support for C-level unit testing.  Complicated, fiddly
>> things with many states, interactions, edge cases etc can be hard to
>> get full test coverage on from the outside.  Consider
>> src/backend/utils/mmgr/freepage.c as a case in point.
> 
> It is my understanding that much of the benefit of unit testing comes
> from maintainability. 

I never had this understanding. I write tests to test expected behavior 
and not the coded one. If possible i separate the persons writing 
unit-tests from the ones writing the function itself. Having someone 
really read the documentation or translate the expectation into a 
test-case, makes sure, the function itself works well.

Also it really safes time in the long run. Subtle changes / bugs can be 
caught which unit-tests. Also a simple bug-report can be translated into 
a unit-test make sure that the bugfix really works and that no 
regression will happen later. I literally saved ones a week of work with 
a single unit-test.

There are many other advantages, but to earn them the code need to be 
written to be testable. And this is often not the case. Most literature 
advises to Mocking, mixins or other techniques, which most times just 
translate into "this code is not written testable" or "the technique / 
language / concept / etc is not very good in being testable".

Greetings,
Torsten