[libcamera-devel] [RFC PATCH v2 3/3] lc-compliance: Refactor using Googletest

Nícolas F. R. A. Prado nfraprado at collabora.com
Mon May 3 21:33:07 CEST 2021


Refactor lc-compliance using Googletest as the test framework.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
---
Changes in v2:
- Changed from static to dynamic test registration
- Removed -c flag

There's still an issue with the refactoring that the shared_ptr of the tests
apparently aren't being deleted on time, which causes:

	[20:26:08.744507935] [103243] ERROR DeviceEnumerator device_enumerator.cpp:165 Removing media device /dev/media0 while still in use
	[20:26:08.744548175] [103243] ERROR DeviceEnumerator device_enumerator.cpp:165 Removing media device /dev/media1 while still in use
	Segmentation fault (core dumped)

I tried explicitly resetting the shared_ptr on the destructor of the tests but
that didn't work. In fact, when just listing the tests, the constructor for the
tests isn't even called. So I think the issue has to do with the passing of the
camera shared pointer to the REGISTER_TESTS() macro through that functor (?):

	[=]() -> testsuite* { return new testcase(c, r, rq); }); \

Not sure how to solve this. Any tip would be welcome.

 src/lc-compliance/main.cpp           |  99 +++++++++---------
 src/lc-compliance/meson.build        |   3 +
 src/lc-compliance/simple_capture.cpp |  74 +++++--------
 src/lc-compliance/simple_capture.h   |   8 +-
 src/lc-compliance/single_stream.cpp  | 151 ++++++++++++++-------------
 src/lc-compliance/tests.h            |  15 ++-
 6 files changed, 175 insertions(+), 175 deletions(-)

diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp
index 54cee54aa978..add0d7729aec 100644
--- a/src/lc-compliance/main.cpp
+++ b/src/lc-compliance/main.cpp
@@ -9,6 +9,8 @@
 #include <iostream>
 #include <string.h>
 
+#include <gtest/gtest.h>
+
 #include <libcamera/libcamera.h>
 
 #include "../cam/options.h"
@@ -17,7 +19,6 @@
 using namespace libcamera;
 
 enum {
-	OptCamera = 'c',
 	OptHelp = 'h',
 };
 
@@ -28,14 +29,15 @@ public:
 	~Harness();
 
 	int exec();
+	int init();
+	void registerTests();
 
 private:
-	int init();
 	void listCameras();
 
 	OptionsParser::Options options_;
 	std::unique_ptr<CameraManager> cm_;
-	std::shared_ptr<Camera> camera_;
+	std::vector<std::shared_ptr<Camera>> cameras_;
 };
 
 Harness::Harness(const OptionsParser::Options &options)
