Weston test suite (Re: [PATCH weston 1/4] ivi-shell: change layer visibility to bool)

[Pekka Paalanen]

On Thu, 15 Feb 2018 15:30:55 +0000
Emil Velikov <emil.l.velikov at gmail.com> wrote:

> On 13 February 2018 at 17:46, Daniel Stone <daniel at fooishbar.org> wrote:
> > Hi Emil,
> >
> > On 13 February 2018 at 16:37, Emil Velikov <emil.l.velikov at gmail.com> wrote:  
> >> The following two questions come to mind:
> >>  - app bugs - using threads? ivi-shell-user-interface.c mentions
> >> pthread, but haven't looked closely  
> >
> > Hm, I couldn't find any threading-related bugs at all: everything I
> > found was clear just looking at the inter-process flows. Did you have
> > anything in particular in mind?
> >  
> >>  - missing dependencies  
> >
> > Again, did you have anything in particular in mind? Looking at the
> > logs, it loads the exact same set of plugins/helpers/configs in both
> > cases (working/broken). Dumping out /proc/self/smaps didn't show any
> > discrepancies either. Everything was fresh and up-to-date: it's seems
> > like it's only the test execution environment which influences this.
> >  
> >> Having a look reveals:
> >>  - extremely convoluted test runner  
> >
> > Sure. We had a few requirements which led to the runner being the way
> > it was, mainly based on it being relatively easy to write tests, and
> > assertion failures / segfaults not destroying the whole testsuite.
> > Personally I like igt, and I guess others like gtest or maybe Piglit,
> > but none of them are anything like simple. Do you have a good
> > replacement in mind?
> >
> > (Whilst we're mentioning test runners and threads, I really wouldn't
> > mind one which ran everything in threads rather than forking
> > children.)
> >  
> >>  - dummy BACKEND variable - always headless-backend.so  
> >
> > It's not a dummy: you can run 'make check BACKEND=foo-backend.so' as a
> > developer, if you'd like to test a particular backend. Most of them
> > are totally unsuitable for general-purpose use (e.g. wayland-backend
> > or x11-backend will pop up a billion windows as it goes about its
> > work, and that rapid de/focus can in fact break some tests), so we
> > default to headless-backend for mere mortals. But it is there as an
> > option for people who know what they're doing, or want to try tests in
> > a different context.
> >  
> >>  - the --backend, --config, --shell and/or --modules file is not part
> >> of the respective dependency chain  
> >
> > The checks here are only run as part of the 'check' target; check
> > depends on check-am, which in turn depends on the all-am target, which
> > in turn depends on all libraries/programs/built-sources, which covers
> > all of the items you've mentioned. Is there some subtlety I'm missing
> > here?
> >  
> Pardon for the delay, Dan.
> While Emre has already resolved the issue, let me add a couple words
> more to my earlier, cryptic, email:
> 
>  - threading
> I could not spot any, although the behaviour that you described
> sounded like an in-weston race condition.
> We now know that wasn't a race, but a layering confusion/violation(?).

Hi Emil

It was a race. Loading a plugin that was not supposed be loaded
(hmi-controller.so) forked a helper process, which raced against the
actual tests (the test has a weston process and a test process, but they
run synchronized via Wayland).

The racing helper process triggered actions in the plugin that was not
supposed to be loaded, changing the weston process state which confused
the weston process part of the test.

I cannot see it as a layering thing. It was a matter of loading two
mutually exclusive plugins, both hooking up to the same ivi-layout API.
The API does not protect against this, because the original idea was
that it would be useful to be able to load multiple plugins with the
caveat that they must be carefully designed to avoid assuming that they
are the only user of the API. However, a test plugin using this API
must really be the only user of the API, otherwise the tests become
unpredictable.

We don't use threads in Weston code base, but we do fork&exec helpers.

> On the testrunner side - I've used cmocka and it seemed rather nice.
> I liked the exception handling for signals, it has no templates plus
> valgrind was always happy.

The Weston test suite is mostly about integration(?) testing rather
than unit testing. There are some tests you could probably classify as
unit tests, but most of them need almost a full-blown compositor. I
would assume that the lack of small contained interfaces in libweston
makes writing meaningful mocks a big effort with prohibitive
maintenance costs. Hence we tend to test the whole compositor with the
public interfaces it has: input devices and rendered output vs. client
Wayland interface operation, or only the Wayland interface.

That said, there are several mock-like components: headless-backend,
the test clients, the IVI controller test plugin, the desktop test
shell plugin. These are used by having libweston load them instead of
what one uses in production.

There have been floating ideas of something like loading libweston
directly in the test process instead of forking the weston executable,
but my memory of how and why is fading. One goal was to make tests
easier to debug, so that one could simply 'gdb ./test-binary' and
expect a meaningful result. The current runner makes debugging a test
very awkward. The zunitc test runner (yes, another one) was aiming to
that goal, however the one-person heroic development effort died before
it got that far.

I certainly agree the current test suite infrastructure is not very
good, but my feeling is that we are foremost lacking tests rather than
an infrastruture to base them on.


Thanks,
pq

> 
> HTH
> Emil
> 
> [1] https://api.cmocka.org/

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180216/96dd228a/attachment-0001.sig>