[libcamera-devel] [PATCH v3 4/4] lc-compliance: Add list and filter parameters
Nícolas F. R. A. Prado
nfraprado at collabora.com
Thu May 20 19:46:21 CEST 2021
Hi Niklas,
thanks for the feedback.
Em 2021-05-16 11:06, Niklas Söderlund escreveu:
> Hi Nícolas,
>
> Thanks for your work.
>
> On 2021-05-14 10:16:52 -0300, Nícolas F. R. A. Prado wrote:
> > Add a --list parameter that lists all current tests (by mapping to
> > googletest's --gtest_list_tests).
> >
> > Add a --filter 'filterString' parameter that filters the tests to run
> > (by mapping to googletest's --gtest_filter).
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> > ---
> > src/lc-compliance/main.cpp | 68 ++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 66 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp
> > index d5ff93f514df..bebf1c51f64b 100644
> > --- a/src/lc-compliance/main.cpp
> > +++ b/src/lc-compliance/main.cpp
> > @@ -20,6 +20,8 @@ using namespace libcamera;
> >
> > enum {
> > OptCamera = 'c',
> > + OptList = 'l',
> > + OptFilter = 'f',
> > OptHelp = 'h',
> > };
> >
> > @@ -54,16 +56,26 @@ public:
> > ~Harness();
> >
> > int init();
> > + int buildGtestParameters(char *arg0);
> > + int gtestArgc() { return gtestArgc_; };
> > + char **gtestArgv() { return gtestArgv_; };
>
> This feels a bit like a layer violation.
>
> Do you think it would make sens to instead move the gtest data
> structures inside the Harness class? Or perhaps the Harness class is not
> as useful now that it likely won't grow to support all the test
> infrastructure provided by gtest and should instead be removed and what
> is left of it moved to main() and such helpers?
You mean moving
::testing::InitGoogleTest(>estArgc, harness.gtestArgv());
testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
return RUN_ALL_TESTS();
inside the Harness function and removing those getters, right? Yeah, seems like
a good idea.
Thanks,
Nícolas
>
> >
> > private:
> > void listCameras();
> >
> > OptionsParser::Options options_;
> > std::unique_ptr<CameraManager> cm_;
> > +
> > + const std::map<std::string, std::string> gtestFlag_ = {{"list", "--gtest_list_tests"},
> > + {"filter", "--gtest_filter"}};
> > +
> > + int gtestArgc_;
> > + char **gtestArgv_;
> > + std::string gtestFilterParam_;
> > };
> >
> > Harness::Harness(const OptionsParser::Options &options)
> > - : options_(options)
> > + : options_(options), gtestArgv_(nullptr)
> > {
> > cm_ = std::make_unique<CameraManager>();
> > }
> > @@ -73,6 +85,8 @@ Harness::~Harness()
> > Environment::instance()->destroy();
> >
> > cm_->stop();
> > +
> > + free(gtestArgv_);
> > }
> >
> > int Harness::init()
> > @@ -86,6 +100,9 @@ int Harness::init()
> > return ret;
> > }
> >
> > + if (options_.isSet(OptList))
> > + return 0;
> > +
> > if (!options_.isSet(OptCamera)) {
> > std::cout << "No camera specified, available cameras:" << std::endl;
> > listCameras();
> > @@ -118,12 +135,54 @@ void Harness::listCameras()
> > std::cout << "- " << cam.get()->id() << std::endl;
> > }
> >
> > +int Harness::buildGtestParameters(char *arg0)
> > +{
> > + int argc = 0;
> > +
> > + /*
> > + * +2 to have space for both the 0th argument that is needed but not
> > + * used and the null at the end.
> > + */
> > + gtestArgv_ = (char**) malloc((gtestFlag_.size() + 2) * sizeof(char*));
> > + if (!gtestArgv_)
> > + return -ENOMEM;
> > +
> > + gtestArgv_[argc] = arg0;
> > + argc++;
> > +
> > + if (options_.isSet(OptList)) {
> > + gtestArgv_[argc] = const_cast<char*>(gtestFlag_.at("list").c_str());
> > + argc++;
> > + }
> > +
> > + if (options_.isSet(OptFilter)) {
> > + /*
> > + * The filter flag needs to be passed as a single parameter, in
> > + * the format --gtest_filter=filterStr
> > + */
> > + const std::string &filter = options_[OptFilter];
> > + gtestFilterParam_ = gtestFlag_.at("filter") + "=" + filter;
> > +
> > + gtestArgv_[argc] = const_cast<char*>(gtestFilterParam_.c_str());
> > + argc++;
> > + }
> > +
> > + gtestArgv_[argc] = 0;
> > + gtestArgc_ = argc;
> > +
> > + return 0;
> > +}
> > +
> > 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(OptList, OptionNone, "List all tests and exit", "list");
> > + parser.addOption(OptFilter, OptionString,
> > + "Specify which tests to run", "filter",
> > + ArgumentRequired, "filter");
> > parser.addOption(OptHelp, OptionNone, "Display this help message",
> > "help");
> >
> > @@ -170,7 +229,12 @@ int main(int argc, char **argv)
> > if (ret)
> > return ret;
> >
> > - ::testing::InitGoogleTest(&argc, argv);
> > + ret = harness.buildGtestParameters(argv[0]);
> > + if (ret)
> > + return ret;
> > + int gtestArgc = harness.gtestArgc();
> > +
> > + ::testing::InitGoogleTest(>estArgc, harness.gtestArgv());
> > testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
> > return RUN_ALL_TESTS();
> > }
> > --
> > 2.31.1
> >
>
> --
> Regards,
> Niklas Söderlund
More information about the libcamera-devel
mailing list