[libcamera-devel] [PATCH v4 5/5] lc-compliance: Add list and filter parameters

[Nícolas F. R. A. Prado]

Hi Niklas,

Em 2021-05-22 07:18, Niklas Söderlund escreveu:

> Hi Nícolas,
>
> Thanks for your patch.
>
> On 2021-05-21 10:30:54 -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>
> > ---
> > No changes in v4
> > 
> >  src/lc-compliance/main.cpp | 71 +++++++++++++++++++++++++++++++++++---
> >  1 file changed, 66 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp
> > index 37884ff70a69..92ef9c1fa082 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',
> >  };
> >  
> > @@ -44,17 +46,25 @@ public:
> >  	~Harness();
> >  
> >  	int init();
> > -	int run(int argc, char **argv);
> > +	int run();
> > +	int buildGtestParameters(char *arg0);
> >  
> >  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>();
> >  }
> > @@ -64,6 +74,8 @@ Harness::~Harness()
> >  	Environment::instance()->destroy();
> >  
> >  	cm_->stop();
> > +
> > +	free(gtestArgv_);
> >  }
> >  
> >  int Harness::init()
> > @@ -77,6 +89,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();
> > @@ -103,9 +118,9 @@ int Harness::init()
> >  	return 0;
> >  }
> >  
> > -int Harness::run(int argc, char **argv)
> > +int Harness::run()
> >  {
> > -	::testing::InitGoogleTest(&argc, argv);
> > +	::testing::InitGoogleTest(&gtestArgc_, gtestArgv_);
> >  
> >  	testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
> >  
> > @@ -118,12 +133,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");
> >  
> > @@ -153,5 +210,9 @@ int main(int argc, char **argv)
> >  	if (ret)
> >  		return ret;
> >  
> > -	return harness.run(argc, argv);
> > +	ret = harness.buildGtestParameters(argv[0]);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return harness.run();
>
> This looks a bit odd, would it make sens to do the
> buildGtestParameters() inside of run() instead of in main()?

Yeah, I didn't do that because I didn't want to have run() need the arg0
parameter as well to pass to buildGtestParameters(), but I'll do it for v5.

Thanks,
Nícolas

>
> >  }
> > -- 
> > 2.31.1
> > 
>
> --
> Regards,
> Niklas Söderlund
>
> --
> To unsubscribe, send mail to kernel-unsubscribe at lists.collabora.co.uk.