[libcamera-devel,v5,5/5] lc-compliance: Add list and filter parameters
diff mbox series

Message ID 20210524191309.90238-6-nfraprado@collabora.com
State Superseded
Headers show
Series
  • lc-compliance: Refactor using Googletest
Related show

Commit Message

Nícolas F. R. A. Prado May 24, 2021, 7:13 p.m. UTC
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@collabora.com>
---
Changes in v5:
- Thanks to Niklas:
  - Moved buildGtestParameters() inside run()

No changes in v4

 src/lc-compliance/main.cpp | 71 +++++++++++++++++++++++++++++++++++---
 1 file changed, 66 insertions(+), 5 deletions(-)

Comments

Niklas Söderlund June 4, 2021, 9:37 p.m. UTC | #1
Hi Nícolas,

Thanks for your work.

On 2021-05-24 16:13:09 -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@collabora.com>
> ---
> Changes in v5:
> - Thanks to Niklas:
>   - Moved buildGtestParameters() inside run()
> 
> 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 27503372d0eb..182807a14c31 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',
>  };
>  
> @@ -45,17 +47,25 @@ public:
>  	~Harness();
>  
>  	int init();
> -	int run(int argc, char **argv);
> +	int run(char *arg0);
> +	int buildGtestParameters(char *arg0);

buildGtestParameters() don't need to be public does it?

>  
>  private:
>  	void listCameras();
>  
>  	OptionsParser::Options options_;
>  	std::shared_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_shared<CameraManager>();
>  }
> @@ -63,6 +73,8 @@ Harness::Harness(const OptionsParser::Options &options)
>  Harness::~Harness()
>  {
>  	cm_->stop();
> +
> +	free(gtestArgv_);
>  }
>  
>  int Harness::init()
> @@ -76,6 +88,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();
> @@ -97,9 +112,13 @@ int Harness::init()
>  	return 0;
>  }
>  
> -int Harness::run(int argc, char **argv)
> +int Harness::run(char *arg0)
>  {
> -	::testing::InitGoogleTest(&argc, argv);
> +	int ret = buildGtestParameters(arg0);
> +	if (ret)
> +		return ret;
> +
> +	::testing::InitGoogleTest(&gtestArgc_, gtestArgv_);

It looks like this could be reworked so gtestArgc_ and gtestArgv_ don't 
need to be class members right. Could not ::testing::InitGoogleTest() be 
called inside buildGtestParameters() (with a possible rename)?

>  
>  	testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
>  
> @@ -112,12 +131,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");
>  
> @@ -147,5 +208,5 @@ int main(int argc, char **argv)
>  	if (ret)
>  		return ret;
>  
> -	return harness.run(argc, argv);
> +	return harness.run(argv[0]);
>  }
> -- 
> 2.31.1
>
Nícolas F. R. A. Prado June 7, 2021, 3:30 p.m. UTC | #2
Hi Niklas,

On Fri, Jun 04, 2021 at 11:37:06PM +0200, Niklas Söderlund wrote:
> Hi Nícolas,
> 
> Thanks for your work.
> 
> On 2021-05-24 16:13:09 -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@collabora.com>
> > ---
> > Changes in v5:
> > - Thanks to Niklas:
> >   - Moved buildGtestParameters() inside run()
> > 
> > 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 27503372d0eb..182807a14c31 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',
> >  };
> >  
> > @@ -45,17 +47,25 @@ public:
> >  	~Harness();
> >  
> >  	int init();
> > -	int run(int argc, char **argv);
> > +	int run(char *arg0);
> > +	int buildGtestParameters(char *arg0);
> 
> buildGtestParameters() don't need to be public does it?

Right, it doesn't.

