[libcamera-devel] [PATCH v2 2/3] test: media_device: Make MediaDeviceTest a MediaDevicePrintTest

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Friday, 11 January 2019 15:27:04 EET Jacopo Mondi wrote:
> As a new class for test link handling will be added as a separate test,
> it makes no sense to have a generic "MediaDeviceTest" class. Rename it
> in "MediaDevicePrintTest", and make it run only printing test.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  ...e_test.cpp => media_device_print_test.cpp} | 31 +++++++++----------
>  test/media_device/meson.build                 |  2 +-
>  2 files changed, 15 insertions(+), 18 deletions(-)
>  rename test/media_device/{media_device_test.cpp =>
> media_device_print_test.cpp} (79%)
> 
> diff --git a/test/media_device/media_device_test.cpp
> b/test/media_device/media_device_print_test.cpp similarity index 79%
> rename from test/media_device/media_device_test.cpp
> rename to test/media_device/media_device_print_test.cpp
> index c482b2e..9746afa 100644
> --- a/test/media_device/media_device_test.cpp
> +++ b/test/media_device/media_device_print_test.cpp
> @@ -1,10 +1,8 @@
>  /* SPDX-License-Identifier: GPL-2.0-or-later */
>  /*
> - * Copyright (C) 2018, Google Inc.
> + * Copyright (C) 2019, Google Inc.

2018-2019. You should use 2019 for new code only, and 2018-2019 for older code 
that is modified in 2019.

Apart from this,

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

I wonder whether this test shouldn't be replaced by something else down the 
road. Printing a media device graph isn't really a unit test, maybe we should 
instead create a test that looks up entities, pads and links through various 
means based on a known media graph (vimc ?) to test the API.

>   *
> - * media_device_test.cpp - Tests for the media device class.
> - *
> - * Test library for the media device class.
> + * media_device_print_test.cpp - Print out media devices
>   */
>  #include <iostream>
> 
> @@ -20,16 +18,16 @@ using namespace libcamera;
>  using namespace std;
> 
>  /*
> - * MediaDeviceTest object: runs a sequence of tests on all media
> - * devices found in the system.
> + * MediaDevicePrintTest takes all media devices found in the system and
> print
> + * them out to verify correctness.
>   *
>   * If no accessible media device is found, the test is skipped.
>   */
> -class MediaDeviceTest : public Test
> +class MediaDevicePrintTest : public Test
>  {
>  public:
> -	MediaDeviceTest() { }
> -	~MediaDeviceTest() { }
> +	MediaDevicePrintTest() { }
> +	~MediaDevicePrintTest() { }
> 
>  protected:
>  	int init() { return 0; }
> @@ -44,7 +42,7 @@ private:
>  	void printNode(const MediaPad *pad, ostream &os);
>  };
> 
> -void MediaDeviceTest::printNode(const MediaPad *pad, ostream &os)
> +void MediaDevicePrintTest::printNode(const MediaPad *pad, ostream &os)
>  {
>  	const MediaEntity *entity = pad->entity();
> 
> @@ -52,7 +50,7 @@ void MediaDeviceTest::printNode(const MediaPad *pad,
> ostream &os) << pad->index() << "]";
>  }
> 
> -void MediaDeviceTest::printLinkFlags(const MediaLink *link, ostream &os)
> +void MediaDevicePrintTest::printLinkFlags(const MediaLink *link, ostream
> &os) {
>  	unsigned int flags = link->flags();
> 
> @@ -68,7 +66,7 @@ void MediaDeviceTest::printLinkFlags(const MediaLink
> *link, ostream &os) * For each entity in the media graph, printout links
> directed to its sinks * and source pads.
>   */
> -void MediaDeviceTest::printMediaGraph(const MediaDevice &media, ostream
> &os)
> +void MediaDevicePrintTest::printMediaGraph(const MediaDevice &media, 
> ostream &os) {
>  	os << "\n" << media.driver() << " - " << media.devnode() << "\n\n";
> 
> @@ -110,7 +108,7 @@ void MediaDeviceTest::printMediaGraph(const MediaDevice
> &media, ostream &os) }
> 
>  /* Test a single media device. */
> -int MediaDeviceTest::testMediaDevice(const string devnode)
> +int MediaDevicePrintTest::testMediaDevice(const string devnode)
>  {
>  	MediaDevice dev(devnode);
>  	int ret;
> @@ -134,9 +132,8 @@ int MediaDeviceTest::testMediaDevice(const string
> devnode) if (ret)
>  		return ret;
> 
> -	/* Run tests in sequence. */
> +	/* Print out the media graph. */
>  	printMediaGraph(dev, cerr);
> -	/* TODO: add more tests here. */
> 
>  	dev.close();
> 
> @@ -145,7 +142,7 @@ int MediaDeviceTest::testMediaDevice(const string
> devnode)
> 
>  /* Run tests on all media devices. */
>  #define MAX_MEDIA_DEV 256
> -int MediaDeviceTest::run()
> +int MediaDevicePrintTest::run()
>  {
>  	const string devnode("/dev/media");
>  	unsigned int i;
> @@ -171,4 +168,4 @@ int MediaDeviceTest::run()
>  	return ret;
>  }
> 
> -TEST_REGISTER(MediaDeviceTest);
> +TEST_REGISTER(MediaDevicePrintTest);
> diff --git a/test/media_device/meson.build b/test/media_device/meson.build
> index 40f9ffa..e4bedb7 100644
> --- a/test/media_device/meson.build
> +++ b/test/media_device/meson.build
> @@ -1,5 +1,5 @@
>  media_device_tests = [
> -    ['media_device_test',               'media_device_test.cpp'],
> +    ['media_device_print_test',         'media_device_print_test.cpp'],
>  ]
> 
>  foreach t : media_device_tests

-- 
Regards,

Laurent Pinchart