[libcamera-devel] [PATCH v2 01/24] test: Extract CameraTest class out of camera tests to libtest
Niklas Söderlund
niklas.soderlund at ragnatech.se
Thu Nov 14 09:07:52 CET 2019
Hi Laurent,
Thanks for your work.
On 2019-11-08 22:53:46 +0200, Laurent Pinchart wrote:
> Many tests other than the camera/ tests use a camera. To increase code
> sharing, move the base CameraTest class to the test library. The class
> becomes a helper that doesn't inherit from Test anymore (to avoid
> diamond inheritance issues when more such helpers will exist).
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
I'm not overly found of how status_ is checked in users, but I like the
overall change more and this is test code.
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> test/camera/buffer_import.cpp | 14 ++++-----
> test/camera/capture.cpp | 15 ++++++---
> test/camera/configuration_default.cpp | 16 ++++++++--
> test/camera/configuration_set.cpp | 15 ++++++---
> test/camera/meson.build | 2 +-
> test/camera/statemachine.cpp | 15 ++++++---
> test/controls/control_list.cpp | 39 +++++-------------------
> test/{camera => libtest}/camera_test.cpp | 24 +++++++++------
> test/{camera => libtest}/camera_test.h | 12 +++-----
> test/libtest/meson.build | 1 +
> 10 files changed, 77 insertions(+), 76 deletions(-)
> rename test/{camera => libtest}/camera_test.cpp (55%)
> rename test/{camera => libtest}/camera_test.h (77%)
>
> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> index bbc5a25c4019..3efe02704c02 100644
> --- a/test/camera/buffer_import.cpp
> +++ b/test/camera/buffer_import.cpp
> @@ -18,6 +18,7 @@
> #include "v4l2_videodevice.h"
>
> #include "camera_test.h"
> +#include "test.h"
>
> using namespace libcamera;
>
> @@ -254,11 +255,11 @@ private:
> bool done_;
> };
>
> -class BufferImportTest : public CameraTest
> +class BufferImportTest : public CameraTest, public Test
> {
> public:
> BufferImportTest()
> - : CameraTest()
> + : CameraTest("VIMC Sensor B")
> {
> }
>
> @@ -350,11 +351,10 @@ protected:
>
> int init()
> {
> - int ret = CameraTest::init();
> - if (ret)
> - return ret;
> + if (status_ != TestPass)
> + return status_;
>
> - ret = sink_.init();
> + int ret = sink_.init();
> if (ret != TestPass) {
> cleanup();
> return ret;
> @@ -422,8 +422,6 @@ protected:
>
> camera_->stop();
> camera_->freeBuffers();
> -
> - CameraTest::cleanup();
> }
>
> private:
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index 791ccad15f70..08cce9c7cbaf 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -8,13 +8,20 @@
> #include <iostream>
>
> #include "camera_test.h"
> +#include "test.h"
>
> using namespace std;
>
> namespace {
>
> -class Capture : public CameraTest
> +class Capture : public CameraTest, public Test
> {
> +public:
> + Capture()
> + : CameraTest("VIMC Sensor B")
> + {
> + }
> +
> protected:
> unsigned int completeBuffersCount_;
> unsigned int completeRequestsCount_;
> @@ -46,14 +53,12 @@ protected:
>
> int init() override
> {
> - int ret = CameraTest::init();
> - if (ret)
> - return ret;
> + if (status_ != TestPass)
> + return status_;
>
> config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
> if (!config_ || config_->size() != 1) {
> cout << "Failed to generate default configuration" << endl;
> - CameraTest::cleanup();
> return TestFail;
> }
>
> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
> index ce2ec5d02e7b..31c908d2449e 100644
> --- a/test/camera/configuration_default.cpp
> +++ b/test/camera/configuration_default.cpp
> @@ -8,15 +8,27 @@
> #include <iostream>
>
> #include "camera_test.h"
> +#include "test.h"
>
> using namespace std;
>
> namespace {
>
> -class ConfigurationDefault : public CameraTest
> +class ConfigurationDefault : public CameraTest, public Test
> {
> +public:
> + ConfigurationDefault()
> + : CameraTest("VIMC Sensor B")
> + {
> + }
> +
> protected:
> - int run()
> + int init() override
> + {
> + return status_;
> + }
> +
> + int run() override
> {
> std::unique_ptr<CameraConfiguration> config;
>
> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
> index f88da96ca2b7..b4b5968115e8 100644
> --- a/test/camera/configuration_set.cpp
> +++ b/test/camera/configuration_set.cpp
> @@ -8,24 +8,29 @@
> #include <iostream>
>
> #include "camera_test.h"
> +#include "test.h"
>
> using namespace std;
>
> namespace {
>
> -class ConfigurationSet : public CameraTest
> +class ConfigurationSet : public CameraTest, public Test
> {
> +public:
> + ConfigurationSet()
> + : CameraTest("VIMC Sensor B")
> + {
> + }
> +
> protected:
> int init() override
> {
> - int ret = CameraTest::init();
> - if (ret)
> - return ret;
> + if (status_ != TestPass)
> + return status_;
>
> config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
> if (!config_ || config_->size() != 1) {
> cout << "Failed to generate default configuration" << endl;
> - CameraTest::cleanup();
> return TestFail;
> }
>
> diff --git a/test/camera/meson.build b/test/camera/meson.build
> index d6fd66b8f89e..e2a6660a7a92 100644
> --- a/test/camera/meson.build
> +++ b/test/camera/meson.build
> @@ -9,7 +9,7 @@ camera_tests = [
> ]
>
> foreach t : camera_tests
> - exe = executable(t[0], [t[1], 'camera_test.cpp'],
> + exe = executable(t[0], t[1],
> dependencies : libcamera_dep,
> link_with : test_libraries,
> include_directories : test_includes_internal)
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index 12d5e0e1d534..afa13ba77b0b 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -8,13 +8,20 @@
> #include <iostream>
>
> #include "camera_test.h"
> +#include "test.h"
>
> using namespace std;
>
> namespace {
>
> -class Statemachine : public CameraTest
> +class Statemachine : public CameraTest, public Test
> {
> +public:
> + Statemachine()
> + : CameraTest("VIMC Sensor B")
> + {
> + }
> +
> protected:
> int testAvailable()
> {
> @@ -233,14 +240,12 @@ protected:
>
> int init() override
> {
> - int ret = CameraTest::init();
> - if (ret)
> - return ret;
> + if (status_ != TestPass)
> + return status_;
>
> defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
> if (!defconf_) {
> cout << "Failed to generate default configuration" << endl;
> - CameraTest::cleanup();
> return TestFail;
> }
>
> diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
> index 5af53f64bb6c..4d212abd09e6 100644
> --- a/test/controls/control_list.cpp
> +++ b/test/controls/control_list.cpp
> @@ -13,32 +13,22 @@
> #include <libcamera/controls.h>
>
> #include "camera_controls.h"
> +
> +#include "camera_test.h"
> #include "test.h"
>
> using namespace std;
> using namespace libcamera;
>
> -class ControlListTest : public Test
> +class ControlListTest : public CameraTest, public Test
> {
> -protected:
> - int init()
> +public:
> + ControlListTest()
> + : CameraTest("VIMC Sensor B")
> {
> - cm_ = new CameraManager();
> -
> - if (cm_->start()) {
> - cout << "Failed to start camera manager" << endl;
> - return TestFail;
> - }
> -
> - camera_ = cm_->get("VIMC Sensor B");
> - if (!camera_) {
> - cout << "Can not find VIMC camera" << endl;
> - return TestSkip;
> - }
> -
> - return TestPass;
> }
>
> +protected:
> int run()
> {
> CameraControlValidator validator(camera_.get());
> @@ -156,21 +146,6 @@ protected:
>
> return TestPass;
> }
> -
> - void cleanup()
> - {
> - if (camera_) {
> - camera_->release();
> - camera_.reset();
> - }
> -
> - cm_->stop();
> - delete cm_;
> - }
> -
> -private:
> - CameraManager *cm_;
> - std::shared_ptr<Camera> camera_;
> };
>
> TEST_REGISTER(ControlListTest)
> diff --git a/test/camera/camera_test.cpp b/test/libtest/camera_test.cpp
> similarity index 55%
> rename from test/camera/camera_test.cpp
> rename to test/libtest/camera_test.cpp
> index 101e31fbce79..2ae4d6776f2e 100644
> --- a/test/camera/camera_test.cpp
> +++ b/test/libtest/camera_test.cpp
> @@ -8,35 +8,39 @@
> #include <iostream>
>
> #include "camera_test.h"
> +#include "test.h"
>
> using namespace libcamera;
> using namespace std;
>
> -int CameraTest::init()
> +CameraTest::CameraTest(const char *name)
> {
> cm_ = new CameraManager();
>
> if (cm_->start()) {
> - cout << "Failed to start camera manager" << endl;
> - return TestFail;
> + cerr << "Failed to start camera manager" << endl;
> + status_ = TestFail;
> + return;
> }
>
> - camera_ = cm_->get("VIMC Sensor B");
> + camera_ = cm_->get(name);
> if (!camera_) {
> - cout << "Can not find VIMC camera" << endl;
> - return TestSkip;
> + cerr << "Can not find '" << name << "' camera" << endl;
> + status_ = TestSkip;
> + return;
> }
>
> /* Sanity check that the camera has streams. */
> if (camera_->streams().empty()) {
> - cout << "Camera has no stream" << endl;
> - return TestFail;
> + cerr << "Camera has no stream" << endl;
> + status_ = TestFail;
> + return;
> }
>
> - return TestPass;
> + status_ = TestPass;
> }
>
> -void CameraTest::cleanup()
> +CameraTest::~CameraTest()
> {
> if (camera_) {
> camera_->release();
> diff --git a/test/camera/camera_test.h b/test/libtest/camera_test.h
> similarity index 77%
> rename from test/camera/camera_test.h
> rename to test/libtest/camera_test.h
> index e57b05eb28a9..0b6bad05e37c 100644
> --- a/test/camera/camera_test.h
> +++ b/test/libtest/camera_test.h
> @@ -9,22 +9,18 @@
>
> #include <libcamera/libcamera.h>
>
> -#include "test.h"
> -
> using namespace libcamera;
>
> -class CameraTest : public Test
> +class CameraTest
> {
> public:
> - CameraTest()
> - : cm_(nullptr) {}
> + CameraTest(const char *name);
> + ~CameraTest();
>
> protected:
> - int init();
> - void cleanup();
> -
> CameraManager *cm_;
> std::shared_ptr<Camera> camera_;
> + int status_;
> };
>
> #endif /* __LIBCAMERA_CAMERA_TEST_H__ */
> diff --git a/test/libtest/meson.build b/test/libtest/meson.build
> index ca762b4421c2..3e798ef3810e 100644
> --- a/test/libtest/meson.build
> +++ b/test/libtest/meson.build
> @@ -1,4 +1,5 @@
> libtest_sources = files([
> + 'camera_test.cpp',
> 'test.cpp',
> ])
>
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list