> 
> >  
> >  private:
> >  	void listCameras();
> >  
> >  	OptionsParser::Options options_;
> >  	std::shared_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_shared<CameraManager>();
> >  }
> > @@ -63,6 +73,8 @@ Harness::Harness(const OptionsParser::Options &options)
> >  Harness::~Harness()
> >  {
> >  	cm_->stop();
> > +
> > +	free(gtestArgv_);
> >  }
> >  
> >  int Harness::init()
> > @@ -76,6 +88,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();
> > @@ -97,9 +112,13 @@ int Harness::init()
> >  	return 0;
> >  }
> >  
> > -int Harness::run(int argc, char **argv)
> > +int Harness::run(char *arg0)
> >  {
> > -	::testing::InitGoogleTest(&argc, argv);
> > +	int ret = buildGtestParameters(arg0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	::testing::InitGoogleTest(&gtestArgc_, gtestArgv_);
> 
> It looks like this could be reworked so gtestArgc_ and gtestArgv_ don't 
> need to be class members right. Could not ::testing::InitGoogleTest() be 
> called inside buildGtestParameters() (with a possible rename)?

I think we could do this to remove gtestArgc_, but we still need to keep track
of gtestArgv_ as a class member so we can free() that pointer in ~Harness(),
otherwise we'll leak its memory, right?

Thanks,
Nícolas

> 
> >  
> >  	testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
> >  
> > @@ -112,12 +131,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");
> >  
> > @@ -147,5 +208,5 @@ int main(int argc, char **argv)
> >  	if (ret)
> >  		return ret;
> >  
> > -	return harness.run(argc, argv);
> > +	return harness.run(argv[0]);
> >  }
> > -- 
> > 2.31.1
> > 
> 
> -- 
> Regards,
> Niklas Söderlund
Niklas Söderlund June 7, 2021, 4:50 p.m. UTC | #3
Hi Nícolas,

On 2021-06-07 12:30:11 -0300, Nícolas F. R. A. Prado wrote:
> Hi Niklas,
> 
> On Fri, Jun 04, 2021 at 11:37:06PM +0200, Niklas Söderlund wrote:
> > Hi Nícolas,
> > 
> > Thanks for your work.
> > 
> > On 2021-05-24 16:13:09 -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@collabora.com>
> > > ---
> > > Changes in v5:
> > > - Thanks to Niklas:
> > >   - Moved buildGtestParameters() inside run()
> > > 
> > > 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 27503372d0eb..182807a14c31 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',
> > >  };
> > >  
> > > @@ -45,17 +47,25 @@ public:
> > >  	~Harness();
> > >  
> > >  	int init();
> > > -	int run(int argc, char **argv);
> > > +	int run(char *arg0);
> > > +	int buildGtestParameters(char *arg0);
> > 
> > buildGtestParameters() don't need to be public does it?
> 
> Right, it doesn't.
> 
> > 
> > >  
> > >  private:
> > >  	void listCameras();
> > >  
> > >  	OptionsParser::Options options_;
> > >  	std::shared_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_shared<CameraManager>();
> > >  }
> > > @@ -63,6 +73,8 @@ Harness::Harness(const OptionsParser::Options &options)
> > >  Harness::~Harness()
> > >  {
> > >  	cm_->stop();
> > > +
> > > +	free(gtestArgv_);
> > >  }
> > >  
> > >  int Harness::init()
> > > @@ -76,6 +88,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();
> > > @@ -97,9 +112,13 @@ int Harness::init()
> > >  	return 0;
> > >  }
> > >  
> > > -int Harness::run(int argc, char **argv)
> > > +int Harness::run(char *arg0)
> > >  {
> > > -	::testing::InitGoogleTest(&argc, argv);
> > > +	int ret = buildGtestParameters(arg0);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	::testing::InitGoogleTest(&gtestArgc_, gtestArgv_);
> > 
> > It looks like this could be reworked so gtestArgc_ and gtestArgv_ don't 
> > need to be class members right. Could not ::testing::InitGoogleTest() be 
> > called inside buildGtestParameters() (with a possible rename)?
> 
> I think we could do this to remove gtestArgc_, but we still need to keep track
> of gtestArgv_ as a class member so we can free() that pointer in ~Harness(),
> otherwise we'll leak its memory, right?

Could we not free it right after calling ::testing::InitGoogleTest() 
instead of the destructor?

> 
> Thanks,
> Nícolas
> 
> > 
> > >  
> > >  	testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
> > >  
> > > @@ -112,12 +131,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");
> > >  
> > > @@ -147,5 +208,5 @@ int main(int argc, char **argv)
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > -	return harness.run(argc, argv);
> > > +	return harness.run(argv[0]);
> > >  }
> > > -- 
> > > 2.31.1
> > > 
> > 
> > -- 
> > Regards,
> > Niklas Söderlund
Nícolas F. R. A. Prado June 7, 2021, 5:05 p.m. UTC | #4
Hi Niklas,

