Testing static functions in testsuite (was Re: [PATCH weston 2/3] drm: port the drm backend to the new init api)

[Pekka Paalanen]

On Wed, 26 Aug 2015 14:26:09 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> On Wed, Aug 26, 2015 at 04:35:59PM +0300, Pekka Paalanen wrote:
> > On Tue, 25 Aug 2015 14:24:23 -0700
> > Bryce Harrington <bryce at osg.samsung.com> wrote:

> > > 2.  Another approach is to #define unit_static (or debug_export, or
> > >     similar) to be static for normal builds, and empty for test builds,
> > >     so tests can link the internal functions.
> > > 
> > >     // project.h
> > >     #ifdef UNIT_TEST
> > >     #  define unit_static
> > >     #else
> > >     #  define unit_static static
> > >     #endif
> > 
> > This could work. In fact, I already used this pattern in matrix.c.
> > 
> > The downside of this is that one still needs the whole .c file built
> > into the test, and you cannot control everything called from the
> > function to be tested.
> 
> Not sure what you mean by the whole .c file?
> 
> You'd link the test against the foo.o module, and that'd link in any of
> the functions that were labeled unit_static.  I don't see that there's a
> big problem with that, though?
> 
> If you're thinking this will bloat the test executable, well maybe but
> disk space is cheap, who cares?
> 
> As to controlling things to be called from the function under test, 
> 
> As to controlling the things that the function to be tested calls, this
> is true but that's kind of orthogonal.  First we have to solve the
> static function problem, in order to get higher granuarity in
> testability of *internal* function calls.  As to the external functions,
> let's consider options for that in a bit (see below).

The problem I see is that, let's say we have a static function foo() we
want to test. This function calls another static function bar() in the
same .c file. If we compile that whole file into an .o and link it
with the test program, we cannot intercept the call from foo() to bar().

So yes, you got the point.


> > I had a wacky idea.
> > 
> > Our coding style is quite strict. We could have e.g. an awk program or
> > something that filters the given functions from a .c file into a new
> > file. Find a line that begins with the given function name and '(',
> > include also the preceding line, and include all lines up to the first
> > one that is "}\n". This way the build system could make an isolated
> > copy of the function to be tested into the file. Then the test does
> > #include "file" to get the function, and would be in complete control
> > of everything around it. Add some hand-waving on how to deal with
> > struct declarations etc.
> > 
> > Completely crazy?
> 
> Not crazy, although trade-off here is it adds some brittleness to the
> test system since we'd be involving code parsers and such.  This sounds
> like essentially a refined version of #4.
> 
> > Maybe I'd prefer number 2 as far as it works...
> 
> I like it because it's simple.  We could also roll it out function by
> function as we write the unit tests.

Sure. It's good for the cases where it works, and maybe it is the
majority or even all.

Only if we really want to overwrite other static functions we will need
something like the crazy.

Two ways, let's pick the appropriate per case.

I'm getting a little ahead of ourselves.

> 				 * * *
> 
> Functions that call to external services is a separate problem from
> accessing the internal functions.  Like we discussed earlier, this often
> is handled with mocks - basically we somehow substitute the normal call
> for one of our own definition.  We have a few different options
> here.  stackoverflow summarizes several approaches:
> 
> http://stackoverflow.com/questions/2924440/advice-on-mocking-system-calls
> 
> A.  Use a wrapper function, which calls our mock functionality when
>     UNIT_TEST is defined, or the real system call otherwise.
> 
> B.  Dependency injection.  Instead of calling the system call directly,
>     we have a function pointer that we reference.  For testing we have
>     it point to our test routine; for production we hook to the actual
>     system call.
> 
> C.  Use "link seam".  Essentially we use the linker to substitute our
>     own definitions of the external function calls.
> 
> D.  gcc compiler magic.  gcc has a --wrap option that allows tests to
>     replace specific functions with their own wrappers.
> 
> Option A adds an extra abstraction layer, but is conceptually
> straightforward; maybe the layer can be avoided if the wrappers are
> implemented purely as preprocessor macros.  Option B seems powerful but
> maybe overly complex.  Option C seems the least invasive and probably
> fine if we're dealing with just simple system calls, but I'm not sure it
> covers cases where there's some external state we're trying to model in
> the test (e.g. monitor configuration).  Option D looks pretty slick but
> we're maybe dealing with compiler compatibility issues, but maybe an
> equivalent feature is available on other compilers?
> 
> I'm less sure what the right approach is for this, but I lean towards
> Option D.  It seems least invasive and most maintainable (apart from
> platform compatibility issues), and compartmentalizes all the mocking
> junk to within the tests, which is probably where it should belong
> anyway.  If it ends up being too reliant on gcc-specific features,
> Option C is like the manually implemented version of D and might be a
> suitable fallback if D proves too limiting.

I don't like A or B, because they obfuscate the production code.

To me, C is the obvious choice. Overwriting library calls is easy, we
already do it for malloc et al. for some basic leak checking in
Wayland's tests/test-runner.c. No-one has complained of that approach
yet, AFAIK, so presumably it works for everyone.

I hadn't heard about D. If it works on the compilers our downstreams
care about, why not. Maybe we should add one simple test using that
when we start the 1.10 development cycle, then we should know around
the middle of 1.11 cycle if we can rely on it.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150827/1dd93185/attachment.sig>