[libcamera-devel,v3,06/11] test: media_device: Add test for acquire() and release()
diff mbox series

Message ID 20190511091907.10050-7-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamerea: Add support for exclusive access to cameras between processes
Related show

Commit Message

Niklas Söderlund May 11, 2019, 9:19 a.m. UTC 
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@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

Comments

Laurent Pinchart May 11, 2019, 12:55 p.m. UTC | #1 
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@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.

> + */
> +
> +#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'],
>  ]
Laurent Pinchart May 11, 2019, 12:56 p.m. UTC | #2 
On Sat, May 11, 2019 at 03:55:33PM +0300, Laurent Pinchart wrote:
> 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@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.

I forgot to mention that, apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > + */
> > +
> > +#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'],
> >  ]
Niklas Söderlund May 16, 2019, 11:57 p.m. UTC | #3 
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@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

Patch
diff mbox series 

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
+ */
+
+#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'],
 ]