@@ -46,33 +48,14 @@ Harness::Harness(const OptionsParser::Options &options)
 
 Harness::~Harness()
 {
-	if (camera_) {
-		camera_->release();
-		camera_.reset();
+	for (auto &c : cameras_) {
+		c->release();
+		c.reset();
 	}
 
 	cm_->stop();
 }
 
-int Harness::exec()
-{
-	int ret = init();
-	if (ret)
-		return ret;
-
-	std::vector<Results> results;
-
-	results.push_back(testSingleStream(camera_));
-
-	for (const Results &result : results) {
-		ret = result.summary();
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
 int Harness::init()
 {
 	int ret = cm_->start();
@@ -82,42 +65,26 @@ int Harness::init()
 		return ret;
 	}
 
-	if (!options_.isSet(OptCamera)) {
-		std::cout << "No camera specified, available cameras:" << std::endl;
-		listCameras();
-		return -ENODEV;
-	}
-
-	const std::string &cameraId = options_[OptCamera];
-	camera_ = cm_->get(cameraId);
-	if (!camera_) {
-		std::cout << "Camera " << cameraId << " not found, available cameras:" << std::endl;
-		listCameras();
-		return -ENODEV;
+	for (auto cam : cm_->cameras()) {
+		if (cam->acquire()) {
+			std::cout << "Failed to acquire camera" << std::endl;
+			return -EINVAL;
+		}
+		cameras_.push_back(cam);
 	}
 
-	if (camera_->acquire()) {
-		std::cout << "Failed to acquire camera" << std::endl;
-		return -EINVAL;
-	}
-
-	std::cout << "Using camera " << cameraId << std::endl;
-
 	return 0;
 }
 
 void Harness::listCameras()
 {
-	for (const std::shared_ptr<Camera> &cam : cm_->cameras())
-		std::cout << "- " << cam.get()->id() << std::endl;
+	for (const std::shared_ptr<Camera> &c : cm_->cameras())
+		std::cout << "- " << c.get()->id() << std::endl;
 }
 
 static int parseOptions(int argc, char **argv, OptionsParser::Options *options)
 {
 	OptionsParser parser;
-	parser.addOption(OptCamera, OptionString,
-			 "Specify which camera to operate on, by id", "camera",
-			 ArgumentRequired, "camera");
 	parser.addOption(OptHelp, OptionNone, "Display this help message",
 			 "help");
 
@@ -133,6 +100,31 @@ static int parseOptions(int argc, char **argv, OptionsParser::Options *options)
 	return 0;
 }
 
+/*
+ * Make asserts act like exceptions, otherwise they only fail (or skip) the
+ * current function. From gtest documentation:
+ * https://google.github.io/googletest/advanced.html#asserting-on-subroutines-with-an-exception
+ */
+class ThrowListener : public testing::EmptyTestEventListener {
+	void OnTestPartResult(const testing::TestPartResult& result) override {
+		if (result.type() == testing::TestPartResult::kFatalFailure
+		    || result.type() == testing::TestPartResult::kSkip) {
+			throw testing::AssertionException(result);
+		}
+	}
+};
+
+
+void Harness::registerTests() {
+	std::map<StreamRole, std::string> roles = {{Raw, "Raw"},
+						   {StillCapture, "Still"},
+						   {VideoRecording, "Video"},
+						   {Viewfinder, "Viewfinder"}};
+	std::vector<int> requests = {1, 2, 3, 5, 8, 13, 21, 34, 55, 89};
+
+	registerSingleStreamTests(cameras_, roles, requests);
+}
+
 int main(int argc, char **argv)
 {
 	OptionsParser::Options options;
@@ -143,6 +135,13 @@ int main(int argc, char **argv)
 		return EXIT_FAILURE;
 
 	Harness harness(options);
+	ret = harness.init();
+	if (ret)
+		return ret;
+
+	harness.registerTests();
 
-	return harness.exec() ? EXIT_FAILURE : EXIT_SUCCESS;
+	::testing::InitGoogleTest(&argc, argv);
+	testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
+	return RUN_ALL_TESTS();
 }
diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build
index a2bfcceb1259..704bc18af3e1 100644
--- a/src/lc-compliance/meson.build
+++ b/src/lc-compliance/meson.build
@@ -18,10 +18,13 @@ lc_compliance_sources = files([
     'single_stream.cpp',
 ])
 
+gtest_dep = dependency('gtest')
+
 lc_compliance  = executable('lc-compliance', lc_compliance_sources,
                             dependencies : [
                                 libatomic,
                                 libcamera_dep,
                                 libevent,
+                                gtest_dep,
                             ],
                             install : true)
diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
index f90fe6d0f9aa..7731eb16f8c2 100644
--- a/src/lc-compliance/simple_capture.cpp
+++ b/src/lc-compliance/simple_capture.cpp
@@ -5,6 +5,8 @@
  * simple_capture.cpp - Simple capture helper
  */
 
+#include <gtest/gtest.h>
+
 #include "simple_capture.h"
 
 using namespace libcamera;
@@ -20,38 +22,34 @@ SimpleCapture::~SimpleCapture()
 	stop();
 }
 
-Results::Result SimpleCapture::configure(StreamRole role)
+void SimpleCapture::configure(StreamRole role)
 {
 	config_ = camera_->generateConfiguration({ role });
 
-	if (!config_)
-		return { Results::Skip, "Role not supported by camera" };
+	if (!config_) {
+		std::cout << "Role not supported by camera" << std::endl;
+		GTEST_SKIP();
+	}
 
 	if (config_->validate() != CameraConfiguration::Valid) {
 		config_.reset();
-		return { Results::Fail, "Configuration not valid" };
+		FAIL() << "Configuration not valid";
 	}
 
 	if (camera_->configure(config_.get())) {
 		config_.reset();
-		return { Results::Fail, "Failed to configure camera" };
+		FAIL() << "Failed to configure camera";
 	}
-
-	return { Results::Pass, "Configure camera" };
 }
 
-Results::Result SimpleCapture::start()
+void SimpleCapture::start()
 {
 	Stream *stream = config_->at(0).stream();
-	if (allocator_->allocate(stream) < 0)
-		return { Results::Fail, "Failed to allocate buffers" };
+	ASSERT_GE(allocator_->allocate(stream), 0) << "Failed to allocate buffers";
 
-	if (camera_->start())
-		return { Results::Fail, "Failed to start camera" };
+	ASSERT_TRUE(!camera_->start()) << "Failed to start camera";
 
 	camera_->requestCompleted.connect(this, &SimpleCapture::requestComplete);
-
-	return { Results::Pass, "Started camera" };
 }
 
 void SimpleCapture::stop()
@@ -77,22 +75,19 @@ SimpleCaptureBalanced::SimpleCaptureBalanced(std::shared_ptr<Camera> camera)
 {
 }
 
-Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
+void SimpleCaptureBalanced::capture(unsigned int numRequests)
 {
-	Results::Result ret = start();
-	if (ret.first != Results::Pass)
-		return ret;
+	start();
 
 	Stream *stream = config_->at(0).stream();
 	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
 
 	/* No point in testing less requests then the camera depth. */
 	if (buffers.size() > numRequests) {
-		/* Cache buffers.size() before we destroy it in stop() */
-		int buffers_size = buffers.size();
-
-		return { Results::Skip, "Camera needs " + std::to_string(buffers_size)
-			+ " requests, can't test only " + std::to_string(numRequests) };
+		std::cout << "Camera needs " + std::to_string(buffers.size())
+			+ " requests, can't test only "
+			+ std::to_string(numRequests) << std::endl;
+		GTEST_SKIP();
 	}
 
 	queueCount_ = 0;
@@ -103,14 +98,11 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
 	std::vector<std::unique_ptr<libcamera::Request>> requests;
 	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
 		std::unique_ptr<Request> request = camera_->createRequest();
-		if (!request)
-			return { Results::Fail, "Can't create request" };
+		ASSERT_TRUE(request) << "Can't create request";
 
-		if (request->addBuffer(stream, buffer.get()))
-			return { Results::Fail, "Can't set buffer for request" };
+		ASSERT_FALSE(request->addBuffer(stream, buffer.get())) << "Can't set buffer for request";
 
-		if (queueRequest(request.get()) < 0)
-			return { Results::Fail, "Failed to queue request" };
+		ASSERT_GE(queueRequest(request.get()), 0) << "Failed to queue request";
 
 		requests.push_back(std::move(request));
 	}
@@ -121,12 +113,7 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests)
 	stop();
 	delete loop_;
 
-	if (captureCount_ != captureLimit_)
-		return { Results::Fail, "Got " + std::to_string(captureCount_) +
-			" request, wanted " + std::to_string(captureLimit_) };
-
-	return { Results::Pass, "Balanced capture of " +
-		std::to_string(numRequests) + " requests" };
+	ASSERT_EQ(captureCount_, captureLimit_);
 }
 
 int SimpleCaptureBalanced::queueRequest(Request *request)
