[libcamera-devel,v2] README: Add unit tests instructions

18810 diff mbox series

The unit tests require kernel modules to be loaded to support the
virtual test cameras. Add notes to the readme to highlight that it can
be done, and what kernel configurations are required.

Suggested-by: William Salmon <pointswaves@gmail.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---

v2:
 - Reword small fixes from Laurent
 - Move to contributing.rst

 Documentation/contributing.rst | 41 ++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

Hi All,

Quoting Kieran Bingham (2023-07-11 15:55:13)
> The unit tests require kernel modules to be loaded to support the
> virtual test cameras. Add notes to the readme to highlight that it can
> be done, and what kernel configurations are required.
> 
> Suggested-by: William Salmon <pointswaves@gmail.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Could I get any commments or tags on this please?

--
Kieran


> ---
> 
> v2:
>  - Reword small fixes from Laurent
>  - Move to contributing.rst
> 
>  Documentation/contributing.rst | 41 ++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/Documentation/contributing.rst b/Documentation/contributing.rst
> index 2f0b4921f7e6..27c77558a6e6 100644
> --- a/Documentation/contributing.rst
> +++ b/Documentation/contributing.rst
> @@ -59,6 +59,47 @@ uses `Doxygen`_.  Please make sure to document all code during development.
>  .. _Sphinx: https://www.sphinx-doc.org
>  .. _Doxygen: https://www.doxygen.nl
>  
> +Running the unit tests
> +----------------------
> +
> +When submitting patches, it is recommended to run the unit tests. The unit
> +tests rely on kernel drivers which produce virtual devices. These can be either
> +built into the kernel, or provided as modules (=y or =m):
> +
> +.. code::
> +
> +    CONFIG_MEDIA_TEST_SUPPORT=y
> +    CONFIG_V4L_TEST_DRIVERS=y
> +
> +    CONFIG_VIDEO_VIM2M=m
> +    CONFIG_VIDEO_VIMC=m
> +    CONFIG_VIDEO_VIVID=m
> +
> +If the kernel provides the test drivers as modules, they need to be loaded
> +before running the tests:
> +
> +.. code::
> +
> +    sudo modprobe vimc
> +    sudo modprobe vivid
> +    sudo modprobe vim2m
> +
> +Make sure your build configuration has tests enabled by running the following
> +in the build directory:
> +
> +.. code::
> +
> +    meson configure -Dtest=true
> +
> +Enabling 'test=true' will implicitly add the VIMC test pipeline handler to the
> +build configuration.
> +
> +Then the tests can be run with:
> +
> +.. code::
> +
> +  ninja -C build test
> +
>  Submitting Patches
>  ------------------
>  
> -- 
> 2.34.1
>

Hi Kieran,

On Tue, Aug 01, 2023 at 09:49:34AM +0100, Kieran Bingham via libcamera-devel wrote:
> Quoting Kieran Bingham (2023-07-11 15:55:13)
> > The unit tests require kernel modules to be loaded to support the
> > virtual test cameras. Add notes to the readme to highlight that it can
> > be done, and what kernel configurations are required.
> > 
> > Suggested-by: William Salmon <pointswaves@gmail.com>
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> Could I get any commments or tags on this please?

Sure :-)

> > ---
> > 
> > v2:
> >  - Reword small fixes from Laurent
> >  - Move to contributing.rst
> > 
> >  Documentation/contributing.rst | 41 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/Documentation/contributing.rst b/Documentation/contributing.rst
> > index 2f0b4921f7e6..27c77558a6e6 100644
> > --- a/Documentation/contributing.rst
> > +++ b/Documentation/contributing.rst
> > @@ -59,6 +59,47 @@ uses `Doxygen`_.  Please make sure to document all code during development.
> >  .. _Sphinx: https://www.sphinx-doc.org
> >  .. _Doxygen: https://www.doxygen.nl
> >  
> > +Running the unit tests

The file uses Title Case.

> > +----------------------
> > +
> > +When submitting patches, it is recommended to run the unit tests. The unit

Let's make it required, not recommended.

"Code submitted to the libcamera project is required to pass the unit tests."

> > +tests rely on kernel drivers which produce virtual devices. These can be either

s/kernel/Linux kernel/

> > +built into the kernel, or provided as modules (=y or =m):
> > +
> > +.. code::
> > +
> > +    CONFIG_MEDIA_TEST_SUPPORT=y
> > +    CONFIG_V4L_TEST_DRIVERS=y
> > +
> > +    CONFIG_VIDEO_VIM2M=m
> > +    CONFIG_VIDEO_VIMC=m
> > +    CONFIG_VIDEO_VIVID=m

Something to make it clear that the above is an excerpt of the kernel
config would be nice. Newcomers to libcamera will not necessarily be
familiar with the Linux kernel.

> > +
> > +If the kernel provides the test drivers as modules, they need to be loaded
> > +before running the tests:
> > +
> > +.. code::

.. code:: shell

Likewise below.

> > +
> > +    sudo modprobe vimc
> > +    sudo modprobe vivid
> > +    sudo modprobe vim2m

I would sort those alphabetically :-)

> > +
> > +Make sure your build configuration has tests enabled by running the following
> > +in the build directory:
> > +
> > +.. code::
> > +
> > +    meson configure -Dtest=true
> > +
> > +Enabling 'test=true' will implicitly add the VIMC test pipeline handler to the
> > +build configuration.
> > +
> > +Then the tests can be run with:
> > +
> > +.. code::
> > +
> > +  ninja -C build test

Above, you mention running the meson command in the build directory,
while here you specify it with -C. I would use

	meson configure build -Dtest=true

above and drop "in the build directory".

Also, indentation is inconsistent, you use a mix of four spaces and two
spaces for the code blocks. This issue is present in README.rst already,
and probably in other files as well, but let's try to be consistent
here. I think my preference would be to align under the 'c' of 'code'
(thus three spaces), but I won't reject other options :-)

Finally, I feel that this file has grown into a bit of a random bits of
contribution-related items, without much structure. It would be nice to
rework it later.

> > +
> >  Submitting Patches
> >  ------------------
> >