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

[paul.elder at ideasonboard.com]

Hi Nicolas,

Thank you for the patch.

On Wed, Jun 16, 2021 at 10:25:35AM -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>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> Changes in v8:
> - Thanks to Kieran:
>   - Switched from malloc/free to new/delete in gtest parameter allocation
> 
> Changes in v7:
> - Thanks to Niklas:
>   - Removed intermediary filter string variable in Harness::initGtestParameters()
> 
> Changes in v6:
> - Thanks to Niklas:
>   - Changed buildGtestParameters() into initGtestParameters() and removed the
>     need for Harness::gtestArgc_ and Harness::gtestArgv_
> 
> Changes in v5:
> - Thanks to Niklas:
>   - Moved buildGtestParameters() inside run()
> 
> No changes in v4
> 
>  src/lc-compliance/main.cpp | 68 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 64 insertions(+), 4 deletions(-)
> 
> diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp
> index 8c18845141de..bf9920008827 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,13 +47,17 @@ public:
>  	~Harness();
>  
>  	int init();
> -	int run(int argc, char **argv);
> +	int run(char *arg0);
>  
>  private:
> +	int initGtestParameters(char *arg0);
>  	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" } };
>  };
>  
>  Harness::Harness(const OptionsParser::Options &options)
> @@ -76,6 +82,12 @@ int Harness::init()
>  		return ret;
>  	}
>  
> +	/*
> +	 * No need to initialize the camera if we'll just list tests
> +	 */
> +	if (options_.isSet(OptList))
> +		return 0;
> +
>  	if (!options_.isSet(OptCamera)) {
>  		std::cout << "No camera specified, available cameras:" << std::endl;
>  		listCameras();
> @@ -97,9 +109,11 @@ int Harness::init()
>  	return 0;
>  }
>  
> -int Harness::run(int argc, char **argv)
> +int Harness::run(char *arg0)
>  {
> -	::testing::InitGoogleTest(&argc, argv);
> +	int ret = initGtestParameters(arg0);
> +	if (ret)
> +		return ret;
>  
>  	testing::UnitTest::GetInstance()->listeners().Append(new ThrowListener);
>  
> @@ -112,12 +126,58 @@ void Harness::listCameras()
>  		std::cout << "- " << cam.get()->id() << std::endl;
>  }
>  
> +int Harness::initGtestParameters(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.
> +	 */
> +	char **argv = new char *[(gtestFlag_.size() + 2)];
> +	std::string filterParam;
> +
> +	if (!argv)
> +		return -ENOMEM;
> +
> +	argv[argc] = arg0;
> +	argc++;
> +
> +	if (options_.isSet(OptList)) {
> +		argv[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
> +		 */
> +		filterParam = gtestFlag_.at("filter") + "=" +
> +			      static_cast<const std::string &>(options_[OptFilter]);
> +
> +		argv[argc] = const_cast<char *>(filterParam.c_str());
> +		argc++;
> +	}
> +
> +	argv[argc] = 0;
> +
> +	::testing::InitGoogleTest(&argc, argv);
> +
> +	delete argv;

I'm getting error: 'delete' applied to a pointer that was allocated with
'new[]'; did you mean 'delete[]'? [-Werror,-Wmismatched-new-delete] here
on clang.


Paul

> +
> +	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 +207,5 @@ int main(int argc, char **argv)
>  	if (ret)
>  		return ret;
>  
> -	return harness.run(argc, argv);
> +	return harness.run(argv[0]);
>  }
> -- 
> 2.32.0
>