[PATCH weston 2/3] drm: port the drm backend to the new init api

[Pekka Paalanen]

Hi

On Tue, 25 Aug 2015 14:24:23 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> On Tue, Aug 25, 2015 at 09:03:36AM -0700, Jon A. Cruz wrote:
> > On 08/25/2015 03:08 AM, Pekka Paalanen wrote:

> > > How would you suggest we arrange the source code such that we can pick
> > > arbitrary functions for unit tests without making a copy of that
> > > function?
> > > 
> > > It should be non-disruptive for the source code used for production,
> > > meaning that we cannot e.g. put #ifdef wrappers around every function.
> > > I think putting one function per file is also too much.
> > > 
> > > I wouldn't want to make a copy and then later find out that we have
> > > been testing the copy while the code actually used in production has
> > > already changed. Maintaining a copy manually is a no-go.
> > 
> > Well... there are a few approaches in general that could help. First of
> > all, you are correct about avoiding duplication.
> > 
> > One good point is that if we use a framework that does not require a
> > single executable per test then linking and source code arrangement is
> > greatly simplified.
> > 
> > I've found it very helpful to have source directories structured to
> > match the deployed layout. That is, if there are header files that will
> > be exposed to included by end users then those should be in an include
> > folder in the source tree to begin with (as opposed to some that have
> > all source files together and then have the build process rearrange them
> > for installation).

That works for things that are in headers in the first place. Many things
are just static functions in .c files and I would like to keep them
that way.

> > Then it ends up easy to add headers that declare functions that might be
> > used in tests. Since they do not go into the include folders, the chance
> > of external code using them is minimized. These can also have
> > "_internal" or such as part of their name.

"_internal" sounds horrible, and I'd like to keep the 'static'.

> > Upon occasion I've used a #define of UNIT_TEST to avoid exposing certain
> > things during normal builds. This doesn't necessarily need to expose
> > functions directly, but could be some hook/proxy function. However, my
> > preference has been to minimize such #ifdef use.
> 
> To expand on Jon's comment here, there's several different solutions I
> know about.
> 
> 1.  Hard core Java folk / testing enthusists advocate just avoiding
>     static[1], so all functions are linkable and unit testable.  Tends
>     to make good sense for OO code, but with C code that seems a bit too
>     extreme to me, where static is serving an important need.
> 
>     // project.h
>     #ifdef UNIT_TEST
>     #  define static
>     #endif

This particular piece of code breaks code that relies on static
variables defined inside functions. Not that those are common or a good
design at all, but it would be a nasty surprise.

Not using static in the first place seems like a bad idea.

> 
> 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.

> 
> 3.  You can add a non-static proxy function that calls the static
>     function, then use the non-static function in the test.  E.g.
> 
>     // foo.c
>     static int foo() { return 42; }
> 
>     #ifdef UNIT_TEST
>     int test_foo() { return foo(); }
>     #endif

Ouch, lots of duplication in declarations and uninteresting code
cluttering the sources. No thanks.

> 
> 4.  A more brute force, yet less invasive approach is to have the unit
>     test #include the .c file it's going to test.  This seems a bit
>     clunky to me but this seems to be not that uncommon[2].
> 
>     // test-foo.c
>     #include "foo.c"

Right. Somewhat similar to number 2.

> 
> 5.  There exist some compilation-level techniques, although I don't know
>     a lot about it.  For instance, objcopy's --globalize-symbol[3] will
>     force a given symbol to be visible outside the file its' defined in.
>     There's also a way to give functions shared-library-visibility
>     instead of file-visibility[4], and just ensure the tests get
>     compiled as part of the given library when you're doing test builds.
> 
>     I've never seen any of these compiler-level techniques in practice
>     myself, and might worry about platform compatibility issues, but 

Seems complicated and with the same caveat as number 2, in that you
cannot (or can you?) control the functions that the tested function is
calling.

> 
> Of these, I would probably pick #2, but I could live with 3-5 too.
> I swear I've seen #2 used in other FOSS projects but I'm not finding
> a good example.
> 
> Unit testing is, by definition, drilling down deeper into the codebase
> than the regular interface, so to some degree you have to give it
> special access of some sort or other.  It doesn't have to be done
> invasively, but sometimes is cleaner if some allowance is made.
> Mocks often benefit from being allowed to be a little invasive into the
> code anyway (e.g. generalizing direct calls to external libraries to be
> overridden by the test with the mock implementation).

> 
> Bryce
> 
> 1: http://googletesting.blogspot.com/2008/12/static-methods-are-death-to-testability.html
> 
> 2: http://www.rolandstigge.de/track/Electronica06.pdf
>    c.f. section 3.2.3
> 
> 3: https://sourceware.org/binutils/docs-2.23/binutils/objcopy.html
> 
> 4: https://gcc.gnu.org/wiki/Visibility
> 

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?

Maybe I'd prefer number 2 as far as it works...


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/20150826/c486c29a/attachment.sig>