[libcamera-devel] [PATCH v3 06/11] test: media_device: Add test for acquire() and release()

[Niklas Söderlund]

Hi Laurent,

Thanks for your feedback.

On 2019-05-11 15:55:33 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Sat, May 11, 2019 at 11:19:02AM +0200, Niklas Söderlund wrote:
> > The interfaces MediaDevice::{open,close}() are about to be made private,
> > replace them with a test of MediaDevice::{acquire,release}() instead.
> > The new test will implicitly tests the open() and close() methods as
> > they are about to be move inside acquire() and release() which will
> > remain public.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> >  test/media_device/media_device_basic.cpp      | 41 +++++++++++++++++++
> >  test/media_device/media_device_print_test.cpp | 11 -----
> >  test/media_device/meson.build                 |  1 +
> >  3 files changed, 42 insertions(+), 11 deletions(-)
> >  create mode 100644 test/media_device/media_device_basic.cpp
> > 
> > diff --git a/test/media_device/media_device_basic.cpp b/test/media_device/media_device_basic.cpp
> > new file mode 100644
> > index 0000000000000000..ba95dd0578fce22d
> > --- /dev/null
> > +++ b/test/media_device/media_device_basic.cpp
> > @@ -0,0 +1,41 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * media_device_basic.cpp - Test basic functions of MediaDevice
> 
> How about renaming this file to media_device_acquire.cpp and updating
> the description accordingly ? We can rename it to something else later
> if we add more test.

Good point, will do so for v4.

> 
> > + */
> > +
> > +#include "media_device_test.h"
> > +
> > +using namespace libcamera;
> > +
> > +class MediaDeviceBasic : public MediaDeviceTest
> > +{
> > +	int testAcquire()
> > +	{
> > +		if (!media_->acquire())
> > +			return TestFail;
> > +
> > +		if (media_->acquire())
> > +			return TestFail;
> > +
> > +		media_->release();
> > +
> > +		if (!media_->acquire())
> > +			return TestFail;
> > +
> > +		media_->release();
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	int run()
> > +	{
> > +		if (testAcquire() != TestPass)
> > +			return TestFail;
> > +
> > +		return TestPass;
> > +	}
> > +};
> > +
> > +TEST_REGISTER(MediaDeviceBasic);
> > diff --git a/test/media_device/media_device_print_test.cpp b/test/media_device/media_device_print_test.cpp
> > index ceffd538e13fca73..30d929b8c76387a7 100644
> > --- a/test/media_device/media_device_print_test.cpp
> > +++ b/test/media_device/media_device_print_test.cpp
> > @@ -113,17 +113,6 @@ int MediaDevicePrintTest::testMediaDevice(const string deviceNode)
> >  	MediaDevice dev(deviceNode);
> >  	int ret;
> >  
> > -	/* Fuzzy open/close sequence. */
> > -	ret = dev.open();
> > -	if (ret)
> > -		return ret;
> > -
> > -	ret = dev.open();
> > -	if (!ret)
> > -		return ret;
> > -
> > -	dev.close();
> > -
> >  	ret = dev.populate();
> >  	if (ret)
> >  		return ret;
> > diff --git a/test/media_device/meson.build b/test/media_device/meson.build
> > index 1005685409d99aa7..7a3266187abd806f 100644
> > --- a/test/media_device/meson.build
> > +++ b/test/media_device/meson.build
> > @@ -1,4 +1,5 @@
> >  media_device_tests = [
> > +    ['media_device_basic',              'media_device_basic.cpp'],
> >      ['media_device_print_test',         'media_device_print_test.cpp'],
> >      ['media_device_link_test',          'media_device_link_test.cpp'],
> >  ]
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund