[libcamera-devel] [PATCH 3/3] test: v4l2_videodevice: buffer_cache: Fail the test if no buffer from cache is obtained

[Kieran Bingham]

Hi Laurent, Umang

On 15/04/2020 04:09, Umang Jain wrote:
> 
> Hi Laurent,
> 
> On Wed, Apr 15, 2020 at 00:56, Laurent Pinchart
> <laurent.pinchart at ideasonboard.com> wrote:
>> Hi Umang, Thank you for the patch. On Tue, Apr 14, 2020 at 07:06:59AM
>> +0000, Umang Jain wrote:
>>
>>     Failing the test guards against negative index (-ENOENT in this
>>     case) being passed to V4L2BufferCache::put(). 
>>
>> Can this happens in practice ?

This is code which /tests/ library code.

Testing things that shouldn't necessarily happen in practice is part of
that right? So that when someone changes V4L2BufferCache and breaks
something - the tests should fire a failure?

In other words, shouldn't test be written defensively?

I don't think this can happen in practice with the *current*
implementation, but that's the point - the implementation could change.

If we didn't ever expect to change implementations - then we don't need
to keep any tests around at all :-)


> Well, V4L2BufferCache::put() accepts only unsigned int argument, so I
> would say, it won't happen in practice.
> Still the patches seems correct to me but commit message should be
> tweaked, is that right?

So in this instance, the ASSERT() will fire, and bail out on
V4L2BufferCache::put() ... but the unexpected fault (which should have
failed the test) is in the get() ... ?

My point is - I would add this check in ..
Laurent - are you suggesting we shouldn't?


> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
> 

-- 
Regards
--
Kieran