@@ -158,11 +145,9 @@ SimpleCaptureUnbalanced::SimpleCaptureUnbalanced(std::shared_ptr<Camera> camera)
 {
 }
 
-Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)
+void SimpleCaptureUnbalanced::capture(unsigned int numRequests)
 {
-	Results::Result ret = start();
-	if (ret.first != Results::Pass)
-		return ret;
+	start();
 
 	Stream *stream = config_->at(0).stream();
 	const std::vector<std::unique_ptr<FrameBuffer>> &buffers = allocator_->buffers(stream);
@@ -174,14 +159,11 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)
 	std::vector<std::unique_ptr<libcamera::Request>> requests;
 	for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
 		std::unique_ptr<Request> request = camera_->createRequest();
-		if (!request)
-			return { Results::Fail, "Can't create request" };
+		ASSERT_TRUE(request) << "Can't create request";
 
-		if (request->addBuffer(stream, buffer.get()))
-			return { Results::Fail, "Can't set buffer for request" };
+		ASSERT_FALSE(request->addBuffer(stream, buffer.get())) << "Can't set buffer for request";
 
-		if (camera_->queueRequest(request.get()) < 0)
-			return { Results::Fail, "Failed to queue request" };
+		ASSERT_GE(camera_->queueRequest(request.get()), 0) << "Failed to queue request";
 
 		requests.push_back(std::move(request));
 	}
@@ -192,7 +174,7 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests)
 	stop();
 	delete loop_;
 
-	return { status ? Results::Fail : Results::Pass, "Unbalanced capture of " + std::to_string(numRequests) + " requests" };
+	ASSERT_FALSE(status);
 }
 
 void SimpleCaptureUnbalanced::requestComplete(Request *request)
diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h
index d9de53fb63a3..0f8465083456 100644
--- a/src/lc-compliance/simple_capture.h
+++ b/src/lc-compliance/simple_capture.h
@@ -17,13 +17,13 @@
 class SimpleCapture
 {
 public:
-	Results::Result configure(libcamera::StreamRole role);
+	void configure(libcamera::StreamRole role);
 
 protected:
 	SimpleCapture(std::shared_ptr<libcamera::Camera> camera);
 	virtual ~SimpleCapture();
 
-	Results::Result start();
+	void start();
 	void stop();
 
 	virtual void requestComplete(libcamera::Request *request) = 0;
@@ -40,7 +40,7 @@ class SimpleCaptureBalanced : public SimpleCapture
 public:
 	SimpleCaptureBalanced(std::shared_ptr<libcamera::Camera> camera);
 
-	Results::Result capture(unsigned int numRequests);
+	void capture(unsigned int numRequests);
 
 private:
 	int queueRequest(libcamera::Request *request);
@@ -56,7 +56,7 @@ class SimpleCaptureUnbalanced : public SimpleCapture
 public:
 	SimpleCaptureUnbalanced(std::shared_ptr<libcamera::Camera> camera);
 
-	Results::Result capture(unsigned int numRequests);
+	void capture(unsigned int numRequests);
 
 private:
 	void requestComplete(libcamera::Request *request) override;
diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp
index 8318b42f42d6..eb6a6f305826 100644
--- a/src/lc-compliance/single_stream.cpp
+++ b/src/lc-compliance/single_stream.cpp
@@ -7,91 +7,94 @@
 
 #include <iostream>
 
+#include <gtest/gtest.h>
+
 #include "simple_capture.h"
 #include "tests.h"
 
 using namespace libcamera;
 
-Results::Result testRequestBalance(std::shared_ptr<Camera> camera,
-				   StreamRole role, unsigned int startCycles,
-				   unsigned int numRequests)
-{
-	SimpleCaptureBalanced capture(camera);
+class SingleStream : public testing::Test {
+public:
+	explicit SingleStream(std::shared_ptr<Camera> camera, StreamRole role, unsigned int numRequests)
+		: camera_(camera), role_(role), numRequests_(numRequests) {}
 
-	Results::Result ret = capture.configure(role);
-	if (ret.first != Results::Pass)
-		return ret;
+protected:
+	std::shared_ptr<Camera> camera_;
+	StreamRole role_;
+	unsigned int numRequests_;
+};
 
-	for (unsigned int starts = 0; starts < startCycles; starts++) {
-		ret = capture.capture(numRequests);
-		if (ret.first != Results::Pass)
-			return ret;
-	}
+/*
+ * Test single capture cycles
+ *
+ * Makes sure the camera completes the exact number of requests queued. Example
+ * failure is a camera that needs N+M requests queued to complete N requests to
+ * the application.
+ */
+class BalancedSingleCycle : public SingleStream {
+public:
+	explicit BalancedSingleCycle(std::shared_ptr<Camera> camera, StreamRole role, unsigned int numRequests)
+		: SingleStream(camera, role, numRequests) {}
 
-	return { Results::Pass, "Balanced capture of " +
-		std::to_string(numRequests) + " requests with " +
-		std::to_string(startCycles) + " start cycles" };
-}
+	void TestBody() override {
+		SimpleCaptureBalanced capture(camera_);
 
-Results::Result testRequestUnbalance(std::shared_ptr<Camera> camera,
-				     StreamRole role, unsigned int numRequests)
-{
-	SimpleCaptureUnbalanced capture(camera);
+		capture.configure(role_);
 
-	Results::Result ret = capture.configure(role);
-	if (ret.first != Results::Pass)
-		return ret;
+		capture.capture(numRequests_);
+	};
+};
 
-	return capture.capture(numRequests);
-}
+/*
+ * Test multiple start/stop cycles
+ *
+ * Makes sure the camera supports multiple start/stop cycles. Example failure is
+ * a camera that does not clean up correctly in its error path but is only
+ * tested by single-capture applications.
+ */
+class BalancedMultiCycle : public SingleStream {
+public:
+	explicit BalancedMultiCycle(std::shared_ptr<Camera> camera, StreamRole role, unsigned int numRequests)
+		: SingleStream(camera, role, numRequests) {}
 
-Results testSingleStream(std::shared_ptr<Camera> camera)
-{
-	static const std::vector<std::pair<std::string, StreamRole>> roles = {
-		{ "raw", Raw },
-		{ "still", StillCapture },
-		{ "video", VideoRecording },
-		{ "viewfinder", Viewfinder },
+	void TestBody() override {
+		unsigned int numRepeats = 3;
+
+		SimpleCaptureBalanced capture(camera_);
+
+		capture.configure(role_);
+
+		for (unsigned int starts = 0; starts < numRepeats; starts++)
+			capture.capture(numRequests_);
 	};
-	static const std::vector<unsigned int> numRequests = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
-
-	Results results(numRequests.size() * roles.size() * 3);
-
-	for (const auto &role : roles) {
-		std::cout << "= Test role " << role.first << std::endl;
-		/*
-		 * Test single capture cycles
-		 *
-		 * Makes sure the camera completes the exact number of requests queued.
-		 * Example failure is a camera that needs N+M requests queued to
-		 * complete N requests to the application.
-		 */
-		std::cout << "* Test single capture cycles" << std::endl;
-		for (unsigned int num : numRequests)
-			results.add(testRequestBalance(camera, role.second, 1, num));
-
-		/*
-		 * Test multiple start/stop cycles
-		 *
-		 * Makes sure the camera supports multiple start/stop cycles.
-		 * Example failure is a camera that does not clean up correctly in its
-		 * error path but is only tested by single-capture applications.
-		 */
-		std::cout << "* Test multiple start/stop cycles" << std::endl;
-		for (unsigned int num : numRequests)
-			results.add(testRequestBalance(camera, role.second, 3, num));
-
-		/*
-		 * Test unbalanced stop
-		 *
-		 * Makes sure the camera supports a stop with requests queued.
-		 * Example failure is a camera that does not handle cancelation
-		 * of buffers coming back from the video device while stopping.
-		 */
-		std::cout << "* Test unbalanced stop" << std::endl;
-		for (unsigned int num : numRequests)
-			results.add(testRequestUnbalance(camera, role.second, num));
-	}
-
-	return results;
+};
+
+/*
+ * Test unbalanced stop
+ *
+ * Makes sure the camera supports a stop with requests queued. Example failure
+ * is a camera that does not handle cancelation of buffers coming back from the
+ * video device while stopping.
+ */
+class Unbalanced : public SingleStream {
+public:
+	explicit Unbalanced(std::shared_ptr<Camera> camera, StreamRole role, unsigned int numRequests)
+		: SingleStream(camera, role, numRequests) {}
+
+	void TestBody() override {
+		SimpleCaptureUnbalanced capture(camera_);
+
+		capture.configure(role_);
+
+		capture.capture(numRequests_);
+	};
+};
+
+void registerSingleStreamTests(std::vector<std::shared_ptr<Camera>> cameras,
+		std::map<StreamRole, std::string> roles, std::vector<int> requests)
+{
+	REGISTER_TESTS(SingleStream, BalancedSingleCycle, cameras, roles, requests);
+	REGISTER_TESTS(SingleStream, BalancedMultiCycle, cameras, roles, requests);
+	REGISTER_TESTS(SingleStream, Unbalanced, cameras, roles, requests);
 }
diff --git a/src/lc-compliance/tests.h b/src/lc-compliance/tests.h
index 396605214e4b..6d5a8b88c287 100644
--- a/src/lc-compliance/tests.h
+++ b/src/lc-compliance/tests.h
@@ -11,6 +11,19 @@
 
 #include "results.h"
 
-Results testSingleStream(std::shared_ptr<libcamera::Camera> camera);
+void registerSingleStreamTests(std::vector<std::shared_ptr<libcamera::Camera>> cameras,
+		std::map<libcamera::StreamRole, std::string> roles, std::vector<int> requests);
+
+#define REGISTER_TESTS(testsuite, testcase, cameras, roles, requests) \
+for (auto c : cameras) { \
+	for (auto [r, rn] : roles) { \
+		for (auto rq : requests) { \
+			testing::RegisterTest( \
+				#testsuite, (std::string(#testcase) + "/" + c->id() + "/" + rn + "/" + std::to_string(rq)).c_str(), \
+				nullptr, nullptr, __FILE__, __LINE__, \
+				[=]() -> testsuite* { return new testcase(c, r, rq); }); \
+		} \
+	} \
+}
 
 #endif /* __LC_COMPLIANCE_TESTS_H__ */
-- 
2.31.1



More information about the libcamera-devel mailing list