On Mon, Jun 07, 2021 at 06:50:50PM +0200, Niklas Söderlund wrote:
> Hi Nícolas,
> 
> On 2021-06-07 12:30:11 -0300, Nícolas F. R. A. Prado wrote:
> > Hi Niklas,
> > 
> > On Fri, Jun 04, 2021 at 11:37:06PM +0200, Niklas Söderlund wrote:
> > > Hi Nícolas,
> > > 
> > > Thanks for your work.
> > > 
> > > On 2021-05-24 16:13:09 -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@collabora.com>
> > > > ---
> > > > Changes in v5:
> > > > - Thanks to Niklas:
> > > >   - Moved buildGtestParameters() inside run()
> > > > 
> > > > 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 27503372d0eb..182807a14c31 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',
> > > >  };
> > > >  
> > > > @@ -45,17 +47,25 @@ public:
> > > >  	~Harness();
> > > >  
> > > >  	int init();
> > > > -	int run(int argc, char **argv);
> > > > +	int run(char *arg0);
> > > > +	int buildGtestParameters(char *arg0);
> > > 
> > > buildGtestParameters() don't need to be public does it?
> > 
> > Right, it doesn't.
> > 
> > > 
> > > >  
> > > >  private:
> > > >  	void listCameras();
> > > >  
> > > >  	OptionsParser::Options options_;
> > > >  	std::shared_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_shared<CameraManager>();
> > > >  }
> > > > @@ -63,6 +73,8 @@ Harness::Harness(const OptionsParser::Options &options)
> > > >  Harness::~Harness()
> > > >  {
> > > >  	cm_->stop();
> > > > +
> > > > +	free(gtestArgv_);
> > > >  }
> > > >  
> > > >  int Harness::init()
> > > > @@ -76,6 +88,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();
> > > > @@ -97,9 +112,13 @@ int Harness::init()
> > > >  	return 0;
> > > >  }
> > > >  
> > > > -int Harness::run(int argc, char **argv)
> > > > +int Harness::run(char *arg0)
> > > >  {
> > > > -	::testing::InitGoogleTest(&argc, argv);
> > > > +	int ret = buildGtestParameters(arg0);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	::testing::InitGoogleTest(&gtestArgc_, gtestArgv_);
> > > 
> > > It looks like this could be reworked so gtestArgc_ and gtestArgv_ don't 
> > > need to be class members right. Could not ::testing::InitGoogleTest() be 
> > > called inside buildGtestParameters() (with a possible rename)?
> > 
> > I think we could do this to remove gtestArgc_, but we still need to keep track
> > of gtestArgv_ as a class member so we can free() that pointer in ~Harness(),
> > otherwise we'll leak its memory, right?
> 
> Could we not free it right after calling ::testing::InitGoogleTest() 
> instead of the destructor?

Just tested it and indeed we can. I was under the impression Gtest relied on
that data after the call, but I guess it must copy it internally before
returning then.

I'll change it for v6 as well then.

Thanks,
Nícolas

Patch
diff mbox series

diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp
index 27503372d0eb..182807a14c31 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',
 };
 
@@ -45,17 +47,25 @@  public:
 	~Harness();
 
 	int init();
-	int run(int argc, char **argv);
+	int run(char *arg0);
+	int buildGtestParameters(char *arg0);
 
 private:
 	void listCameras();
 
 	OptionsParser::Options options_;
 	std::shared_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_shared<CameraManager>();
 }
@@ -63,6 +73,8 @@  Harness::Harness(const OptionsParser::Options &options)
 Harness::~Harness()
 {
 	cm_->stop();
+
+	free(gtestArgv_);
 }
 
 int Harness::init()
@@ -76,6 +88,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();
@@ -97,9 +112,13 @@  int Harness::init()
 	return 0;
 }
 
-int Harness::run(int argc, char **argv)
+int Harness::run(char *arg0)
 {
-	::testing::InitGoogleTest(&argc, argv);
+	int ret = buildGtestParameters(arg0);
+	if (ret)
+		return ret;
+
+	::testing::InitGoogleTest(&gtestArgc_, gtestArgv_);
 
 	testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
 
@@ -112,12 +131,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");
 
@@ -147,5 +208,5 @@  int main(int argc, char **argv)
 	if (ret)
 		return ret;
 
-	return harness.run(argc, argv);
+	return harness.run(argv[0]);
 }