[libcamera-devel] [PATCH v1 1/2] test: Store path to the test executable in Test class

[Kieran Bingham]

Quoting Laurent Pinchart (2021-11-30 01:23:02)
> Store the path to the test executable, found in argv[0], in the Test
> instance. This can be useful for tests that need to fork processes.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  test/ipc/unixsocket.cpp       |  4 +++-
>  test/ipc/unixsocket_ipc.cpp   |  4 +++-
>  test/libtest/test.cpp         |  5 +++++
>  test/libtest/test.h           | 15 ++++++++++++---
>  test/log/log_process.cpp      |  4 +++-
>  test/process/process_test.cpp |  5 +++--
>  6 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp
> index 7270bf4d2fe7..4fc1c10a2125 100644
> --- a/test/ipc/unixsocket.cpp
> +++ b/test/ipc/unixsocket.cpp
> @@ -501,5 +501,7 @@ int main(int argc, char **argv)
>                 return slave.run(ipcfd);
>         }
>  
> -       return UnixSocketTest().execute();
> +       UnixSocketTest test;
> +       test.setArgs(argc, argv);
> +       return test.execute();
>  }
> diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp
> index ab5d25572d83..2e3b52ca4d4b 100644
> --- a/test/ipc/unixsocket_ipc.cpp
> +++ b/test/ipc/unixsocket_ipc.cpp
> @@ -227,5 +227,7 @@ int main(int argc, char **argv)
>                 return slave.run(ipcfd);
>         }
>  
> -       return UnixSocketTestIPC().execute();
> +       UnixSocketTestIPC test;
> +       test.setArgs(argc, argv);
> +       return test.execute();
>  }
> diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp
> index fd9f3d74d6ce..af37b4dd28ff 100644
> --- a/test/libtest/test.cpp
> +++ b/test/libtest/test.cpp
> @@ -17,6 +17,11 @@ Test::~Test()
>  {
>  }
>  
> +void Test::setArgs([[maybe_unused]] int argc, char *argv[])
> +{
> +       self_ = argv[0];
> +}
> +
>  int Test::execute()
>  {
>         int ret;
> diff --git a/test/libtest/test.h b/test/libtest/test.h
> index ee01a22591f8..23b07743fd2a 100644
> --- a/test/libtest/test.h
> +++ b/test/libtest/test.h
> @@ -8,6 +8,7 @@
>  #pragma once
>  
>  #include <sstream>
> +#include <string>
>  
>  enum TestStatus {
>         TestPass = 0,
> @@ -21,16 +22,24 @@ public:
>         Test();
>         virtual ~Test();
>  
> +       void setArgs(int argc, char *argv[]);

Hrm... I would have been tempted to pass these into the constructor.

They are now somewhat arbitrarily 'required' in some tests, so it may as
well be that they are always required. (or at least requierd to be
considered).

>         int execute();
>  
> +       const std::string &self() const { return self_; }
> +
>  protected:
>         virtual int init() { return 0; }
>         virtual int run() = 0;
>         virtual void cleanup() {}
> +
> +private:
> +       std::string self_;
>  };
>  
> -#define TEST_REGISTER(klass)                                           \
> -int main([[maybe_unused]] int argc, [[maybe_unused]] char *argv[])     \
> +#define TEST_REGISTER(Klass)                                           \
> +int main(int argc, char *argv[])                                       \
>  {                                                                      \
> -       return klass().execute();                                       \
> +       Klass klass;                                                    \
> +       klass.setArgs(argc, argv);                                      \
> +       return klass.execute();                                         \
>  }
> diff --git a/test/log/log_process.cpp b/test/log/log_process.cpp
> index a56a399848a7..ca8351335f3a 100644
> --- a/test/log/log_process.cpp
> +++ b/test/log/log_process.cpp
> @@ -154,5 +154,7 @@ int main(int argc, char **argv)
>                 return child.run(status, num);
>         }
>  
> -       return LogProcessTest().execute();
> +       LogProcessTest test;
> +       test.setArgs(argc, argv);

Especially as here we see that now we have to manually 'know' that we
have to make a call to setArgs() after every construction...?

It's local test code, so it's not particularly critical though.

So if you want to keep it this way so-be-it.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


> +       return test.execute();
>  }
> diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
> index 378d680bf4ef..96bea17f8dce 100644
> --- a/test/process/process_test.cpp
> +++ b/test/process/process_test.cpp
> @@ -9,7 +9,6 @@
>  #include <unistd.h>
>  #include <vector>
>  
> -
>  #include <libcamera/base/event_dispatcher.h>
>  #include <libcamera/base/thread.h>
>  #include <libcamera/base/timer.h>
> @@ -106,5 +105,7 @@ int main(int argc, char **argv)
>                 return child.run(status);
>         }
>  
> -       return ProcessTest().execute();
> +       ProcessTest test;
> +       test.setArgs(argc, argv);
> +       return test.execute();
>  }
> -- 
> Regards,
> 
> Laurent Pinchart
>