RFC: Test plugin and operation specs



Hi Grileros!

Some ideas grew in my mind, and I'd be happy to have some feedback on them.

As you all know, from a technical point of view, Grilo cannot function without
any plugin[1]. Every operation it can do (browse, search, get metadata, ...) is
relative to a plugin.

This is a problem for testing, especially for regression tests: we need to use
a plugin to test functionality of the core. But then, if there's a regression
in the plugin we use, it would be erroneously detected as a regression in the
core (currently, we have some tests using the fs plugin, others using the first
plugin it finds), and we want to separate these cases.

In that end, I started writing a dummy plugin specifically for test
purposes[2]. The idea is to have a signal emitted for each operation that is
triggered, and let the test define in a signal callback what the behaviour of
the source would be for that operation.

>From a plugin point of view, when an operation is called by the application, a
vmethod of the plugin gets called, for instance, for search:

  void (*search) (GrlMediaSource *source, GrlMediaSourceSearchSpec *ss);

All the other operation vmethods have a very similar prototype, with a
different Grl*Spec structure.

Here are the various things that I noted while developing this plugin.

(A) The first problem I've had here is that, to define a signal, you need all
the parameters to be of a type that is defined for GLib (at least a GBoxed).
(B) A second issue that I considered while I'm at it, is that it would be damn
cool if this could work from language bindings, because tests are boring to
write, so I'd rather write them fast in a language more expressive than the C
language.
(C) And the third aspect of things is that all these Grl*Spec have a lot in
common.

(A) can be fixed by wrapping the structs around a GBoxed. But most
of the specs contain GLists (typically lists of keys), and these cannot be
accessed from bindings, which is a problem for (B).
And (C) calls for unification of these structs.

With all this considered, I started writing a GrlOperationSpec GObject class,
that encompasses all these things common to all operations.
For ideal cleanliness, I think each operation should be implemented as a
subclass of GrlOperationSpec, adding the bits specific to it. Right now in my
branch, I went for the "lazy" way by putting the info about all operations in
GrlOperationSpec (using unions aka "poor man's inheritance").

And I now use that GrlOperationSpec in my test plugin, and that way, all the
operations can be dynamically defined by connecting to the right signals, which
I do (in python!) in [3].

Seeing that GrlOperationSpec can be very useful to allow plugins to be defined
in "external" languages, I have put it in the core library (instead of in the
test plugin). Moreover, imaging the duplication stated in (C), a lot of
operations code in the core is duplicated (look at grl-media-source.c if you're
not convinced). I think we can rather easily factorise that by dropping all the
existing Grl*Spec structs and replacing them by GrlOperationSpec (maybe after
fixing the poor man's inheritance issue).


And now I am wondering: what do you guys think about that idea? Should we go
forward with GrlOperationSpec? As they are now, or with proper inheritance?
Should we only use that in the test plugin, or actively replace existing
Grl*Spec structures with it?


Cheers,

Guij


[1] In this email, I don't make the distinction between "plugin" and "source";
technically, Grilo doesn't really make it either, so I'll consider that's all
right for that discussion
[2] on git gitorious org:~guijemont/grilo/guijemont-sandbox.git in the
"test_plugin_wip" branch. As the name of the branch suggests, this is work in
progress, there might still be quite a few ugly things in it, such as
inconsistent APIs.
[3] https://www.gitorious.org/~guijemont/grilo/guijemont-sandbox/blobs/test_plugin_wip/tests/python/test_media_source.py


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]