[{"id":27643,"web_url":"https://patchwork.libcamera.org/comment/27643/","msgid":"<169087977448.137962.14288185439333617545@Monstersaurus>","date":"2023-08-01T08:49:34","subject":"Re: [libcamera-devel] [PATCH v2] README: Add unit tests instructions","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi All,\n\nQuoting Kieran Bingham (2023-07-11 15:55:13)\n> The unit tests require kernel modules to be loaded to support the\n> virtual test cameras. Add notes to the readme to highlight that it can\n> be done, and what kernel configurations are required.\n> \n> Suggested-by: William Salmon \n> Signed-off-by: Kieran Bingham \n\nCould I get any commments or tags on this please?\n\n--\nKieran\n\n\n> ---\n> \n> v2:\n> - Reword small fixes from Laurent\n> - Move to contributing.rst\n> \n> Documentation/contributing.rst | 41 ++++++++++++++++++++++++++++++++++\n> 1 file changed, 41 insertions(+)\n> \n> diff --git a/Documentation/contributing.rst b/Documentation/contributing.rst\n> index 2f0b4921f7e6..27c77558a6e6 100644\n> --- a/Documentation/contributing.rst\n> +++ b/Documentation/contributing.rst\n> @@ -59,6 +59,47 @@ uses `Doxygen`_. Please make sure to document all code during development.\n> .. _Sphinx: https://www.sphinx-doc.org\n> .. _Doxygen: https://www.doxygen.nl\n> \n> +Running the unit tests\n> +----------------------\n> +\n> +When submitting patches, it is recommended to run the unit tests. The unit\n> +tests rely on kernel drivers which produce virtual devices. These can be either\n> +built into the kernel, or provided as modules (=y or =m):\n> +\n> +.. code::\n> +\n> + CONFIG_MEDIA_TEST_SUPPORT=y\n> + CONFIG_V4L_TEST_DRIVERS=y\n> +\n> + CONFIG_VIDEO_VIM2M=m\n> + CONFIG_VIDEO_VIMC=m\n> + CONFIG_VIDEO_VIVID=m\n> +\n> +If the kernel provides the test drivers as modules, they need to be loaded\n> +before running the tests:\n> +\n> +.. code::\n> +\n> + sudo modprobe vimc\n> + sudo modprobe vivid\n> + sudo modprobe vim2m\n> +\n> +Make sure your build configuration has tests enabled by running the following\n> +in the build directory:\n> +\n> +.. code::\n> +\n> + meson configure -Dtest=true\n> +\n> +Enabling 'test=true' will implicitly add the VIMC test pipeline handler to the\n> +build configuration.\n> +\n> +Then the tests can be run with:\n> +\n> +.. code::\n> +\n> + ninja -C build test\n> +\n> Submitting Patches\n> ------------------\n> \n> -- \n> 2.34.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 DDF2EBDB13\n\tfor ;\n\tTue, 1 Aug 2023 08:49:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5A96A627E9;\n\tTue, 1 Aug 2023 10:49:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D5763627E6\n\tfor ;\n\tTue, 1 Aug 2023 10:49:37 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C9B948D;\n\tTue, 1 Aug 2023 10:48:34 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1690879779;\n\tbh=6gpjzRNor9H/S5dqxCcnc9VY8fmtg5ogM4EbDRDJdI4=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ESsoTsaRdjVsDRYMcRbnZVg9OBy5qUu3gQA/oTd++XC0jPucy+sIB5bpS6EfClrwv\n\tL2yEQT/vgV8W0BMtBRFQViXQ9bdl9OKxD2FXy4WBBuy+bXIqQUtAKfBwbdGBBEkbJd\n\teWY2/6rSRYQ1VIVrhNDYNqXHEFSnvBvY47PFkiu9KknSf/bm6bbMwDl1z5GaFcS3+Q\n\tMZXrgH/xf7BqG5e/+7DF+5jTTDSlOpoA6cdPhIu9qkzUrkdyETXZFW727tinZNjF5j\n\tu4Atm1XEK64RC/N/7stWz6CsB/CFGNT2zBKldCZXHEMNRP4El/2+P/ORhLCp7oXi1i\n\tXacaeHKt6n5WQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1690879714;\n\tbh=6gpjzRNor9H/S5dqxCcnc9VY8fmtg5ogM4EbDRDJdI4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=UCAyYHs9qA36aM816mKVPEjRhANmECAkYdb8fkWZghqdnWfwAUi7b4ZfADFlzcQOz\n\t4Qo71oG4SngljV7jdht1tocafNSathD7DXqvIU0BB31632pe72V1ZsN46FtOGZnHOg\n\twVw/SfTY+bxVy9BRZ+P70uXBvfpuqKQ9VZqGDADc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"UCAyYHs9\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230711145513.714742-1-kieran.bingham@ideasonboard.com>","References":"<20230711145513.714742-1-kieran.bingham@ideasonboard.com>","To":"libcamera devel ","Date":"Tue, 01 Aug 2023 09:49:34 +0100","Message-ID":"<169087977448.137962.14288185439333617545@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2] README: Add unit tests instructions","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"William Salmon ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27657,"web_url":"https://patchwork.libcamera.org/comment/27657/","msgid":"<20230802210040.GE29718@pendragon.ideasonboard.com>","date":"2023-08-02T21:00:40","subject":"Re: [libcamera-devel] [PATCH v2] README: Add unit tests instructions","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, Aug 01, 2023 at 09:49:34AM +0100, Kieran Bingham via libcamera-devel wrote:\n> Quoting Kieran Bingham (2023-07-11 15:55:13)\n> > The unit tests require kernel modules to be loaded to support the\n> > virtual test cameras. Add notes to the readme to highlight that it can\n> > be done, and what kernel configurations are required.\n> > \n> > Suggested-by: William Salmon \n> > Signed-off-by: Kieran Bingham \n> \n> Could I get any commments or tags on this please?\n\nSure :-)\n\n> > ---\n> > \n> > v2:\n> > - Reword small fixes from Laurent\n> > - Move to contributing.rst\n> > \n> > Documentation/contributing.rst | 41 ++++++++++++++++++++++++++++++++++\n> > 1 file changed, 41 insertions(+)\n> > \n> > diff --git a/Documentation/contributing.rst b/Documentation/contributing.rst\n> > index 2f0b4921f7e6..27c77558a6e6 100644\n> > --- a/Documentation/contributing.rst\n> > +++ b/Documentation/contributing.rst\n> > @@ -59,6 +59,47 @@ uses `Doxygen`_. Please make sure to document all code during development.\n> > .. _Sphinx: https://www.sphinx-doc.org\n> > .. _Doxygen: https://www.doxygen.nl\n> > \n> > +Running the unit tests\n\nThe file uses Title Case.\n\n> > +----------------------\n> > +\n> > +When submitting patches, it is recommended to run the unit tests. The unit\n\nLet's make it required, not recommended.\n\n\"Code submitted to the libcamera project is required to pass the unit tests.\"\n\n> > +tests rely on kernel drivers which produce virtual devices. These can be either\n\ns/kernel/Linux kernel/\n\n> > +built into the kernel, or provided as modules (=y or =m):\n> > +\n> > +.. code::\n> > +\n> > + CONFIG_MEDIA_TEST_SUPPORT=y\n> > + CONFIG_V4L_TEST_DRIVERS=y\n> > +\n> > + CONFIG_VIDEO_VIM2M=m\n> > + CONFIG_VIDEO_VIMC=m\n> > + CONFIG_VIDEO_VIVID=m\n\nSomething to make it clear that the above is an excerpt of the kernel\nconfig would be nice. Newcomers to libcamera will not necessarily be\nfamiliar with the Linux kernel.\n\n> > +\n> > +If the kernel provides the test drivers as modules, they need to be loaded\n> > +before running the tests:\n> > +\n> > +.. code::\n\n.. code:: shell\n\nLikewise below.\n\n> > +\n> > + sudo modprobe vimc\n> > + sudo modprobe vivid\n> > + sudo modprobe vim2m\n\nI would sort those alphabetically :-)\n\n> > +\n> > +Make sure your build configuration has tests enabled by running the following\n> > +in the build directory:\n> > +\n> > +.. code::\n> > +\n> > + meson configure -Dtest=true\n> > +\n> > +Enabling 'test=true' will implicitly add the VIMC test pipeline handler to the\n> > +build configuration.\n> > +\n> > +Then the tests can be run with:\n> > +\n> > +.. code::\n> > +\n> > + ninja -C build test\n\nAbove, you mention running the meson command in the build directory,\nwhile here you specify it with -C. I would use\n\n\tmeson configure build -Dtest=true\n\nabove and drop \"in the build directory\".\n\nAlso, indentation is inconsistent, you use a mix of four spaces and two\nspaces for the code blocks. This issue is present in README.rst already,\nand probably in other files as well, but let's try to be consistent\nhere. I think my preference would be to align under the 'c' of 'code'\n(thus three spaces), but I won't reject other options :-)\n\nFinally, I feel that this file has grown into a bit of a random bits of\ncontribution-related items, without much structure. It would be nice to\nrework it later.\n\n> > +\n> > Submitting Patches\n> > ------------------\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 482B8BDCBF\n\tfor ;\n\tWed, 2 Aug 2023 21:00:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8FE60627EB;\n\tWed, 2 Aug 2023 23:00:36 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BA35861E1C\n\tfor ;\n\tWed, 2 Aug 2023 23:00:35 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 82785922;\n\tWed, 2 Aug 2023 22:59:31 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1691010036;\n\tbh=SsMDVT1+zAOV1ugXe2QTb18TLuyvwPVEEasRnd9csgI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=rwcnEqTyc4teT+WYcS/jzDt1SeO7NmzLcGKN7GseE+oUAcEwtQYfRoIl83yOAub5J\n\t34pziIqaw/Aq5XMZX+G0c7WbbKMbQNa7JhzlnBjEqsCQhXUirnLG7c6W1BUsnSHFJ2\n\t4AU6vS+NRTzOXX42SeHkOuFSht5Fjikjbw0fYhgZmuKu/FyXC/ANKjcy2Kw7VXgieH\n\tiLae1cj3YRyasnccsLVMZFEaBjhwnifMqCOl4jQ5yu0q/GXoxslH2x+rNzuRMQHcwA\n\t7pNbTRVzlOhDSeoe0+YGDhGUu2wehJsNN5RvMyvU7Cz47zgkXOpyIxOTvDQb8W+JQF\n\tuD68Bq9V3RQsw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1691009971;\n\tbh=SsMDVT1+zAOV1ugXe2QTb18TLuyvwPVEEasRnd9csgI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CbFdAg0rDQkcd43ETz1YEWcgiBRiH8OfnWUXKARo4gKhbWFVSxLR4Xd7wO8HI2EOA\n\tcqByqX75c/CT3omWHR92hNRiAR1GlazzD06s2+QdEXK8frzBkZBz6op7/gPI+/MzTK\n\tpad+QtiBnPuHPDxuanX+usMKxYiUmFO3hanjafAc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CbFdAg0r\"; dkim-atps=neutral","Date":"Thu, 3 Aug 2023 00:00:40 +0300","To":"Kieran Bingham ","Message-ID":"<20230802210040.GE29718@pendragon.ideasonboard.com>","References":"<20230711145513.714742-1-kieran.bingham@ideasonboard.com>\n\t<169087977448.137962.14288185439333617545@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<169087977448.137962.14288185439333617545@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v2] README: Add unit tests instructions","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera devel ,\n\tWilliam Salmon ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]