[libcamera-devel] [PATCH] meson: disable tests by default

[Kieran Bingham]

Quoting Christian Rauch (2022-09-05 20:26:19)
> Hi Kieran,
> 
> Am 05.09.22 um 11:51 schrieb Kieran Bingham:
> > Hi All,
> >
> > Quoting Umang Jain via libcamera-devel (2022-09-05 07:41:50)
> >> Hi Christian,
> >>
> >> Thank you for the patch.
> >>
> >> On 9/5/22 3:29 AM, Laurent Pinchart via libcamera-devel wrote:
> >>> Hi Christian,
> >>>
> >>> Thank you for the patch.
> >>>
> >>> On Sun, Sep 04, 2022 at 06:27:15PM +0200, Christian Rauch via libcamera-devel wrote:
> >>>> Compiling with tests adds a significant amount of build targets to the
> >>>> meson build. However, tests are only of interest when a feature has been
> >>>> implemented or for continuous integration.
> >>>> Disable tests by default to reduce the computational resource requirements
> >>>> and the duration of a complete libcamera built.
> >>>>
> >>>> Signed-off-by: Christian Rauch <Rauch.Christian at gmx.de>
> >>> As most people building libcamera won't run tests, this seems reasonable
> >>> to me. I'm however a bit worried that it would lead to lower
> >>> compilation-test coverage, as people may send patches without realizing
> >>> they break compilation of tests. We already have that problem for
> >>
> >> I'm concerned about this as well.
> >
> > When I first saw this patch over the weekend, indeed my reaction was
> > "Ahhh" ... but I also agree this is a better default to have off right
> > now.
> >
> > We would like patches to be submitted after having all tests run, but
> > the majority of people who compile libcamera (currently) may not be
> > developers. We have lots of people compiling on RPi / IPU3 and low power
> > devices directly, and they certainly don't care for running tests.
> 
> I compile libcamera more often than I submit patches. I also compile on
> a Raspberry Pi Zero 2. So compiling the tests is a huge waste of
> resources for me.

Yes, I understand that. If libcamera were 'well packaged, stable, and
finished' ... there would be less users having to compile it though.

So some time in the <waves hand> future... libcamera should become
rarely built, and tests won't be such a burden.

Also - compiling on an RPi02 is certainly a pain. Did I share the RPi
cross compile environment with you already ?



> > So to improve user experience, I do think this should be off by default
> > (at least now) and that stance may change depending on our upcoming
> > release strategies ... but developers should be able to enable tests
> > anyway.
> 
> I would argue that tests should only be enabled when it is intended to
> run them.
> 
> > Ideally of course these tests will then be enabled and run on any branch
> > push to the common public hosting services, so I think we should
> > integrate a gitlab and github CI script that could build and run the
> > unit tests. (These could skip the kernel related tests initially).
> 
> I am also used to have tests run automatically on a CI server when I
> send PRs to other projects. Having some public libcamera CI instance
> that compiles and tests sent patches would be very helpful.
> If you intend to use GitHub actions, I can contribute the scripts that I
> already use for my own purpose.

As mentioned in the thread with Laurent - this is more complex because
we need a custom kernel with virtual drivers (as you've experienced).

I think this can be solved, but that requires some investigations that I
haven't had time for.

'We' won't use github actions, but I think integrating a github actions
script to the repository is certainly reasonable, if it helps anyone who
pushes a branch to their repository compile test in a consistent manner.

If you have something usable for that, please do post it to the list.

--
Kieran


> 
> >
> > Anyway,
> >
> > Yes -
> >
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >
> >>> runtime breakages of tests, and it's most likely be addressed by test
> >>> automation on patches submitted upstream, so I think I'm fine with the
> >>> change.
> >>>
> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >>
> >> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> >>
> >>>
> >>>> ---
> >>>>   meson_options.txt | 1 +
> >>>>   1 file changed, 1 insertion(+)
> >>>>
> >>>> diff --git a/meson_options.txt b/meson_options.txt
> >>>> index 7a9aecfc..f1d67808 100644
> >>>> --- a/meson_options.txt
> >>>> +++ b/meson_options.txt
> >>>> @@ -47,6 +47,7 @@ option('qcam',
> >>>>
> >>>>   option('test',
> >>>>           type : 'boolean',
> >>>> +        value : false,
> >>>>           description: 'Compile and include the tests')
> >>>>
> >>>>   option('tracing',
> >>