[libcamera-devel] [PATCH v7 4/5] lc-compliance: Refactor using Googletest
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Jun 15 05:40:50 CEST 2021
A few other comments:
On Tue, Jun 15, 2021 at 06:28:43AM +0300, Laurent Pinchart wrote:
> Hi Nícolas,
>
> Thank you for the patch.
>
> On Thu, Jun 10, 2021 at 03:37:31PM -0300, Nícolas F. R. A. Prado wrote:
> > Refactor lc-compliance using Googletest as the test framework.
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > Changes in v7:
> > - Thanks to Jacopo:
> > - Made CameraManager a unique_ptr and passed to Environment as raw pointer
> >
> > No changes in v6
> >
> > Changes in v5:
> > - Thanks to Laurent:
> > - Fixed style issues
> > - Added SetUp and TearDown functions to acquire and release the camera for
> > each test
> >
> > Changes in v4:
> > - Removed old lc-compliance results classes
> > - Thanks to Niklas:
> > - Improved naming of tests
> >
> > Changes in v3:
> > - Thanks to Niklas:
> > - Went back to static test registration, and created a Singleton Environment
> > class to store the camera global variable
> > - Added a nameParameters() function to give meaningful names to the test
> > parameters, removing the need to register each test suite for every role
> >
> > src/lc-compliance/main.cpp | 73 +++++------
> > src/lc-compliance/meson.build | 4 +-
> > src/lc-compliance/results.cpp | 75 ------------
> > src/lc-compliance/results.h | 47 --------
> > src/lc-compliance/simple_capture.cpp | 74 +++++-------
> > src/lc-compliance/simple_capture.h | 9 +-
> > src/lc-compliance/single_stream.cpp | 174 ++++++++++++++++-----------
> > src/lc-compliance/tests.h | 16 ---
> > 8 files changed, 175 insertions(+), 297 deletions(-)
> > delete mode 100644 src/lc-compliance/results.cpp
> > delete mode 100644 src/lc-compliance/results.h
> > delete mode 100644 src/lc-compliance/tests.h
> >
> > diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp
> > index 54cee54aa978..8c18845141de 100644
> > --- a/src/lc-compliance/main.cpp
> > +++ b/src/lc-compliance/main.cpp
> > @@ -9,10 +9,12 @@
> > #include <iostream>
> > #include <string.h>
> >
> > +#include <gtest/gtest.h>
> > +
> > #include <libcamera/libcamera.h>
> >
> > +#include "environment.h"
> > #include "../cam/options.h"
> > -#include "tests.h"
> >
> > using namespace libcamera;
> >
> > @@ -21,21 +23,35 @@ enum {
> > OptHelp = 'h',
> > };
> >
> > +/*
> > + * 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);
> > + }
> > +};
> > +
> > class Harness
> > {
> > public:
> > Harness(const OptionsParser::Options &options);
> > ~Harness();
> >
> > - int exec();
> > + int init();
> > + int run(int argc, char **argv);
> >
> > private:
> > - int init();
> > void listCameras();
> >
> > OptionsParser::Options options_;
> > std::unique_ptr<CameraManager> cm_;
> > - std::shared_ptr<Camera> camera_;
> > };
> >
> > Harness::Harness(const OptionsParser::Options &options)
> > @@ -46,35 +62,13 @@ Harness::Harness(const OptionsParser::Options &options)
> >
> > Harness::~Harness()
> > {
> > - if (camera_) {
> > - camera_->release();
> > - camera_.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()
> > {
> > + std::shared_ptr<Camera> camera;
> > +
> > int ret = cm_->start();
> > if (ret) {
> > std::cout << "Failed to start camera manager: "
> > @@ -89,23 +83,29 @@ int Harness::init()
> > }
> >
> > const std::string &cameraId = options_[OptCamera];
> > - camera_ = cm_->get(cameraId);
> > - if (!camera_) {
> > + camera = cm_->get(cameraId);
> > + if (!camera) {
> > std::cout << "Camera " << cameraId << " not found, available cameras:" << std::endl;
> > listCameras();
> > return -ENODEV;
> > }
> >
> > - if (camera_->acquire()) {
> > - std::cout << "Failed to acquire camera" << std::endl;
> > - return -EINVAL;
> > - }
> > + Environment::get()->setup(cm_.get(), cameraId);
> >
> > std::cout << "Using camera " << cameraId << std::endl;
> >
> > return 0;
> > }
> >
> > +int Harness::run(int argc, char **argv)
> > +{
> > + ::testing::InitGoogleTest(&argc, argv);
> > +
> > + testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
> > +
> > + return RUN_ALL_TESTS();
> > +}
> > +
> > void Harness::listCameras()
> > {
> > for (const std::shared_ptr<Camera> &cam : cm_->cameras())
> > @@ -143,6 +143,9 @@ int main(int argc, char **argv)
> > return EXIT_FAILURE;
> >
> > Harness harness(options);
> > + ret = harness.init();
> > + if (ret)
> > + return ret;
> >
> > - return harness.exec() ? EXIT_FAILURE : EXIT_SUCCESS;
> > + return harness.run(argc, argv);
> > }
> > diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build
> > index 6dd49085569f..156b938ee9ad 100644
> > --- a/src/lc-compliance/meson.build
> > +++ b/src/lc-compliance/meson.build
> > @@ -14,15 +14,17 @@ lc_compliance_sources = files([
> > '../cam/options.cpp',
> > 'environment.cpp',
> > 'main.cpp',
> > - 'results.cpp',
> > 'simple_capture.cpp',
> > 'single_stream.cpp',
> > ])
> >
> > +libgtest = dependency('gtest')
>
> This will cause configuration to fail if gtest isn't found. As
> lc-compliance is a feature option, the auto case needs to be handled
> correctly. You can see at the top of this file how the dependency on
> libevent is handled, and do the same for gtest.
>
> > +
> > lc_compliance = executable('lc-compliance', lc_compliance_sources,
> > dependencies : [
> > libatomic,
> > libcamera_dep,
> > libevent,
> > + libgtest,
> > ],
> > install : true)
> > diff --git a/src/lc-compliance/results.cpp b/src/lc-compliance/results.cpp
> > deleted file mode 100644
> > index f149f7859e28..000000000000
> > --- a/src/lc-compliance/results.cpp
> > +++ /dev/null
> > @@ -1,75 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0-or-later */
> > -/*
> > - * Copyright (C) 2020, Google Inc.
> > - *
> > - * results.cpp - Test result aggregator
> > - */
> > -
> > -#include "results.h"
> > -
> > -#include <iostream>
> > -
> > -void Results::add(const Result &result)
> > -{
> > - if (result.first == Pass)
> > - passed_++;
> > - else if (result.first == Fail)
> > - failed_++;
> > - else if (result.first == Skip)
> > - skipped_++;
> > -
> > - printResult(result);
> > -}
> > -
> > -void Results::add(Status status, const std::string &message)
> > -{
> > - add({ status, message });
> > -}
> > -
> > -void Results::fail(const std::string &message)
> > -{
> > - add(Fail, message);
> > -}
> > -
> > -void Results::pass(const std::string &message)
> > -{
> > - add(Pass, message);
> > -}
> > -
> > -void Results::skip(const std::string &message)
> > -{
> > - add(Skip, message);
> > -}
> > -
> > -int Results::summary() const
> > -{
> > - if (failed_ + passed_ + skipped_ != planned_) {
> > - std::cout << "Planned and executed number of tests differ "
> > - << failed_ + passed_ + skipped_ << " executed "
> > - << planned_ << " planned" << std::endl;
> > -
> > - return -EINVAL;
> > - }
> > -
> > - std::cout << planned_ << " tests executed, "
> > - << passed_ << " tests passed, "
> > - << skipped_ << " tests skipped and "
> > - << failed_ << " tests failed " << std::endl;
> > -
> > - return 0;
> > -}
> > -
> > -void Results::printResult(const Result &result)
> > -{
> > - std::string prefix;
> > -
> > - /* \todo Make parsable as TAP. */
> > - if (result.first == Pass)
> > - prefix = "PASS";
> > - else if (result.first == Fail)
> > - prefix = "FAIL";
> > - else if (result.first == Skip)
> > - prefix = "SKIP";
> > -
> > - std::cout << "- " << prefix << ": " << result.second << std::endl;
> > -}
> > diff --git a/src/lc-compliance/results.h b/src/lc-compliance/results.h
> > deleted file mode 100644
> > index 2a3722b841a6..000000000000
> > --- a/src/lc-compliance/results.h
> > +++ /dev/null
> > @@ -1,47 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0-or-later */
> > -/*
> > - * Copyright (C) 2020, Google Inc.
> > - *
> > - * results.h - Test result aggregator
> > - */
> > -#ifndef __LC_COMPLIANCE_RESULTS_H__
> > -#define __LC_COMPLIANCE_RESULTS_H__
> > -
> > -#include <string>
> > -#include <utility>
> > -
> > -/* \todo Check if result aggregator can be shared with self tests in test/ */
> > -class Results
> > -{
> > -public:
> > - enum Status {
> > - Fail,
> > - Pass,
> > - Skip,
> > - };
> > -
> > - using Result = std::pair<Status, std::string>;
> > -
> > - Results(unsigned int planned)
> > - : planned_(planned), passed_(0), failed_(0), skipped_(0)
> > - {
> > - }
> > -
> > - void add(const Result &result);
> > - void add(Status status, const std::string &message);
> > - void fail(const std::string &message);
> > - void pass(const std::string &message);
> > - void skip(const std::string &message);
> > -
> > - int summary() const;
> > -
> > -private:
> > - void printResult(const Result &result);
> > -
> > - unsigned int planned_;
> > - unsigned int passed_;
> > - unsigned int failed_;
> > - unsigned int skipped_;
> > -};
> > -
> > -#endif /* __LC_COMPLIANCE_RESULTS_H__ */
> > diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp
> > index bfc91b26444e..d5a9d9e8cebe 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()
> > @@ -74,22 +72,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;
> > @@ -100,14 +95,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));
> > }
> > @@ -118,12 +110,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)
> > @@ -155,11 +142,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);
> > @@ -171,14 +156,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));
> > }
> > @@ -189,7 +171,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..62984243a1fa 100644
> > --- a/src/lc-compliance/simple_capture.h
> > +++ b/src/lc-compliance/simple_capture.h
> > @@ -12,18 +12,17 @@
> > #include <libcamera/libcamera.h>
> >
> > #include "../cam/event_loop.h"
> > -#include "results.h"
> >
> > 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 +39,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 +55,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..837e17a1c189 100644
> > --- a/src/lc-compliance/single_stream.cpp
> > +++ b/src/lc-compliance/single_stream.cpp
> > @@ -7,91 +7,121 @@
> >
> > #include <iostream>
> >
> > +#include <gtest/gtest.h>
> > +
> > +#include "environment.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)
> > +const std::vector<int> NUMREQUESTS = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
> > +const std::vector<StreamRole> ROLES = { Raw, StillCapture, VideoRecording, Viewfinder };
> > +
> > +class FixedRequestsTest : public testing::TestWithParam<std::tuple<StreamRole, int>>
> > {
> > - SimpleCaptureBalanced capture(camera);
> > +public:
> > + static std::string nameParameters(const testing::TestParamInfo<FixedRequestsTest::ParamType> &info);
> >
> > - Results::Result ret = capture.configure(role);
> > - if (ret.first != Results::Pass)
> > - return ret;
> > +protected:
> > + void SetUp() override;
> > + void TearDown() override;
> >
> > - for (unsigned int starts = 0; starts < startCycles; starts++) {
> > - ret = capture.capture(numRequests);
> > - if (ret.first != Results::Pass)
> > - return ret;
> > - }
> > + std::shared_ptr<Camera> camera_;
> > +};
> >
> > - return { Results::Pass, "Balanced capture of " +
> > - std::to_string(numRequests) + " requests with " +
> > - std::to_string(startCycles) + " start cycles" };
> > +/*
> > + * We use gtest's SetUp() and TearDown() instead of constructor and destructor
> > + * in order to be able to assert on them.
> > + */
> > +void FixedRequestsTest::SetUp()
> > +{
> > + Environment *env = Environment::get();
> > +
> > + camera_ = env->cm()->get(env->cameraId());
> > +
> > + ASSERT_FALSE(camera_->acquire());
> > }
> >
> > -Results::Result testRequestUnbalance(std::shared_ptr<Camera> camera,
> > - StreamRole role, unsigned int numRequests)
> > +void FixedRequestsTest::TearDown()
> > {
> > - SimpleCaptureUnbalanced capture(camera);
> > + if (!camera_)
> > + return;
> >
> > - Results::Result ret = capture.configure(role);
> > - if (ret.first != Results::Pass)
> > - return ret;
> > -
> > - return capture.capture(numRequests);
> > + camera_->release();
> > + camera_.reset();
> > }
> >
> > -Results testSingleStream(std::shared_ptr<Camera> camera)
> > +std::string FixedRequestsTest::nameParameters(const testing::TestParamInfo<FixedRequestsTest::ParamType> &info)
> > {
> > - static const std::vector<std::pair<std::string, StreamRole>> roles = {
> > - { "raw", Raw },
> > - { "still", StillCapture },
> > - { "video", VideoRecording },
> > - { "viewfinder", Viewfinder },
> > - };
> > - 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;
> > + std::map<StreamRole, std::string> rolesMap = { { Raw, "Raw" },
> > + { StillCapture, "StillCapture" },
> > + { VideoRecording, "VideoRecording" },
> > + { Viewfinder, "Viewfinder" } };
> > +
> > + std::string roleName = rolesMap[std::get<0>(info.param)];
> > + std::string numRequestsName = std::to_string(std::get<1>(info.param));
> > +
> > + return roleName + "_" + numRequestsName;
> > }
> > +
> > +/*
> > + * 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.
> > + */
> > +TEST_P(FixedRequestsTest, BalancedSingleCycle)
> > +{
> > + auto [role, numRequests] = GetParam();
> > +
> > + SimpleCaptureBalanced capture(camera_);
> > +
> > + capture.configure(role);
> > +
> > + capture.capture(numRequests);
> > +};
When compiling with clang,
../../src/lc-compliance/single_stream.cpp:83:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
};
^
../../src/lc-compliance/single_stream.cpp:103:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
};
^
../../src/lc-compliance/single_stream.cpp:121:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
};
^
3 errors generated.
There's another more annoying issue:
[15/79] Linking target src/lc-compliance/lc-compliance
FAILED: src/lc-compliance/lc-compliance
clang++-10 -o src/lc-compliance/lc-compliance src/lc-compliance/lc-compliance.p/.._cam_event_loop.cpp.o src/lc-compliance/lc-compliance.p/.._cam_options.cpp.o src/lc-compliance/lc-compliance.p/environment.cpp.o src/lc-compliance/lc-compliance.p/main.cpp.o src/lc-compliance/lc-compliance.p/simple_capture.cpp.o src/lc-compliance/lc-compliance.p/single_stream.cpp.o -Wl,--as-needed -Wl,--no-undefined -stdlib=libc++ -Wextra-semi -Wshadow -include config.h -Wno-c99-designator '-Wl,-rpath,$ORIGIN/../libcamera' -Wl,-rpath-link,src/libcamera -Wl,--start-group src/libcamera/libcamera.so -latomic /usr/lib64/libevent_pthreads.so /usr/lib64/libevent.so /usr/lib64/libgtest.so -lpthread -Wl,--end-group
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: src/lc-compliance/lc-compliance.p/single_stream.cpp.o: in function `FixedRequestsTest::SetUp()':
../../src/lc-compliance/single_stream.cpp:42: undefined reference to `testing::internal::GetBoolAssertionFailureMessage(testing::AssertionResult const&, char const*, char const*, char const*)'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: src/lc-compliance/lc-compliance.p/single_stream.cpp.o: in function `testing::internal::ParameterizedTestSuiteInfo<FixedRequestsTest>::RegisterTests()':
/usr/include/gtest/internal/gtest-param-util.h:591: undefined reference to `testing::Message::GetString() const'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: /usr/include/gtest/internal/gtest-param-util.h:604: undefined reference to `testing::internal::InsertSyntheticTestCase(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, testing::internal::CodeLocation, bool)'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: src/lc-compliance/lc-compliance.p/simple_capture.cpp.o: in function `SimpleCapture::start()':
../../src/lc-compliance/simple_capture.cpp:50: undefined reference to `testing::internal::GetBoolAssertionFailureMessage(testing::AssertionResult const&, char const*, char const*, char const*)'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: src/lc-compliance/lc-compliance.p/simple_capture.cpp.o: in function `SimpleCaptureBalanced::capture(unsigned int)':
../../src/lc-compliance/simple_capture.cpp:98: undefined reference to `testing::internal::GetBoolAssertionFailureMessage(testing::AssertionResult const&, char const*, char const*, char const*)'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: ../../src/lc-compliance/simple_capture.cpp:100: undefined reference to `testing::internal::GetBoolAssertionFailureMessage(testing::AssertionResult const&, char const*, char const*, char const*)'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: src/lc-compliance/lc-compliance.p/simple_capture.cpp.o: in function `SimpleCaptureUnbalanced::capture(unsigned int)':
../../src/lc-compliance/simple_capture.cpp:159: undefined reference to `testing::internal::GetBoolAssertionFailureMessage(testing::AssertionResult const&, char const*, char const*, char const*)'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: ../../src/lc-compliance/simple_capture.cpp:161: undefined reference to `testing::internal::GetBoolAssertionFailureMessage(testing::AssertionResult const&, char const*, char const*, char const*)'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: src/lc-compliance/lc-compliance.p/simple_capture.cpp.o:../../src/lc-compliance/simple_capture.cpp:174: more undefined references to `testing::internal::GetBoolAssertionFailureMessage(testing::AssertionResult const&, char const*, char const*, char const*)' follow
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: src/lc-compliance/lc-compliance.p/simple_capture.cpp.o: in function `testing::AssertionResult::AppendMessage(testing::Message const&)':
/usr/include/gtest/gtest.h:357: undefined reference to `testing::Message::GetString() const'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: src/lc-compliance/lc-compliance.p/simple_capture.cpp.o: in function `testing::AssertionResult testing::internal::CmpHelperEQFailure<unsigned int, unsigned int>(char const*, char const*, unsigned int const&, unsigned int const&)':
/usr/include/gtest/gtest.h:1524: undefined reference to `testing::internal::EqFailure(char const*, char const*, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool)'
clang-10: error: linker command failed with exit code 1 (use -v to see invocation)
I expect this to be caused by libcamera linking to libc++ while gtest is
compiled (on my system) against libstdc++. I don't think we can do much
about it, so let's ignore it for now. The semicolon issue should still
be fixed.
> > +
> > +/*
> > + * 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.
> > + */
> > +TEST_P(FixedRequestsTest, BalancedMultiCycle)
> > +{
> > + auto [role, numRequests] = GetParam();
> > + unsigned int numRepeats = 3;
> > +
> > + SimpleCaptureBalanced capture(camera_);
> > +
> > + capture.configure(role);
> > +
> > + for (unsigned int starts = 0; starts < numRepeats; starts++)
> > + capture.capture(numRequests);
> > +};
> > +
> > +/*
> > + * 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.
> > + */
> > +TEST_P(FixedRequestsTest, Unbalanced)
> > +{
> > + auto [role, numRequests] = GetParam();
> > +
> > + SimpleCaptureUnbalanced capture(camera_);
> > +
> > + capture.configure(role);
> > +
> > + capture.capture(numRequests);
> > +};
> > +
> > +INSTANTIATE_TEST_SUITE_P(RolesAndRequests,
> > + FixedRequestsTest,
> > + testing::Combine(testing::ValuesIn(ROLES),
> > + testing::ValuesIn(NUMREQUESTS)),
> > + FixedRequestsTest::nameParameters);
> > diff --git a/src/lc-compliance/tests.h b/src/lc-compliance/tests.h
> > deleted file mode 100644
> > index 396605214e4b..000000000000
> > --- a/src/lc-compliance/tests.h
> > +++ /dev/null
> > @@ -1,16 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0-or-later */
> > -/*
> > - * Copyright (C) 2020, Google Inc.
> > - *
> > - * tests.h - Test modules
> > - */
> > -#ifndef __LC_COMPLIANCE_TESTS_H__
> > -#define __LC_COMPLIANCE_TESTS_H__
> > -
> > -#include <libcamera/libcamera.h>
> > -
> > -#include "results.h"
> > -
> > -Results testSingleStream(std::shared_ptr<libcamera::Camera> camera);
> > -
> > -#endif /* __LC_COMPLIANCE_TESTS_H__ */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list