[{"id":21426,"web_url":"https://patchwork.libcamera.org/comment/21426/","msgid":"<20211130082414.kvsknhw3hbi32gpx@uno.localdomain>","date":"2021-11-30T08:24:14","subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Tue, Nov 30, 2021 at 03:23:02AM +0200, Laurent Pinchart wrote:\n> Store the path to the test executable, found in argv[0], in the Test\n> instance. This can be useful for tests that need to fork processes.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> test/ipc/unixsocket.cpp | 4 +++-\n> test/ipc/unixsocket_ipc.cpp | 4 +++-\n> test/libtest/test.cpp | 5 +++++\n> test/libtest/test.h | 15 ++++++++++++---\n> test/log/log_process.cpp | 4 +++-\n> test/process/process_test.cpp | 5 +++--\n> 6 files changed, 29 insertions(+), 8 deletions(-)\n>\n> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> index 7270bf4d2fe7..4fc1c10a2125 100644\n> --- a/test/ipc/unixsocket.cpp\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -501,5 +501,7 @@ int main(int argc, char **argv)\n> \t\treturn slave.run(ipcfd);\n> \t}\n>\n> -\treturn UnixSocketTest().execute();\n> +\tUnixSocketTest test;\n> +\ttest.setArgs(argc, argv);\n> +\treturn test.execute();\n> }\n> diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp\n> index ab5d25572d83..2e3b52ca4d4b 100644\n> --- a/test/ipc/unixsocket_ipc.cpp\n> +++ b/test/ipc/unixsocket_ipc.cpp\n> @@ -227,5 +227,7 @@ int main(int argc, char **argv)\n> \t\treturn slave.run(ipcfd);\n> \t}\n>\n> -\treturn UnixSocketTestIPC().execute();\n> +\tUnixSocketTestIPC test;\n> +\ttest.setArgs(argc, argv);\n> +\treturn test.execute();\n> }\n> diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp\n> index fd9f3d74d6ce..af37b4dd28ff 100644\n> --- a/test/libtest/test.cpp\n> +++ b/test/libtest/test.cpp\n> @@ -17,6 +17,11 @@ Test::~Test()\n> {\n> }\n>\n> +void Test::setArgs([[maybe_unused]] int argc, char *argv[])\n> +{\n> +\tself_ = argv[0];\n\nParanoia says this could be made intentionally very very long\n\nApart from that, for the series\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +}\n> +\n> int Test::execute()\n> {\n> \tint ret;\n> diff --git a/test/libtest/test.h b/test/libtest/test.h\n> index ee01a22591f8..23b07743fd2a 100644\n> --- a/test/libtest/test.h\n> +++ b/test/libtest/test.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n>\n> #include \n> +#include \n>\n> enum TestStatus {\n> \tTestPass = 0,\n> @@ -21,16 +22,24 @@ public:\n> \tTest();\n> \tvirtual ~Test();\n>\n> +\tvoid setArgs(int argc, char *argv[]);\n> \tint execute();\n>\n> +\tconst std::string &self() const { return self_; }\n> +\n> protected:\n> \tvirtual int init() { return 0; }\n> \tvirtual int run() = 0;\n> \tvirtual void cleanup() {}\n> +\n> +private:\n> +\tstd::string self_;\n> };\n>\n> -#define TEST_REGISTER(klass)\t\t\t\t\t\t\\\n> -int main([[maybe_unused]] int argc, [[maybe_unused]] char *argv[])\t\\\n> +#define TEST_REGISTER(Klass)\t\t\t\t\t\t\\\n> +int main(int argc, char *argv[])\t\t\t\t\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> -\treturn klass().execute();\t\t\t\t\t\\\n> +\tKlass klass;\t\t\t\t\t\t\t\\\n> +\tklass.setArgs(argc, argv);\t\t\t\t\t\\\n> +\treturn klass.execute();\t\t\t\t\t\t\\\n> }\n> diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n> index a56a399848a7..ca8351335f3a 100644\n> --- a/test/log/log_process.cpp\n> +++ b/test/log/log_process.cpp\n> @@ -154,5 +154,7 @@ int main(int argc, char **argv)\n> \t\treturn child.run(status, num);\n> \t}\n>\n> -\treturn LogProcessTest().execute();\n> +\tLogProcessTest test;\n> +\ttest.setArgs(argc, argv);\n> +\treturn test.execute();\n> }\n> diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp\n> index 378d680bf4ef..96bea17f8dce 100644\n> --- a/test/process/process_test.cpp\n> +++ b/test/process/process_test.cpp\n> @@ -9,7 +9,6 @@\n> #include \n> #include \n>\n> -\n> #include \n> #include \n> #include \n> @@ -106,5 +105,7 @@ int main(int argc, char **argv)\n> \t\treturn child.run(status);\n> \t}\n>\n> -\treturn ProcessTest().execute();\n> +\tProcessTest test;\n> +\ttest.setArgs(argc, argv);\n> +\treturn test.execute();\n> }\n> --\n> Regards,\n>\n> Laurent Pinchart\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 C4EE2BF415\n\tfor ;\n\tTue, 30 Nov 2021 08:23:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DA3FE605B4;\n\tTue, 30 Nov 2021 09:23:23 +0100 (CET)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5220160230\n\tfor ;\n\tTue, 30 Nov 2021 09:23:22 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id C38ED10000E;\n\tTue, 30 Nov 2021 08:23:21 +0000 (UTC)"],"Date":"Tue, 30 Nov 2021 09:24:14 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20211130082414.kvsknhw3hbi32gpx@uno.localdomain>","References":"<20211130012303.10574-1-laurent.pinchart@ideasonboard.com>\n\t<20211130012303.10574-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211130012303.10574-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21427,"web_url":"https://patchwork.libcamera.org/comment/21427/","msgid":"","date":"2021-11-30T09:21:06","subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Nov 30, 2021 at 09:24:14AM +0100, Jacopo Mondi wrote:\n> On Tue, Nov 30, 2021 at 03:23:02AM +0200, Laurent Pinchart wrote:\n> > Store the path to the test executable, found in argv[0], in the Test\n> > instance. This can be useful for tests that need to fork processes.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > test/ipc/unixsocket.cpp | 4 +++-\n> > test/ipc/unixsocket_ipc.cpp | 4 +++-\n> > test/libtest/test.cpp | 5 +++++\n> > test/libtest/test.h | 15 ++++++++++++---\n> > test/log/log_process.cpp | 4 +++-\n> > test/process/process_test.cpp | 5 +++--\n> > 6 files changed, 29 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> > index 7270bf4d2fe7..4fc1c10a2125 100644\n> > --- a/test/ipc/unixsocket.cpp\n> > +++ b/test/ipc/unixsocket.cpp\n> > @@ -501,5 +501,7 @@ int main(int argc, char **argv)\n> > \t\treturn slave.run(ipcfd);\n> > \t}\n> >\n> > -\treturn UnixSocketTest().execute();\n> > +\tUnixSocketTest test;\n> > +\ttest.setArgs(argc, argv);\n> > +\treturn test.execute();\n> > }\n> > diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp\n> > index ab5d25572d83..2e3b52ca4d4b 100644\n> > --- a/test/ipc/unixsocket_ipc.cpp\n> > +++ b/test/ipc/unixsocket_ipc.cpp\n> > @@ -227,5 +227,7 @@ int main(int argc, char **argv)\n> > \t\treturn slave.run(ipcfd);\n> > \t}\n> >\n> > -\treturn UnixSocketTestIPC().execute();\n> > +\tUnixSocketTestIPC test;\n> > +\ttest.setArgs(argc, argv);\n> > +\treturn test.execute();\n> > }\n> > diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp\n> > index fd9f3d74d6ce..af37b4dd28ff 100644\n> > --- a/test/libtest/test.cpp\n> > +++ b/test/libtest/test.cpp\n> > @@ -17,6 +17,11 @@ Test::~Test()\n> > {\n> > }\n> >\n> > +void Test::setArgs([[maybe_unused]] int argc, char *argv[])\n> > +{\n> > +\tself_ = argv[0];\n> \n> Paranoia says this could be made intentionally very very long\n\nLimited to PATH_MAX probably (which should be 4096 bytes at most on most\nsystems I think). And if someone wants to attack unit tests on their own\nsystem, well, I won't stop them :-)\n\n> Apart from that, for the series\n> \n> Reviewed-by: Jacopo Mondi \n> \n> > +}\n> > +\n> > int Test::execute()\n> > {\n> > \tint ret;\n> > diff --git a/test/libtest/test.h b/test/libtest/test.h\n> > index ee01a22591f8..23b07743fd2a 100644\n> > --- a/test/libtest/test.h\n> > +++ b/test/libtest/test.h\n> > @@ -8,6 +8,7 @@\n> > #pragma once\n> >\n> > #include \n> > +#include \n> >\n> > enum TestStatus {\n> > \tTestPass = 0,\n> > @@ -21,16 +22,24 @@ public:\n> > \tTest();\n> > \tvirtual ~Test();\n> >\n> > +\tvoid setArgs(int argc, char *argv[]);\n> > \tint execute();\n> >\n> > +\tconst std::string &self() const { return self_; }\n> > +\n> > protected:\n> > \tvirtual int init() { return 0; }\n> > \tvirtual int run() = 0;\n> > \tvirtual void cleanup() {}\n> > +\n> > +private:\n> > +\tstd::string self_;\n> > };\n> >\n> > -#define TEST_REGISTER(klass)\t\t\t\t\t\t\\\n> > -int main([[maybe_unused]] int argc, [[maybe_unused]] char *argv[])\t\\\n> > +#define TEST_REGISTER(Klass)\t\t\t\t\t\t\\\n> > +int main(int argc, char *argv[])\t\t\t\t\t\\\n> > {\t\t\t\t\t\t\t\t\t\\\n> > -\treturn klass().execute();\t\t\t\t\t\\\n> > +\tKlass klass;\t\t\t\t\t\t\t\\\n> > +\tklass.setArgs(argc, argv);\t\t\t\t\t\\\n> > +\treturn klass.execute();\t\t\t\t\t\t\\\n> > }\n> > diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n> > index a56a399848a7..ca8351335f3a 100644\n> > --- a/test/log/log_process.cpp\n> > +++ b/test/log/log_process.cpp\n> > @@ -154,5 +154,7 @@ int main(int argc, char **argv)\n> > \t\treturn child.run(status, num);\n> > \t}\n> >\n> > -\treturn LogProcessTest().execute();\n> > +\tLogProcessTest test;\n> > +\ttest.setArgs(argc, argv);\n> > +\treturn test.execute();\n> > }\n> > diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp\n> > index 378d680bf4ef..96bea17f8dce 100644\n> > --- a/test/process/process_test.cpp\n> > +++ b/test/process/process_test.cpp\n> > @@ -9,7 +9,6 @@\n> > #include \n> > #include \n> >\n> > -\n> > #include \n> > #include \n> > #include \n> > @@ -106,5 +105,7 @@ int main(int argc, char **argv)\n> > \t\treturn child.run(status);\n> > \t}\n> >\n> > -\treturn ProcessTest().execute();\n> > +\tProcessTest test;\n> > +\ttest.setArgs(argc, argv);\n> > +\treturn test.execute();\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 75687BF415\n\tfor ;\n\tTue, 30 Nov 2021 09:21:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA777605B4;\n\tTue, 30 Nov 2021 10:21:33 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A730E60230\n\tfor ;\n\tTue, 30 Nov 2021 10:21:31 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1C66E8F0;\n\tTue, 30 Nov 2021 10:21:31 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hUB9UJ2s\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638264091;\n\tbh=KM93/Vu5X4N6IN9Uf+6JnyWMjcvtsOSyIIfkqv/qhx4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hUB9UJ2s0BXhUSpIav3Jyg52XnRUcjwIazPV1SZAG5ZuBci9VvihfNMTujKgemivY\n\tlxdD+5Zp61ElQDqfjQlhdDHEg9v+ePKo22IAaQddRda68MnBAj6IGTL8qY6goolf8A\n\t35nmGgpoSqHAUj9+tONrESqC3SQfacvajP5EpOnw=","Date":"Tue, 30 Nov 2021 11:21:06 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20211130012303.10574-1-laurent.pinchart@ideasonboard.com>\n\t<20211130012303.10574-2-laurent.pinchart@ideasonboard.com>\n\t<20211130082414.kvsknhw3hbi32gpx@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211130082414.kvsknhw3hbi32gpx@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21443,"web_url":"https://patchwork.libcamera.org/comment/21443/","msgid":"<163826974217.3059017.15103616780548132909@Monstersaurus>","date":"2021-11-30T10:55:42","subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2021-11-30 01:23:02)\n> Store the path to the test executable, found in argv[0], in the Test\n> instance. This can be useful for tests that need to fork processes.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> test/ipc/unixsocket.cpp | 4 +++-\n> test/ipc/unixsocket_ipc.cpp | 4 +++-\n> test/libtest/test.cpp | 5 +++++\n> test/libtest/test.h | 15 ++++++++++++---\n> test/log/log_process.cpp | 4 +++-\n> test/process/process_test.cpp | 5 +++--\n> 6 files changed, 29 insertions(+), 8 deletions(-)\n> \n> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> index 7270bf4d2fe7..4fc1c10a2125 100644\n> --- a/test/ipc/unixsocket.cpp\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -501,5 +501,7 @@ int main(int argc, char **argv)\n> return slave.run(ipcfd);\n> }\n> \n> - return UnixSocketTest().execute();\n> + UnixSocketTest test;\n> + test.setArgs(argc, argv);\n> + return test.execute();\n> }\n> diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp\n> index ab5d25572d83..2e3b52ca4d4b 100644\n> --- a/test/ipc/unixsocket_ipc.cpp\n> +++ b/test/ipc/unixsocket_ipc.cpp\n> @@ -227,5 +227,7 @@ int main(int argc, char **argv)\n> return slave.run(ipcfd);\n> }\n> \n> - return UnixSocketTestIPC().execute();\n> + UnixSocketTestIPC test;\n> + test.setArgs(argc, argv);\n> + return test.execute();\n> }\n> diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp\n> index fd9f3d74d6ce..af37b4dd28ff 100644\n> --- a/test/libtest/test.cpp\n> +++ b/test/libtest/test.cpp\n> @@ -17,6 +17,11 @@ Test::~Test()\n> {\n> }\n> \n> +void Test::setArgs([[maybe_unused]] int argc, char *argv[])\n> +{\n> + self_ = argv[0];\n> +}\n> +\n> int Test::execute()\n> {\n> int ret;\n> diff --git a/test/libtest/test.h b/test/libtest/test.h\n> index ee01a22591f8..23b07743fd2a 100644\n> --- a/test/libtest/test.h\n> +++ b/test/libtest/test.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> \n> enum TestStatus {\n> TestPass = 0,\n> @@ -21,16 +22,24 @@ public:\n> Test();\n> virtual ~Test();\n> \n> + void setArgs(int argc, char *argv[]);\n\nHrm... I would have been tempted to pass these into the constructor.\n\nThey are now somewhat arbitrarily 'required' in some tests, so it may as\nwell be that they are always required. (or at least requierd to be\nconsidered).\n\n> int execute();\n> \n> + const std::string &self() const { return self_; }\n> +\n> protected:\n> virtual int init() { return 0; }\n> virtual int run() = 0;\n> virtual void cleanup() {}\n> +\n> +private:\n> + std::string self_;\n> };\n> \n> -#define TEST_REGISTER(klass) \\\n> -int main([[maybe_unused]] int argc, [[maybe_unused]] char *argv[]) \\\n> +#define TEST_REGISTER(Klass) \\\n> +int main(int argc, char *argv[]) \\\n> { \\\n> - return klass().execute(); \\\n> + Klass klass; \\\n> + klass.setArgs(argc, argv); \\\n> + return klass.execute(); \\\n> }\n> diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n> index a56a399848a7..ca8351335f3a 100644\n> --- a/test/log/log_process.cpp\n> +++ b/test/log/log_process.cpp\n> @@ -154,5 +154,7 @@ int main(int argc, char **argv)\n> return child.run(status, num);\n> }\n> \n> - return LogProcessTest().execute();\n> + LogProcessTest test;\n> + test.setArgs(argc, argv);\n\nEspecially as here we see that now we have to manually 'know' that we\nhave to make a call to setArgs() after every construction...?\n\nIt's local test code, so it's not particularly critical though.\n\nSo if you want to keep it this way so-be-it.\n\n\nReviewed-by: Kieran Bingham \n\n\n> + return test.execute();\n> }\n> diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp\n> index 378d680bf4ef..96bea17f8dce 100644\n> --- a/test/process/process_test.cpp\n> +++ b/test/process/process_test.cpp\n> @@ -9,7 +9,6 @@\n> #include \n> #include \n> \n> -\n> #include \n> #include \n> #include \n> @@ -106,5 +105,7 @@ int main(int argc, char **argv)\n> return child.run(status);\n> }\n> \n> - return ProcessTest().execute();\n> + ProcessTest test;\n> + test.setArgs(argc, argv);\n> + return test.execute();\n> }\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 295FDBF415\n\tfor ;\n\tTue, 30 Nov 2021 10:55:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 792FF605B4;\n\tTue, 30 Nov 2021 11:55:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 55F6560230\n\tfor ;\n\tTue, 30 Nov 2021 11:55:45 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E11E32FD;\n\tTue, 30 Nov 2021 11:55:44 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"IJb5XD3R\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638269745;\n\tbh=fmMFlt/0AvV66HgauykMua1R43JWBphtCiPr0jVb/jI=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=IJb5XD3RDEs6RnjPq64ITyFgtnJKjbP59bQ6EekKQwgUVXk4gtEq/DeV1snI0OzuR\n\tRvhYrPPND7cG4A/YuJYdSq+SasUnZj3Vch9bENiy0xhnarX6LjIaIARWHlcq55a0N7\n\t1bSbde9Vf87zUoGSeVSWbaJ5Kvjf0UzzjPTl4oR8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211130012303.10574-2-laurent.pinchart@ideasonboard.com>","References":"<20211130012303.10574-1-laurent.pinchart@ideasonboard.com>\n\t<20211130012303.10574-2-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 30 Nov 2021 10:55:42 +0000","Message-ID":"<163826974217.3059017.15103616780548132909@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21445,"web_url":"https://patchwork.libcamera.org/comment/21445/","msgid":"","date":"2021-11-30T11:02:33","subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Nov 30, 2021 at 10:55:42AM +0000, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2021-11-30 01:23:02)\n> > Store the path to the test executable, found in argv[0], in the Test\n> > instance. This can be useful for tests that need to fork processes.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > test/ipc/unixsocket.cpp | 4 +++-\n> > test/ipc/unixsocket_ipc.cpp | 4 +++-\n> > test/libtest/test.cpp | 5 +++++\n> > test/libtest/test.h | 15 ++++++++++++---\n> > test/log/log_process.cpp | 4 +++-\n> > test/process/process_test.cpp | 5 +++--\n> > 6 files changed, 29 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> > index 7270bf4d2fe7..4fc1c10a2125 100644\n> > --- a/test/ipc/unixsocket.cpp\n> > +++ b/test/ipc/unixsocket.cpp\n> > @@ -501,5 +501,7 @@ int main(int argc, char **argv)\n> > return slave.run(ipcfd);\n> > }\n> > \n> > - return UnixSocketTest().execute();\n> > + UnixSocketTest test;\n> > + test.setArgs(argc, argv);\n> > + return test.execute();\n> > }\n> > diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp\n> > index ab5d25572d83..2e3b52ca4d4b 100644\n> > --- a/test/ipc/unixsocket_ipc.cpp\n> > +++ b/test/ipc/unixsocket_ipc.cpp\n> > @@ -227,5 +227,7 @@ int main(int argc, char **argv)\n> > return slave.run(ipcfd);\n> > }\n> > \n> > - return UnixSocketTestIPC().execute();\n> > + UnixSocketTestIPC test;\n> > + test.setArgs(argc, argv);\n> > + return test.execute();\n> > }\n> > diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp\n> > index fd9f3d74d6ce..af37b4dd28ff 100644\n> > --- a/test/libtest/test.cpp\n> > +++ b/test/libtest/test.cpp\n> > @@ -17,6 +17,11 @@ Test::~Test()\n> > {\n> > }\n> > \n> > +void Test::setArgs([[maybe_unused]] int argc, char *argv[])\n> > +{\n> > + self_ = argv[0];\n> > +}\n> > +\n> > int Test::execute()\n> > {\n> > int ret;\n> > diff --git a/test/libtest/test.h b/test/libtest/test.h\n> > index ee01a22591f8..23b07743fd2a 100644\n> > --- a/test/libtest/test.h\n> > +++ b/test/libtest/test.h\n> > @@ -8,6 +8,7 @@\n> > #pragma once\n> > \n> > #include \n> > +#include \n> > \n> > enum TestStatus {\n> > TestPass = 0,\n> > @@ -21,16 +22,24 @@ public:\n> > Test();\n> > virtual ~Test();\n> > \n> > + void setArgs(int argc, char *argv[]);\n> \n> Hrm... I would have been tempted to pass these into the constructor.\n> \n> They are now somewhat arbitrarily 'required' in some tests, so it may as\n> well be that they are always required. (or at least requierd to be\n> considered).\n\nThat's what I initially did, but then every single test will have to\ndefine a constructor just to forward the arguments to the base Test\nclass. I thought I'd get some push back :-) If you think that's better,\nI'll go that way.\n\n> > int execute();\n> > \n> > + const std::string &self() const { return self_; }\n> > +\n> > protected:\n> > virtual int init() { return 0; }\n> > virtual int run() = 0;\n> > virtual void cleanup() {}\n> > +\n> > +private:\n> > + std::string self_;\n> > };\n> > \n> > -#define TEST_REGISTER(klass) \\\n> > -int main([[maybe_unused]] int argc, [[maybe_unused]] char *argv[]) \\\n> > +#define TEST_REGISTER(Klass) \\\n> > +int main(int argc, char *argv[]) \\\n> > { \\\n> > - return klass().execute(); \\\n> > + Klass klass; \\\n> > + klass.setArgs(argc, argv); \\\n> > + return klass.execute(); \\\n> > }\n> > diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n> > index a56a399848a7..ca8351335f3a 100644\n> > --- a/test/log/log_process.cpp\n> > +++ b/test/log/log_process.cpp\n> > @@ -154,5 +154,7 @@ int main(int argc, char **argv)\n> > return child.run(status, num);\n> > }\n> > \n> > - return LogProcessTest().execute();\n> > + LogProcessTest test;\n> > + test.setArgs(argc, argv);\n> \n> Especially as here we see that now we have to manually 'know' that we\n> have to make a call to setArgs() after every construction...?\n> \n> It's local test code, so it's not particularly critical though.\n> \n> So if you want to keep it this way so-be-it.\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> \n> > + return test.execute();\n> > }\n> > diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp\n> > index 378d680bf4ef..96bea17f8dce 100644\n> > --- a/test/process/process_test.cpp\n> > +++ b/test/process/process_test.cpp\n> > @@ -9,7 +9,6 @@\n> > #include \n> > #include \n> > \n> > -\n> > #include \n> > #include \n> > #include \n> > @@ -106,5 +105,7 @@ int main(int argc, char **argv)\n> > return child.run(status);\n> > }\n> > \n> > - return ProcessTest().execute();\n> > + ProcessTest test;\n> > + test.setArgs(argc, argv);\n> > + return test.execute();\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 1CF7ABF415\n\tfor ;\n\tTue, 30 Nov 2021 11:03:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E141605AA;\n\tTue, 30 Nov 2021 12:03:00 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A72C6011A\n\tfor ;\n\tTue, 30 Nov 2021 12:02:59 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8E0898F0;\n\tTue, 30 Nov 2021 12:02:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Zq3I+81L\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638270178;\n\tbh=vNc/8fkQa9QdFqKz+04+KdgHiaserXfkCJA2JWAJJpI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Zq3I+81L0/Ykk3744nQRFF+5P4601hiB47Wvq787rFMAJJz2ldmulmCmFSlSmmxQV\n\tVsw9NB4RvTKALri25nS/aiCPyJurbbJT7EXWWJs86nEk0f7SC1bhrUHuO+wktA1Pyk\n\tfV50KPIMKc1+wYHyoE0l6bBTThQEmukmz1T5QScA=","Date":"Tue, 30 Nov 2021 13:02:33 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211130012303.10574-1-laurent.pinchart@ideasonboard.com>\n\t<20211130012303.10574-2-laurent.pinchart@ideasonboard.com>\n\t<163826974217.3059017.15103616780548132909@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163826974217.3059017.15103616780548132909@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21452,"web_url":"https://patchwork.libcamera.org/comment/21452/","msgid":"<163827532088.3059017.9072654749699366348@Monstersaurus>","date":"2021-11-30T12:28:40","subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2021-11-30 11:02:33)\n> Hi Kieran,\n> \n> On Tue, Nov 30, 2021 at 10:55:42AM +0000, Kieran Bingham wrote:\n> > Quoting Laurent Pinchart (2021-11-30 01:23:02)\n> > > Store the path to the test executable, found in argv[0], in the Test\n> > > instance. This can be useful for tests that need to fork processes.\n> > > \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > test/ipc/unixsocket.cpp | 4 +++-\n> > > test/ipc/unixsocket_ipc.cpp | 4 +++-\n> > > test/libtest/test.cpp | 5 +++++\n> > > test/libtest/test.h | 15 ++++++++++++---\n> > > test/log/log_process.cpp | 4 +++-\n> > > test/process/process_test.cpp | 5 +++--\n> > > 6 files changed, 29 insertions(+), 8 deletions(-)\n> > > \n> > > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> > > index 7270bf4d2fe7..4fc1c10a2125 100644\n> > > --- a/test/ipc/unixsocket.cpp\n> > > +++ b/test/ipc/unixsocket.cpp\n> > > @@ -501,5 +501,7 @@ int main(int argc, char **argv)\n> > > return slave.run(ipcfd);\n> > > }\n> > > \n> > > - return UnixSocketTest().execute();\n> > > + UnixSocketTest test;\n> > > + test.setArgs(argc, argv);\n> > > + return test.execute();\n> > > }\n> > > diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp\n> > > index ab5d25572d83..2e3b52ca4d4b 100644\n> > > --- a/test/ipc/unixsocket_ipc.cpp\n> > > +++ b/test/ipc/unixsocket_ipc.cpp\n> > > @@ -227,5 +227,7 @@ int main(int argc, char **argv)\n> > > return slave.run(ipcfd);\n> > > }\n> > > \n> > > - return UnixSocketTestIPC().execute();\n> > > + UnixSocketTestIPC test;\n> > > + test.setArgs(argc, argv);\n> > > + return test.execute();\n> > > }\n> > > diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp\n> > > index fd9f3d74d6ce..af37b4dd28ff 100644\n> > > --- a/test/libtest/test.cpp\n> > > +++ b/test/libtest/test.cpp\n> > > @@ -17,6 +17,11 @@ Test::~Test()\n> > > {\n> > > }\n> > > \n> > > +void Test::setArgs([[maybe_unused]] int argc, char *argv[])\n> > > +{\n> > > + self_ = argv[0];\n> > > +}\n> > > +\n> > > int Test::execute()\n> > > {\n> > > int ret;\n> > > diff --git a/test/libtest/test.h b/test/libtest/test.h\n> > > index ee01a22591f8..23b07743fd2a 100644\n> > > --- a/test/libtest/test.h\n> > > +++ b/test/libtest/test.h\n> > > @@ -8,6 +8,7 @@\n> > > #pragma once\n> > > \n> > > #include \n> > > +#include \n> > > \n> > > enum TestStatus {\n> > > TestPass = 0,\n> > > @@ -21,16 +22,24 @@ public:\n> > > Test();\n> > > virtual ~Test();\n> > > \n> > > + void setArgs(int argc, char *argv[]);\n> > \n> > Hrm... I would have been tempted to pass these into the constructor.\n> > \n> > They are now somewhat arbitrarily 'required' in some tests, so it may as\n> > well be that they are always required. (or at least requierd to be\n> > considered).\n> \n> That's what I initially did, but then every single test will have to\n> define a constructor just to forward the arguments to the base Test\n> class. I thought I'd get some push back :-) If you think that's better,\n> I'll go that way.\n\nArgh, I had forgotten/not realised that the Test() class is subclassed\nto make the tests, so indeed then everything has to have a constructor\ndefined explicitly.\n\nYes, I can see the pain there...\n\nOk - probably not much better that way, so lets keep it like this. Not\nfond of either ;-) so stick with the least effort...\n\n--\nKieran\n\n\n> \n> > > int execute();\n> > > \n> > > + const std::string &self() const { return self_; }\n> > > +\n> > > protected:\n> > > virtual int init() { return 0; }\n> > > virtual int run() = 0;\n> > > virtual void cleanup() {}\n> > > +\n> > > +private:\n> > > + std::string self_;\n> > > };\n> > > \n> > > -#define TEST_REGISTER(klass) \\\n> > > -int main([[maybe_unused]] int argc, [[maybe_unused]] char *argv[]) \\\n> > > +#define TEST_REGISTER(Klass) \\\n> > > +int main(int argc, char *argv[]) \\\n> > > { \\\n> > > - return klass().execute(); \\\n> > > + Klass klass; \\\n> > > + klass.setArgs(argc, argv); \\\n> > > + return klass.execute(); \\\n> > > }\n> > > diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp\n> > > index a56a399848a7..ca8351335f3a 100644\n> > > --- a/test/log/log_process.cpp\n> > > +++ b/test/log/log_process.cpp\n> > > @@ -154,5 +154,7 @@ int main(int argc, char **argv)\n> > > return child.run(status, num);\n> > > }\n> > > \n> > > - return LogProcessTest().execute();\n> > > + LogProcessTest test;\n> > > + test.setArgs(argc, argv);\n> > \n> > Especially as here we see that now we have to manually 'know' that we\n> > have to make a call to setArgs() after every construction...?\n> > \n> > It's local test code, so it's not particularly critical though.\n> > \n> > So if you want to keep it this way so-be-it.\n> > \n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> > \n> > > + return test.execute();\n> > > }\n> > > diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp\n> > > index 378d680bf4ef..96bea17f8dce 100644\n> > > --- a/test/process/process_test.cpp\n> > > +++ b/test/process/process_test.cpp\n> > > @@ -9,7 +9,6 @@\n> > > #include \n> > > #include \n> > > \n> > > -\n> > > #include \n> > > #include \n> > > #include \n> > > @@ -106,5 +105,7 @@ int main(int argc, char **argv)\n> > > return child.run(status);\n> > > }\n> > > \n> > > - return ProcessTest().execute();\n> > > + ProcessTest test;\n> > > + test.setArgs(argc, argv);\n> > > + return test.execute();\n> > > }\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 95625BF415\n\tfor ;\n\tTue, 30 Nov 2021 12:28:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CEABA605D1;\n\tTue, 30 Nov 2021 13:28:45 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B3BAD6011A\n\tfor ;\n\tTue, 30 Nov 2021 13:28:43 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4A3518F0;\n\tTue, 30 Nov 2021 13:28:43 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UcwGsOhz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638275323;\n\tbh=zXw+BBMywxZioWvxWRjdXOOEyfXttpg1POAwVcoRgV8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=UcwGsOhzmpvn0fKDKnx48dAePz7Qj/K00cyE6jGpBOx8GXBXXFpKvgMazHjaXoliP\n\tu/tQmbFfc2e/178A2vqjfBhUKQcW9nYsIYImd+jFHLQu5MGULRjZnaidSBMOptjDIP\n\t9Urri7ei37q4Matd0Jjr3X2+HCAmcS6Yt+tPzDa4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211130012303.10574-1-laurent.pinchart@ideasonboard.com>\n\t<20211130012303.10574-2-laurent.pinchart@ideasonboard.com>\n\t<163826974217.3059017.15103616780548132909@Monstersaurus>\n\t","From":"Kieran Bingham ","To":"Laurent Pinchart ","Date":"Tue, 30 Nov 2021 12:28:40 +0000","Message-ID":"<163827532088.3059017.9072654749699366348@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] test: Store path to the test\n\texecutable in Test class","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]