[{"id":17407,"web_url":"https://patchwork.libcamera.org/comment/17407/","msgid":"","date":"2021-06-04T21:37:06","subject":"Re: [libcamera-devel] [PATCH v5 5/5] lc-compliance: Add list and\n\tfilter parameters","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Nícolas,\n\nThanks for your work.\n\nOn 2021-05-24 16:13:09 -0300, Nícolas F. R. A. Prado wrote:\n> Add a --list parameter that lists all current tests (by mapping to\n> googletest's --gtest_list_tests).\n> \n> Add a --filter 'filterString' parameter that filters the tests to run\n> (by mapping to googletest's --gtest_filter).\n> \n> Signed-off-by: Nícolas F. R. A. Prado \n> ---\n> Changes in v5:\n> - Thanks to Niklas:\n> - Moved buildGtestParameters() inside run()\n> \n> No changes in v4\n> \n> src/lc-compliance/main.cpp | 71 +++++++++++++++++++++++++++++++++++---\n> 1 file changed, 66 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp\n> index 27503372d0eb..182807a14c31 100644\n> --- a/src/lc-compliance/main.cpp\n> +++ b/src/lc-compliance/main.cpp\n> @@ -20,6 +20,8 @@ using namespace libcamera;\n> \n> enum {\n> \tOptCamera = 'c',\n> +\tOptList = 'l',\n> +\tOptFilter = 'f',\n> \tOptHelp = 'h',\n> };\n> \n> @@ -45,17 +47,25 @@ public:\n> \t~Harness();\n> \n> \tint init();\n> -\tint run(int argc, char **argv);\n> +\tint run(char *arg0);\n> +\tint buildGtestParameters(char *arg0);\n\nbuildGtestParameters() don't need to be public does it?\n\n> \n> private:\n> \tvoid listCameras();\n> \n> \tOptionsParser::Options options_;\n> \tstd::shared_ptr cm_;\n> +\n> +\tconst std::map gtestFlag_ = { { \"list\", \"--gtest_list_tests\" },\n> +\t\t\t\t\t\t\t\t{ \"filter\", \"--gtest_filter\" } };\n> +\n> +\tint gtestArgc_;\n> +\tchar **gtestArgv_;\n> +\tstd::string gtestFilterParam_;\n> };\n> \n> Harness::Harness(const OptionsParser::Options &options)\n> -\t: options_(options)\n> +\t: options_(options), gtestArgv_(nullptr)\n> {\n> \tcm_ = std::make_shared();\n> }\n> @@ -63,6 +73,8 @@ Harness::Harness(const OptionsParser::Options &options)\n> Harness::~Harness()\n> {\n> \tcm_->stop();\n> +\n> +\tfree(gtestArgv_);\n> }\n> \n> int Harness::init()\n> @@ -76,6 +88,9 @@ int Harness::init()\n> \t\treturn ret;\n> \t}\n> \n> +\tif (options_.isSet(OptList))\n> +\t\treturn 0;\n> +\n> \tif (!options_.isSet(OptCamera)) {\n> \t\tstd::cout << \"No camera specified, available cameras:\" << std::endl;\n> \t\tlistCameras();\n> @@ -97,9 +112,13 @@ int Harness::init()\n> \treturn 0;\n> }\n> \n> -int Harness::run(int argc, char **argv)\n> +int Harness::run(char *arg0)\n> {\n> -\t::testing::InitGoogleTest(&argc, argv);\n> +\tint ret = buildGtestParameters(arg0);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t::testing::InitGoogleTest(>estArgc_, gtestArgv_);\n\nIt looks like this could be reworked so gtestArgc_ and gtestArgv_ don't \nneed to be class members right. Could not ::testing::InitGoogleTest() be \ncalled inside buildGtestParameters() (with a possible rename)?\n\n> \n> \ttesting::UnitTest::GetInstance()->listeners().Append(new ThrowListener);\n> \n> @@ -112,12 +131,54 @@ void Harness::listCameras()\n> \t\tstd::cout << \"- \" << cam.get()->id() << std::endl;\n> }\n> \n> +int Harness::buildGtestParameters(char *arg0)\n> +{\n> +\tint argc = 0;\n> +\n> +\t/*\n> +\t * +2 to have space for both the 0th argument that is needed but not\n> +\t * used and the null at the end.\n> +\t */\n> +\tgtestArgv_ = (char **) malloc((gtestFlag_.size() + 2) * sizeof(char *));\n> +\tif (!gtestArgv_)\n> +\t\treturn -ENOMEM;\n> +\n> +\tgtestArgv_[argc] = arg0;\n> +\targc++;\n> +\n> +\tif (options_.isSet(OptList)) {\n> +\t\tgtestArgv_[argc] = const_cast(gtestFlag_.at(\"list\").c_str());\n> +\t\targc++;\n> +\t}\n> +\n> +\tif (options_.isSet(OptFilter)) {\n> +\t\t/*\n> +\t\t * The filter flag needs to be passed as a single parameter, in\n> +\t\t * the format --gtest_filter=filterStr\n> +\t\t */\n> +\t\tconst std::string &filter = options_[OptFilter];\n> +\t\tgtestFilterParam_ = gtestFlag_.at(\"filter\") + \"=\" + filter;\n> +\n> +\t\tgtestArgv_[argc] = const_cast(gtestFilterParam_.c_str());\n> +\t\targc++;\n> +\t}\n> +\n> +\tgtestArgv_[argc] = 0;\n> +\tgtestArgc_ = argc;\n> +\n> +\treturn 0;\n> +}\n> +\n> static int parseOptions(int argc, char **argv, OptionsParser::Options *options)\n> {\n> \tOptionsParser parser;\n> \tparser.addOption(OptCamera, OptionString,\n> \t\t\t \"Specify which camera to operate on, by id\", \"camera\",\n> \t\t\t ArgumentRequired, \"camera\");\n> +\tparser.addOption(OptList, OptionNone, \"List all tests and exit\", \"list\");\n> +\tparser.addOption(OptFilter, OptionString,\n> +\t\t\t \"Specify which tests to run\", \"filter\",\n> +\t\t\t ArgumentRequired, \"filter\");\n> \tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n> \t\t\t \"help\");\n> \n> @@ -147,5 +208,5 @@ int main(int argc, char **argv)\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\treturn harness.run(argc, argv);\n> +\treturn harness.run(argv[0]);\n> }\n> -- \n> 2.31.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 510B5C3208\n\tfor ;\n\tFri, 4 Jun 2021 21:37:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A9C376892B;\n\tFri, 4 Jun 2021 23:37:09 +0200 (CEST)","from mail-lj1-x231.google.com (mail-lj1-x231.google.com\n\t[IPv6:2a00:1450:4864:20::231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3834F68925\n\tfor ;\n\tFri, 4 Jun 2021 23:37:08 +0200 (CEST)","by mail-lj1-x231.google.com with SMTP id 131so13361394ljj.3\n\tfor ;\n\tFri, 04 Jun 2021 14:37:08 -0700 (PDT)","from localhost (h-62-63-236-217.A463.priv.bahnhof.se.\n\t[62.63.236.217]) by smtp.gmail.com with ESMTPSA id\n\ts1sm800080ljj.69.2021.06.04.14.37.06\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 04 Jun 2021 14:37:07 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"WinPay+1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=Wu0H5I8hH1bGYVo5/aC+IbH98VflWRpvM9+efGdLabQ=;\n\tb=WinPay+10EM7iwqWKKmtJRQB2MDBAuBkqEQO1OAU8fgr1hZh0Lx8zmGFPxeQcei1lC\n\ta1Hb05kfvLb9ROjb+8UMc50coczcsFFaklkTWxnLDCLAp/TtjXnOacrD+SN3DLb6vMG/\n\tLY9buo9Nchi0AMhSRBz0qqEI+XaaeKAOz78ofKNPrF3CAo4VN2v0mG7X/Kx81Cia1X73\n\tqUa2CFbt3eBsOgnB0hLnfxNmm0Ga+d0arG4yAYXMvlNwfs51hnTU74b20OhXu5a9EIKh\n\txYaCHnyUM7HJO0wCpERp4+AQv1KQN7q7Rg5nr6xnVkGHQKnIGLZdj5mJfRROjCuDScqz\n\tL5Qw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=Wu0H5I8hH1bGYVo5/aC+IbH98VflWRpvM9+efGdLabQ=;\n\tb=caybkgqFoRstl2XtQziTfgTXz3sju6vtTRBoWDzLVNB8ZoNQEkCWKvZxfEfmvqbf9N\n\tHZm3xgLxF7CZnjgqzq6tFYwe2GDaWBh+fMSk7wg/w9xf7nYEDns+uQdtq68PFBu/0y3b\n\tkS7Rv7ZSpQwR43k6kaDyHEOV89mKYGVH8Dm6rBxzNB1MG1s6DXCbl8nCwv3Q92vbsHTa\n\tNbKyUpAZi8cH21SMxNeS/5fW8NvKP2XB+/BY4sPweL/FLCCYw7QfIulT+1ZR95/MaTa6\n\tBIFCscVEG9M3ZnY22JFDeZ3GbYsvC+ojS6MK/cNjPrgkghPRyLJ5UoucOx9a7IBbvk63\n\tjYqw==","X-Gm-Message-State":"AOAM533puwEU0BETP3xUzmeQUk6PmDBwzgqROpYU/B/OziXcPzRIzlvs\n\tvJN7DfBKo0A1W1WXO3iyBVuS1Q==","X-Google-Smtp-Source":"ABdhPJx3NhA2/9ULLz2eAetbedbgHqah1mnDFUYVU/H7+YgBBgSZo4fQSg0KCWETVOEjONp6NqWzyw==","X-Received":"by 2002:a2e:bf14:: with SMTP id c20mr4982166ljr.57.1622842627518;\n\tFri, 04 Jun 2021 14:37:07 -0700 (PDT)","Date":"Fri, 4 Jun 2021 23:37:06 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210524191309.90238-1-nfraprado@collabora.com>\n\t<20210524191309.90238-6-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210524191309.90238-6-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v5 5/5] lc-compliance: Add list and\n\tfilter parameters","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17442,"web_url":"https://patchwork.libcamera.org/comment/17442/","msgid":"<20210607153011.qoiwddkj3gkwaixw@notapiano>","date":"2021-06-07T15:30:11","subject":"Re: [libcamera-devel] [PATCH v5 5/5] lc-compliance: Add list and\n\tfilter parameters","submitter":{"id":84,"url":"https://patchwork.libcamera.org/api/people/84/","name":"Nícolas F. R. A. Prado","email":"nfraprado@collabora.com"},"content":"Hi Niklas,\n\nOn Fri, Jun 04, 2021 at 11:37:06PM +0200, Niklas Söderlund wrote:\n> Hi Nícolas,\n> \n> Thanks for your work.\n> \n> On 2021-05-24 16:13:09 -0300, Nícolas F. R. A. Prado wrote:\n> > Add a --list parameter that lists all current tests (by mapping to\n> > googletest's --gtest_list_tests).\n> > \n> > Add a --filter 'filterString' parameter that filters the tests to run\n> > (by mapping to googletest's --gtest_filter).\n> > \n> > Signed-off-by: Nícolas F. R. A. Prado \n> > ---\n> > Changes in v5:\n> > - Thanks to Niklas:\n> > - Moved buildGtestParameters() inside run()\n> > \n> > No changes in v4\n> > \n> > src/lc-compliance/main.cpp | 71 +++++++++++++++++++++++++++++++++++---\n> > 1 file changed, 66 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp\n> > index 27503372d0eb..182807a14c31 100644\n> > --- a/src/lc-compliance/main.cpp\n> > +++ b/src/lc-compliance/main.cpp\n> > @@ -20,6 +20,8 @@ using namespace libcamera;\n> > \n> > enum {\n> > \tOptCamera = 'c',\n> > +\tOptList = 'l',\n> > +\tOptFilter = 'f',\n> > \tOptHelp = 'h',\n> > };\n> > \n> > @@ -45,17 +47,25 @@ public:\n> > \t~Harness();\n> > \n> > \tint init();\n> > -\tint run(int argc, char **argv);\n> > +\tint run(char *arg0);\n> > +\tint buildGtestParameters(char *arg0);\n> \n> buildGtestParameters() don't need to be public does it?\n\nRight, it doesn't.\n\n> \n> > \n> > private:\n> > \tvoid listCameras();\n> > \n> > \tOptionsParser::Options options_;\n> > \tstd::shared_ptr cm_;\n> > +\n> > +\tconst std::map gtestFlag_ = { { \"list\", \"--gtest_list_tests\" },\n> > +\t\t\t\t\t\t\t\t{ \"filter\", \"--gtest_filter\" } };\n> > +\n> > +\tint gtestArgc_;\n> > +\tchar **gtestArgv_;\n> > +\tstd::string gtestFilterParam_;\n> > };\n> > \n> > Harness::Harness(const OptionsParser::Options &options)\n> > -\t: options_(options)\n> > +\t: options_(options), gtestArgv_(nullptr)\n> > {\n> > \tcm_ = std::make_shared();\n> > }\n> > @@ -63,6 +73,8 @@ Harness::Harness(const OptionsParser::Options &options)\n> > Harness::~Harness()\n> > {\n> > \tcm_->stop();\n> > +\n> > +\tfree(gtestArgv_);\n> > }\n> > \n> > int Harness::init()\n> > @@ -76,6 +88,9 @@ int Harness::init()\n> > \t\treturn ret;\n> > \t}\n> > \n> > +\tif (options_.isSet(OptList))\n> > +\t\treturn 0;\n> > +\n> > \tif (!options_.isSet(OptCamera)) {\n> > \t\tstd::cout << \"No camera specified, available cameras:\" << std::endl;\n> > \t\tlistCameras();\n> > @@ -97,9 +112,13 @@ int Harness::init()\n> > \treturn 0;\n> > }\n> > \n> > -int Harness::run(int argc, char **argv)\n> > +int Harness::run(char *arg0)\n> > {\n> > -\t::testing::InitGoogleTest(&argc, argv);\n> > +\tint ret = buildGtestParameters(arg0);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\t::testing::InitGoogleTest(>estArgc_, gtestArgv_);\n> \n> It looks like this could be reworked so gtestArgc_ and gtestArgv_ don't \n> need to be class members right. Could not ::testing::InitGoogleTest() be \n> called inside buildGtestParameters() (with a possible rename)?\n\nI think we could do this to remove gtestArgc_, but we still need to keep track\nof gtestArgv_ as a class member so we can free() that pointer in ~Harness(),\notherwise we'll leak its memory, right?\n\nThanks,\nNícolas\n\n> \n> > \n> > \ttesting::UnitTest::GetInstance()->listeners().Append(new ThrowListener);\n> > \n> > @@ -112,12 +131,54 @@ void Harness::listCameras()\n> > \t\tstd::cout << \"- \" << cam.get()->id() << std::endl;\n> > }\n> > \n> > +int Harness::buildGtestParameters(char *arg0)\n> > +{\n> > +\tint argc = 0;\n> > +\n> > +\t/*\n> > +\t * +2 to have space for both the 0th argument that is needed but not\n> > +\t * used and the null at the end.\n> > +\t */\n> > +\tgtestArgv_ = (char **) malloc((gtestFlag_.size() + 2) * sizeof(char *));\n> > +\tif (!gtestArgv_)\n> > +\t\treturn -ENOMEM;\n> > +\n> > +\tgtestArgv_[argc] = arg0;\n> > +\targc++;\n> > +\n> > +\tif (options_.isSet(OptList)) {\n> > +\t\tgtestArgv_[argc] = const_cast(gtestFlag_.at(\"list\").c_str());\n> > +\t\targc++;\n> > +\t}\n> > +\n> > +\tif (options_.isSet(OptFilter)) {\n> > +\t\t/*\n> > +\t\t * The filter flag needs to be passed as a single parameter, in\n> > +\t\t * the format --gtest_filter=filterStr\n> > +\t\t */\n> > +\t\tconst std::string &filter = options_[OptFilter];\n> > +\t\tgtestFilterParam_ = gtestFlag_.at(\"filter\") + \"=\" + filter;\n> > +\n> > +\t\tgtestArgv_[argc] = const_cast(gtestFilterParam_.c_str());\n> > +\t\targc++;\n> > +\t}\n> > +\n> > +\tgtestArgv_[argc] = 0;\n> > +\tgtestArgc_ = argc;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > static int parseOptions(int argc, char **argv, OptionsParser::Options *options)\n> > {\n> > \tOptionsParser parser;\n> > \tparser.addOption(OptCamera, OptionString,\n> > \t\t\t \"Specify which camera to operate on, by id\", \"camera\",\n> > \t\t\t ArgumentRequired, \"camera\");\n> > +\tparser.addOption(OptList, OptionNone, \"List all tests and exit\", \"list\");\n> > +\tparser.addOption(OptFilter, OptionString,\n> > +\t\t\t \"Specify which tests to run\", \"filter\",\n> > +\t\t\t ArgumentRequired, \"filter\");\n> > \tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n> > \t\t\t \"help\");\n> > \n> > @@ -147,5 +208,5 @@ int main(int argc, char **argv)\n> > \tif (ret)\n> > \t\treturn ret;\n> > \n> > -\treturn harness.run(argc, argv);\n> > +\treturn harness.run(argv[0]);\n> > }\n> > -- \n> > 2.31.1\n> > \n> \n> -- \n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CF631C3206\n\tfor ;\n\tMon, 7 Jun 2021 15:30:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42E426891F;\n\tMon, 7 Jun 2021 17:30:59 +0200 (CEST)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 59ECF602A7\n\tfor ;\n\tMon, 7 Jun 2021 17:30:57 +0200 (CEST)","from notapiano (unknown\n\t[IPv6:2804:14c:1a9:2978:d3df:ab8:a31a:aacc])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\t(Authenticated sender: nfraprado)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 4042E1F41F78;\n\tMon, 7 Jun 2021 16:30:55 +0100 (BST)"],"Date":"Mon, 7 Jun 2021 12:30:11 -0300","From":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4=?= Prado ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210607153011.qoiwddkj3gkwaixw@notapiano>","References":"<20210524191309.90238-1-nfraprado@collabora.com>\n\t<20210524191309.90238-6-nfraprado@collabora.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 5/5] lc-compliance: Add list and\n\tfilter parameters","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com, =?utf-8?q?A?=\n\t=?utf-8?b?bmRyw6k=?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17444,"web_url":"https://patchwork.libcamera.org/comment/17444/","msgid":"","date":"2021-06-07T16:50:50","subject":"Re: [libcamera-devel] [PATCH v5 5/5] lc-compliance: Add list and\n\tfilter parameters","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Nícolas,\n\nOn 2021-06-07 12:30:11 -0300, Nícolas F. R. A. Prado wrote:\n> Hi Niklas,\n> \n> On Fri, Jun 04, 2021 at 11:37:06PM +0200, Niklas Söderlund wrote:\n> > Hi Nícolas,\n> > \n> > Thanks for your work.\n> > \n> > On 2021-05-24 16:13:09 -0300, Nícolas F. R. A. Prado wrote:\n> > > Add a --list parameter that lists all current tests (by mapping to\n> > > googletest's --gtest_list_tests).\n> > > \n> > > Add a --filter 'filterString' parameter that filters the tests to run\n> > > (by mapping to googletest's --gtest_filter).\n> > > \n> > > Signed-off-by: Nícolas F. R. A. Prado \n> > > ---\n> > > Changes in v5:\n> > > - Thanks to Niklas:\n> > > - Moved buildGtestParameters() inside run()\n> > > \n> > > No changes in v4\n> > > \n> > > src/lc-compliance/main.cpp | 71 +++++++++++++++++++++++++++++++++++---\n> > > 1 file changed, 66 insertions(+), 5 deletions(-)\n> > > \n> > > diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp\n> > > index 27503372d0eb..182807a14c31 100644\n> > > --- a/src/lc-compliance/main.cpp\n> > > +++ b/src/lc-compliance/main.cpp\n> > > @@ -20,6 +20,8 @@ using namespace libcamera;\n> > > \n> > > enum {\n> > > \tOptCamera = 'c',\n> > > +\tOptList = 'l',\n> > > +\tOptFilter = 'f',\n> > > \tOptHelp = 'h',\n> > > };\n> > > \n> > > @@ -45,17 +47,25 @@ public:\n> > > \t~Harness();\n> > > \n> > > \tint init();\n> > > -\tint run(int argc, char **argv);\n> > > +\tint run(char *arg0);\n> > > +\tint buildGtestParameters(char *arg0);\n> > \n> > buildGtestParameters() don't need to be public does it?\n> \n> Right, it doesn't.\n> \n> > \n> > > \n> > > private:\n> > > \tvoid listCameras();\n> > > \n> > > \tOptionsParser::Options options_;\n> > > \tstd::shared_ptr cm_;\n> > > +\n> > > +\tconst std::map gtestFlag_ = { { \"list\", \"--gtest_list_tests\" },\n> > > +\t\t\t\t\t\t\t\t{ \"filter\", \"--gtest_filter\" } };\n> > > +\n> > > +\tint gtestArgc_;\n> > > +\tchar **gtestArgv_;\n> > > +\tstd::string gtestFilterParam_;\n> > > };\n> > > \n> > > Harness::Harness(const OptionsParser::Options &options)\n> > > -\t: options_(options)\n> > > +\t: options_(options), gtestArgv_(nullptr)\n> > > {\n> > > \tcm_ = std::make_shared();\n> > > }\n> > > @@ -63,6 +73,8 @@ Harness::Harness(const OptionsParser::Options &options)\n> > > Harness::~Harness()\n> > > {\n> > > \tcm_->stop();\n> > > +\n> > > +\tfree(gtestArgv_);\n> > > }\n> > > \n> > > int Harness::init()\n> > > @@ -76,6 +88,9 @@ int Harness::init()\n> > > \t\treturn ret;\n> > > \t}\n> > > \n> > > +\tif (options_.isSet(OptList))\n> > > +\t\treturn 0;\n> > > +\n> > > \tif (!options_.isSet(OptCamera)) {\n> > > \t\tstd::cout << \"No camera specified, available cameras:\" << std::endl;\n> > > \t\tlistCameras();\n> > > @@ -97,9 +112,13 @@ int Harness::init()\n> > > \treturn 0;\n> > > }\n> > > \n> > > -int Harness::run(int argc, char **argv)\n> > > +int Harness::run(char *arg0)\n> > > {\n> > > -\t::testing::InitGoogleTest(&argc, argv);\n> > > +\tint ret = buildGtestParameters(arg0);\n> > > +\tif (ret)\n> > > +\t\treturn ret;\n> > > +\n> > > +\t::testing::InitGoogleTest(>estArgc_, gtestArgv_);\n> > \n> > It looks like this could be reworked so gtestArgc_ and gtestArgv_ don't \n> > need to be class members right. Could not ::testing::InitGoogleTest() be \n> > called inside buildGtestParameters() (with a possible rename)?\n> \n> I think we could do this to remove gtestArgc_, but we still need to keep track\n> of gtestArgv_ as a class member so we can free() that pointer in ~Harness(),\n> otherwise we'll leak its memory, right?\n\nCould we not free it right after calling ::testing::InitGoogleTest() \ninstead of the destructor?\n\n> \n> Thanks,\n> Nícolas\n> \n> > \n> > > \n> > > \ttesting::UnitTest::GetInstance()->listeners().Append(new ThrowListener);\n> > > \n> > > @@ -112,12 +131,54 @@ void Harness::listCameras()\n> > > \t\tstd::cout << \"- \" << cam.get()->id() << std::endl;\n> > > }\n> > > \n> > > +int Harness::buildGtestParameters(char *arg0)\n> > > +{\n> > > +\tint argc = 0;\n> > > +\n> > > +\t/*\n> > > +\t * +2 to have space for both the 0th argument that is needed but not\n> > > +\t * used and the null at the end.\n> > > +\t */\n> > > +\tgtestArgv_ = (char **) malloc((gtestFlag_.size() + 2) * sizeof(char *));\n> > > +\tif (!gtestArgv_)\n> > > +\t\treturn -ENOMEM;\n> > > +\n> > > +\tgtestArgv_[argc] = arg0;\n> > > +\targc++;\n> > > +\n> > > +\tif (options_.isSet(OptList)) {\n> > > +\t\tgtestArgv_[argc] = const_cast(gtestFlag_.at(\"list\").c_str());\n> > > +\t\targc++;\n> > > +\t}\n> > > +\n> > > +\tif (options_.isSet(OptFilter)) {\n> > > +\t\t/*\n> > > +\t\t * The filter flag needs to be passed as a single parameter, in\n> > > +\t\t * the format --gtest_filter=filterStr\n> > > +\t\t */\n> > > +\t\tconst std::string &filter = options_[OptFilter];\n> > > +\t\tgtestFilterParam_ = gtestFlag_.at(\"filter\") + \"=\" + filter;\n> > > +\n> > > +\t\tgtestArgv_[argc] = const_cast(gtestFilterParam_.c_str());\n> > > +\t\targc++;\n> > > +\t}\n> > > +\n> > > +\tgtestArgv_[argc] = 0;\n> > > +\tgtestArgc_ = argc;\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > static int parseOptions(int argc, char **argv, OptionsParser::Options *options)\n> > > {\n> > > \tOptionsParser parser;\n> > > \tparser.addOption(OptCamera, OptionString,\n> > > \t\t\t \"Specify which camera to operate on, by id\", \"camera\",\n> > > \t\t\t ArgumentRequired, \"camera\");\n> > > +\tparser.addOption(OptList, OptionNone, \"List all tests and exit\", \"list\");\n> > > +\tparser.addOption(OptFilter, OptionString,\n> > > +\t\t\t \"Specify which tests to run\", \"filter\",\n> > > +\t\t\t ArgumentRequired, \"filter\");\n> > > \tparser.addOption(OptHelp, OptionNone, \"Display this help message\",\n> > > \t\t\t \"help\");\n> > > \n> > > @@ -147,5 +208,5 @@ int main(int argc, char **argv)\n> > > \tif (ret)\n> > > \t\treturn ret;\n> > > \n> > > -\treturn harness.run(argc, argv);\n> > > +\treturn harness.run(argv[0]);\n> > > }\n> > > -- \n> > > 2.31.1\n> > > \n> > \n> > -- \n> > Regards,\n> > Niklas Söderlund","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 542AEC3206\n\tfor ;\n\tMon, 7 Jun 2021 16:50:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84F6C68926;\n\tMon, 7 Jun 2021 18:50:53 +0200 (CEST)","from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com\n\t[IPv6:2a00:1450:4864:20::22a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 430D0602A7\n\tfor ;\n\tMon, 7 Jun 2021 18:50:52 +0200 (CEST)","by mail-lj1-x22a.google.com with SMTP id s22so2731731ljg.5\n\tfor ;\n\tMon, 07 Jun 2021 09:50:52 -0700 (PDT)","from localhost (h-62-63-236-217.A463.priv.bahnhof.se.\n\t[62.63.236.217]) by smtp.gmail.com with ESMTPSA id\n\ty26sm1549343lfj.298.2021.06.07.09.50.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 07 Jun 2021 09:50:50 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"AKcdNYxQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=ZOFMjmodWIgxwKvru3L864sS612LZSaJ+K6jYkUm0y8=;\n\tb=AKcdNYxQJpqwHc1n0YyG4hXtGUSycWF/xu0hGauI6Xn5PdmLM5qp1XPSMQGS9SPRS6\n\tpdRfleeRBqbddHwBdUARMwmDLNKOtXvq6QekvZqcDHi+PCLXjUbN8w4dwWNFps5NfMOj\n\tZJ17J+QBbIpGAz7yrhzdAadU3SyPlNK1UUhmRChjM2JZfecd2QcbWkGE2TX5bwiFtIEq\n\tl79XJXhC7HxkTr8nPHGwA374rhCU23jqGxxs5rQ1iNeldoPC4lixTSg77PDPkQroJjQb\n\thN6EeVXymMamGDyG42OwSwNmXBbH4hGd/2JLnsbHEToas7c8HQR/ZXLSUcFZ0eHzWhg5\n\tLOHA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=ZOFMjmodWIgxwKvru3L864sS612LZSaJ+K6jYkUm0y8=;\n\tb=JzybN/lnRSOjV8x4BzoW1ZQpxtz4L0QPWwmlE3iOFOKwCB5/RR9e7clPMSzXsFEKHV\n\t9iCTG/tkbLDHkbUpX7Ke+5Bf3aFZXI8OmMsdeuzLi6lMmqjP+5QYh4hp6APkGUtgiZ0u\n\tg7c1r4DhRPbNy9w+6lQVyHkB0WS0OLAslcaBSJXS8YRSRvkzSpp/lZO7gTBggpGcvkct\n\tjt6NN0CQxK1TUSHQilDWO6MME4BcTJlYfY/vGLCgQ3D7YLmdFMd9e3C+rl+b+CP+Mvu7\n\tB3V4PXWQKo8q9yRtugmS/PF/oI65IReGRCIKhZxwmysCJNqK97r7gRW9pJxHDLLmdqaD\n\t+PEA==","X-Gm-Message-State":"AOAM5332TITdLmOqcGsPJu1lIcxQid2PfBqQ+fVK8oBHHoF64xehhr2R\n\tDQI0+8Sybwp/St2QRZqzu2nSOA==","X-Google-Smtp-Source":"ABdhPJyGXc5p4WlHnQMbjZiAMvCesmWqyrM2taDAsvQsQV56PXZUcE7snFliI2A+Y5HS6z1z7piTTA==","X-Received":"by 2002:a2e:9c85:: with SMTP id x5mr8899152lji.78.1623084651444; \n\tMon, 07 Jun 2021 09:50:51 -0700 (PDT)","Date":"Mon, 7 Jun 2021 18:50:50 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210524191309.90238-1-nfraprado@collabora.com>\n\t<20210524191309.90238-6-nfraprado@collabora.com>\n\t\n\t<20210607153011.qoiwddkj3gkwaixw@notapiano>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210607153011.qoiwddkj3gkwaixw@notapiano>","Subject":"Re: [libcamera-devel] [PATCH v5 5/5] lc-compliance: Add list and\n\tfilter parameters","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17445,"web_url":"https://patchwork.libcamera.org/comment/17445/","msgid":"<20210607170535.dyz6fjdunvtisl5o@notapiano>","date":"2021-06-07T17:05:35","subject":"Re: [libcamera-devel] [PATCH v5 5/5] lc-compliance: Add list and\n\tfilter parameters","submitter":{"id":84,"url":"https://patchwork.libcamera.org/api/people/84/","name":"Nícolas F. R. A. Prado","email":"nfraprado@collabora.com"},"content":"Hi Niklas,\n\nOn Mon, Jun 07, 2021 at 06:50:50PM +0200, Niklas Söderlund wrote:\n> Hi Nícolas,\n> \n> On 2021-06-07 12:30:11 -0300, Nícolas F. R. A. Prado wrote:\n> > Hi Niklas,\n> > \n> > On Fri, Jun 04, 2021 at 11:37:06PM +0200, Niklas Söderlund wrote:\n> > > Hi Nícolas,\n> > > \n> > > Thanks for your work.\n> > > \n> > > On 2021-05-24 16:13:09 -0300, Nícolas F. R. A. Prado wrote:\n> > > > Add a --list parameter that lists all current tests (by mapping to\n> > > > googletest's --gtest_list_tests).\n> > > > \n> > > > Add a --filter 'filterString' parameter that filters the tests to run\n> > > > (by mapping to googletest's --gtest_filter).\n> > > > \n> > > > Signed-off-by: Nícolas F. R. A. Prado \n> > > > ---\n> > > > Changes in v5:\n> > > > - Thanks to Niklas:\n> > > > - Moved buildGtestParameters() inside run()\n> > > > \n> > > > No changes in v4\n> > > > \n> > > > src/lc-compliance/main.cpp | 71 +++++++++++++++++++++++++++++++++++---\n> > > > 1 file changed, 66 insertions(+), 5 deletions(-)\n> > > > \n> > > > diff --git a/src/lc-compliance/main.cpp b/src/lc-compliance/main.cpp\n> > > > index 27503372d0eb..182807a14c31 100644\n> > > > --- a/src/lc-compliance/main.cpp\n> > > > +++ b/src/lc-compliance/main.cpp\n> > > > @@ -20,6 +20,8 @@ using namespace libcamera;\n> > > > \n> > > > enum {\n> > > > \tOptCamera = 'c',\n> > > > +\tOptList = 'l',\n> > > > +\tOptFilter = 'f',\n> > > > \tOptHelp = 'h',\n> > > > };\n> > > > \n> > > > @@ -45,17 +47,25 @@ public:\n> > > > \t~Harness();\n> > > > \n> > > > \tint init();\n> > > > -\tint run(int argc, char **argv);\n> > > > +\tint run(char *arg0);\n> > > > +\tint buildGtestParameters(char *arg0);\n> > > \n> > > buildGtestParameters() don't need to be public does it?\n> > \n> > Right, it doesn't.\n> > \n> > > \n> > > > \n> > > > private:\n> > > > \tvoid listCameras();\n> > > > \n> > > > \tOptionsParser::Options options_;\n> > > > \tstd::shared_ptr cm_;\n> > > > +\n> > > > +\tconst std::map gtestFlag_ = { { \"list\", \"--gtest_list_tests\" },\n> > > > +\t\t\t\t\t\t\t\t{ \"filter\", \"--gtest_filter\" } };\n> > > > +\n> > > > +\tint gtestArgc_;\n> > > > +\tchar **gtestArgv_;\n> > > > +\tstd::string gtestFilterParam_;\n> > > > };\n> > > > \n> > > > Harness::Harness(const OptionsParser::Options &options)\n> > > > -\t: options_(options)\n> > > > +\t: options_(options), gtestArgv_(nullptr)\n> > > > {\n> > > > \tcm_ = std::make_shared();\n> > > > }\n> > > > @@ -63,6 +73,8 @@ Harness::Harness(const OptionsParser::Options &options)\n> > > > Harness::~Harness()\n> > > > {\n> > > > \tcm_->stop();\n> > > > +\n> > > > +\tfree(gtestArgv_);\n> > > > }\n> > > > \n> > > > int Harness::init()\n> > > > @@ -76,6 +88,9 @@ int Harness::init()\n> > > > \t\treturn ret;\n> > > > \t}\n> > > > \n> > > > +\tif (options_.isSet(OptList))\n> > > > +\t\treturn 0;\n> > > > +\n> > > > \tif (!options_.isSet(OptCamera)) {\n> > > > \t\tstd::cout << \"No camera specified, available cameras:\" << std::endl;\n> > > > \t\tlistCameras();\n> > > > @@ -97,9 +112,13 @@ int Harness::init()\n> > > > \treturn 0;\n> > > > }\n> > > > \n> > > > -int Harness::run(int argc, char **argv)\n> > > > +int Harness::run(char *arg0)\n> > > > {\n> > > > -\t::testing::InitGoogleTest(&argc, argv);\n> > > > +\tint ret = buildGtestParameters(arg0);\n> > > > +\tif (ret)\n> > > > +\t\treturn ret;\n> > > > +\n> > > > +\t::testing::InitGoogleTest(>estArgc_, gtestArgv_);\n> > > \n> > > It looks like this could be reworked so gtestArgc_ and gtestArgv_ don't \n> > > need to be class members right. Could not ::testing::InitGoogleTest() be \n> > > called inside buildGtestParameters() (with a possible rename)?\n> > \n> > I think we could do this to remove gtestArgc_, but we still need to keep track\n> > of gtestArgv_ as a class member so we can free() that pointer in ~Harness(),\n> > otherwise we'll leak its memory, right?\n> \n> Could we not free it right after calling ::testing::InitGoogleTest() \n> instead of the destructor?\n\nJust tested it and indeed we can. I was under the impression Gtest relied on\nthat data after the call, but I guess it must copy it internally before\nreturning then.\n\nI'll change it for v6 as well then.\n\nThanks,\nNícolas","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2532DC320B\n\tfor ;\n\tMon, 7 Jun 2021 17:06:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8642868926;\n\tMon, 7 Jun 2021 19:06:21 +0200 (CEST)","from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 75314602A7\n\tfor ;\n\tMon, 7 Jun 2021 19:06:20 +0200 (CEST)","from notapiano (unknown\n\t[IPv6:2804:14c:1a9:2978:d3df:ab8:a31a:aacc])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits)) (No client certificate requested)\n\t(Authenticated sender: nfraprado)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 672051F4213C;\n\tMon, 7 Jun 2021 18:06:18 +0100 (BST)"],"Date":"Mon, 7 Jun 2021 14:05:35 -0300","From":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4=?= Prado ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210607170535.dyz6fjdunvtisl5o@notapiano>","References":"<20210524191309.90238-1-nfraprado@collabora.com>\n\t<20210524191309.90238-6-nfraprado@collabora.com>\n\t\n\t<20210607153011.qoiwddkj3gkwaixw@notapiano>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 5/5] lc-compliance: Add list and\n\tfilter parameters","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com, =?utf-8?q?A?=\n\t=?utf-8?b?bmRyw6k=?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]