[{"id":13044,"web_url":"https://patchwork.libcamera.org/comment/13044/","msgid":"<20201007120301.vsxgffg7vae2gbkh@oden.dyn.berto.se>","date":"2020-10-07T12:03:01","subject":"Re: [libcamera-devel] [RFC PATCH 1/1] test: cam tool testing with\n\tvalgrind","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nOn 2020-10-07 12:25:44 +0100, Kieran Bingham wrote:\n> Implement a test which runs cam to capture 10 frames,\n> and validates there are no memory leaks with valgrind.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n> \n> A few points to consider here:\n> \n> Duration\n> --------\n> Unfortuantely this test adds an extra 13 seconds to the test run on my\n> system. We could reduce the duration of course, but it will still be a\n> reasonably substantial addition.\n\nI think this is indeed a good tool to have in our arsenal. I'm starting \nto wonder if we need to split our test directory in a 'release' and \n'normal' mode. In release we would have tests that takes longer and are \nmore focused to catch deeper errors such as memory leaks or other issues \nthat requires a longer runtime. While in 'normal' we would have tests \nsimilar that we have today that focus on incorrect behavior of the API.  \nWhat do you think?\n\nI think I would be unhappy for this test to run (without any way to \ndisable it) for every ninja test run as I already think our tests takes \ntoo much time ;-)\n\n> \n> TAP\n> ---\n> This runs a test for mulitiple cameras, and meson now supports 'tap'\n> format tests (multiple tests in one execution). In this patch - the\n> 'protocol : \"tap\" is disabled, because meson doesn't like the output of\n> the rest of the test being mixed with the tap output. I'm not sure of a\n> good way to handle that. The output is useful, so we want it somewhere,\n> and ideally in the test log - but we also want the tap parsing to work\n> in that case too.\n> Anyway, with the tap protocol disabled, it just falls back to the return\n> status which is also handled here. Still, this is a good opportunity to\n> see how 'tap' could help our tests, and might revive my old patches to\n> use common test helpers across the rest of test/\n\nI'm a huge fan of TAP, do you think it would require a lot of work to \nchange our current tests output to be valid TAP? If I recall correctly \nprepending a # too all non-TAP lines is nought to make any text stream \ninto a valid (yet not so useful) TAP stream.\n\nOne thing about TAP I learnt the hard way is that the number of planed \ntests (keyword \"1..5\") does not need to go first in the TAP stream, it's \nperfectly valid to provide this last. The results should only be \nconsidered incomplete if there is no plan provided anywhere. This have \nhelped me in the past turning things into being TAP compatible.\n\nLastly a word of caution of the TAP13 specification, please don't :-)\n\n> \n> Valgrind\n> --------\n> Parsing valgrind is a real pain. This test *only* checks to see if there\n> are any memory leaks, and does not report on any of the other topics\n> valgirind might highlight. (They could be added later of course).\n> Ideally we would want a machine parsable output from valgrind. It can\n> output an xml report ... but ... that's a lot more parsing too ;-)\n> \n> Thoughts/Suggestions?\n\nIs it really useful information to parse this out? As long as the \nvalgrind output is keept in a log file I think a PASS/FAIL of the run is \nenough as someone who debugs it would like find more informaiton in the \nlog then from parsed values presented in the test run.\n\n> \n> https://github.com/adobe/chromium/blob/master/tools/valgrind/memcheck_analyze.py\n> seems to show an existing tool that parses the valgrind output as one\n> possible path - but I'm happy to hear of any better ideas too.\n> \n> \n\nAs a last point I wonder if we should create a new binary in tests that \niterates over all cameras while using the same CameraManager instance \nand looper over them 2-3 times. I think that could exercise more code \npaths then creating a new CamereManger for each run (as we do with cam).  \nBut this can of course be done on top or maybe even in addition.\n\n> \n>  test/cam/capture-test.sh | 71 ++++++++++++++++++++++++++++++++++++++++\n>  test/cam/meson.build     | 10 ++++++\n>  test/meson.build         |  1 +\n>  3 files changed, 82 insertions(+)\n>  create mode 100755 test/cam/capture-test.sh\n>  create mode 100644 test/cam/meson.build\n> \n> diff --git a/test/cam/capture-test.sh b/test/cam/capture-test.sh\n> new file mode 100755\n> index 000000000000..ab808976be72\n> --- /dev/null\n> +++ b/test/cam/capture-test.sh\n> @@ -0,0 +1,71 @@\n> +#!/bin/sh\n> +\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +\n> +TestPass=0\n> +TestFail=255\n> +TestSkip=77\n> +\n> +# Initialise success, set for failure.\n> +ret=$TestPass\n> +\n> +ok() {\n> +\techo \"ok $*\"\n> +}\n> +\n> +nok() {\n> +\techo \"not ok $*\"\n> +\tret=$TestFail\n> +}\n> +\n> +valgrind=$(command -v valgrind)\n> +\n> +if [ x\"\" = x\"$valgrind\" ] ; then\n> +\techo \"skip 1 - Valgrind unavailable ...\"\n> +\texit $TestSkip\n> +fi\n> +\n> +# Tests expect to be run from the meson.project_build_root()\n> +cam=${1:-src/cam/cam}\n> +\n> +if [ ! -e \"$cam\" ] ; then\n> +\tnok \"1 - failed to find cam utility.\"\n> +\texit $TestFail\n> +fi\n> +\n> +# Unfortunately, we don't have a 'machine interface', so we rely on parsing the\n> +# output of cam...\n> +num_cameras=$(\"$cam\" -l | grep -v \"Available\" | wc -l)\n> +\n> +# Enter TAP plan\n> +echo \"1..$num_cameras\"\n> +\n> +for i in $(seq 1 1 \"$num_cameras\");\n> +do\n> +\t\"$cam\" -c \"$i\" -C10\n> +\tret=$?\n> +\tif [ $ret != 0 ] ; then\n> +\t\tnok \"$i - $cam returned $ret\"\n> +\t\tcontinue\n> +\tfi\n> +\n> +\tlog_file=\"valgrind-cam-$i.log\"\n> +\t\"$valgrind\" \"$cam\" -c \"$i\" -C10 > \"$log_file\" 2>&1\n> +\tret=$?\n> +\tif [ $ret != 0 ] ; then\n> +\t\tnok \"$i - $valgrind returned $ret\"\n> +\t\tcontinue\n> +\tfi\n> +\n> +\t# I'd prefer a better way of checking there are no leaks, as well as reporting\n> +\t# the different categories from valgrind as distinct tests.\n> +\tif ! grep \"no leaks are possible\" \"$log_file\" > /dev/null; then\n> +\t\tnok \"$i - Valgrind Errors detected\"\n> +\t\tcat $log_file > /dev/stderr\n> +\t\tcontinue\n> +\tfi\n> +\n> +\tok \"$i - Camera $i reports no leaks\"\n> +done;\n> +\n> +exit $ret\n> diff --git a/test/cam/meson.build b/test/cam/meson.build\n> new file mode 100644\n> index 000000000000..834c9bcf6b86\n> --- /dev/null\n> +++ b/test/cam/meson.build\n> @@ -0,0 +1,10 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +cam_capture = files('capture-test.sh')\n> +\n> +test('cam-capture-test', cam_capture,\n> +    args : cam,\n> +    suite : 'integration',\n> +    is_parallel : false,\n> +    #protocol : 'tap',\n> +    timeout : 60)\n> diff --git a/test/meson.build b/test/meson.build\n> index 0a1d434e3996..d1b24220dc7c 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -2,6 +2,7 @@\n>  \n>  subdir('libtest')\n>  \n> +subdir('cam')\n>  subdir('camera')\n>  subdir('controls')\n>  subdir('ipa')\n> -- \n> 2.25.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6098BBEEDF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Oct 2020 12:03:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D757760553;\n\tWed,  7 Oct 2020 14:03:05 +0200 (CEST)","from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7648760579\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Oct 2020 14:03:04 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id z8so1945464lfd.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 07 Oct 2020 05:03:04 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\td21sm309204lfl.150.2020.10.07.05.03.01\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 07 Oct 2020 05:03:01 -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=\"LEoqwkGP\"; 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=5qrLuH3nPK6MAiP4Jdq8M/bzPKXrmfQxhUD+L+xgtcU=;\n\tb=LEoqwkGPOWCGVI0yZE6+Z0lcawdYtfpjqDTSVL0nJAju0nMpQC0flbcoDyEJ3tumnu\n\t+UrStJ9vD8ySaZ0WeeSxXuayLEZrxrFLRqrdod4mFgH1iAr9Fsoz2O8A01qwSIDrX93Z\n\tBKlOGe6ouJpod/WRa46PUPVXSHRp/1GB4tUjfIOcfdvP+wJz4STN7Fl8xGe0owqXBe4+\n\tvu4gCzDRLb5iqpbuPp1Dpx/qtnvGPQd2dmrnDXl57SkCLi387PdN7LNchPptow99n8pl\n\tthJAUGNJt4BI2hOd4LUngtcGe/+XwkmJg4aT1Ld3Nq2HX0jfEbd92TAjScsr+H7tjJHK\n\t0BFw==","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=5qrLuH3nPK6MAiP4Jdq8M/bzPKXrmfQxhUD+L+xgtcU=;\n\tb=oFpa0acsPbznwGmCpbmSvddhHkasqQzDi20HGPUGKObEcgd2aIwVr+6q5Q4FWj1YyL\n\ty/0wAJBAauj9hv3lCPC5Z6JHmGsKZcxLZ9ESKJygljdlWS2HyFMvQEdQJHRA7JAHrflP\n\tqQI/fw+m96gzyum+ojyUYPRf8Rr6fq/RJq+zFx1uMaCewGR3AnN4CPpwKn9p42i114vC\n\tymDTEZSmYSBWOWAz1Gg6AUSlG4epdlHh9FkE9JJ4E+M+H9W0Y16qsT2BUVIqdRmtm0h1\n\tPl9rzGEbaKm9phRW53sgDVRm+GONU41RuWaeocw1RtLIDibzulh/kLRsedKiCWmQ78WJ\n\tYj4A==","X-Gm-Message-State":"AOAM5325OMNC8kj5CPqk6UZE7axVZydGdsOTLvzN6lFJu+jL38b7rF/n\n\tohsXkKLyJTEHzsGqrUiGQImWIsY6hbCdIw==","X-Google-Smtp-Source":"ABdhPJzWe4GlqMc2pcIOhhVOS9gxuAdPcicfP1y755aCS4Rs8Ofr3EIJmiZ7ZVN8xboIZ2ibFgqQeA==","X-Received":"by 2002:a05:6512:3692:: with SMTP id\n\td18mr802189lfs.62.1602072182441; \n\tWed, 07 Oct 2020 05:03:02 -0700 (PDT)","Date":"Wed, 7 Oct 2020 14:03:01 +0200","From":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20201007120301.vsxgffg7vae2gbkh@oden.dyn.berto.se>","References":"<20201007112544.3169293-1-kieran.bingham@ideasonboard.com>\n\t<20201007112544.3169293-2-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201007112544.3169293-2-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 1/1] test: cam tool testing with\n\tvalgrind","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13045,"web_url":"https://patchwork.libcamera.org/comment/13045/","msgid":"<7a808811-f225-39f1-2255-ffd0c157de4e@ideasonboard.com>","date":"2020-10-07T12:26:40","subject":"Re: [libcamera-devel] [RFC PATCH 1/1] test: cam tool testing with\n\tvalgrind","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 07/10/2020 13:03, Niklas Söderlund wrote:\n> Hi Kieran,\n> \n> On 2020-10-07 12:25:44 +0100, Kieran Bingham wrote:\n>> Implement a test which runs cam to capture 10 frames,\n>> and validates there are no memory leaks with valgrind.\n>>\n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> ---\n>>\n>> A few points to consider here:\n>>\n>> Duration\n>> --------\n>> Unfortuantely this test adds an extra 13 seconds to the test run on my\n>> system. We could reduce the duration of course, but it will still be a\n>> reasonably substantial addition.\n> \n> I think this is indeed a good tool to have in our arsenal. I'm starting \n> to wonder if we need to split our test directory in a 'release' and \n> 'normal' mode. In release we would have tests that takes longer and are \n> more focused to catch deeper errors such as memory leaks or other issues \n> that requires a longer runtime. While in 'normal' we would have tests \n> similar that we have today that focus on incorrect behavior of the API.  \n> What do you think?\n> \n> I think I would be unhappy for this test to run (without any way to \n> disable it) for every ninja test run as I already think our tests takes \n> too much time ;-)\n\nI'm fine with that, a 'smoke-test' suite, and a full suite. I'll leave\nthe definition of what goes it the smoke-test to later, after we\ndetermine if it's possible. But I certainly don't object ;-)\n\n\n>> TAP\n>> ---\n>> This runs a test for mulitiple cameras, and meson now supports 'tap'\n>> format tests (multiple tests in one execution). In this patch - the\n>> 'protocol : \"tap\" is disabled, because meson doesn't like the output of\n>> the rest of the test being mixed with the tap output. I'm not sure of a\n>> good way to handle that. The output is useful, so we want it somewhere,\n>> and ideally in the test log - but we also want the tap parsing to work\n>> in that case too.\n>> Anyway, with the tap protocol disabled, it just falls back to the return\n>> status which is also handled here. Still, this is a good opportunity to\n>> see how 'tap' could help our tests, and might revive my old patches to\n>> use common test helpers across the rest of test/\n> \n> I'm a huge fan of TAP, do you think it would require a lot of work to \n> change our current tests output to be valid TAP? If I recall correctly \n> prepending a # too all non-TAP lines is nought to make any text stream \n> into a valid (yet not so useful) TAP stream.\n\nOk - prepending sounds like a good route then. Maybe we can throw\nsomething together that automatically redirects stdout/stderr to two\ndifferent prefixes.\n\n1...3\n#out: a standard out message\n#err: A standard error mesage/warning\nnot ok - 1 There was an error in this test.\n\n> One thing about TAP I learnt the hard way is that the number of planed \n> tests (keyword \"1..5\") does not need to go first in the TAP stream, it's \n> perfectly valid to provide this last. The results should only be \n\nYes, I discovered this recently too. That helps a lot when you don't\nknow how many tests you are going to run until you run them ;-)\n\n> considered incomplete if there is no plan provided anywhere. This have \n> helped me in the past turning things into being TAP compatible.\n> \n> Lastly a word of caution of the TAP13 specification, please don't :-)\n\nAck. I haven't looked at TAP13. I'll avoid it ;-)\n\n\n>> Valgrind\n>> --------\n>> Parsing valgrind is a real pain. This test *only* checks to see if there\n>> are any memory leaks, and does not report on any of the other topics\n>> valgirind might highlight. (They could be added later of course).\n>> Ideally we would want a machine parsable output from valgrind. It can\n>> output an xml report ... but ... that's a lot more parsing too ;-)\n>>\n>> Thoughts/Suggestions?\n> \n> Is it really useful information to parse this out? As long as the \n> valgrind output is keept in a log file I think a PASS/FAIL of the run is \n> enough as someone who debugs it would like find more informaiton in the \n> log then from parsed values presented in the test run.\n\n\nThe issue is - It's hard to (easily/simply) identify if there is a\nreason to consider valgrind a pass/fail.\n\ngrepping the logs seems fragile and awkward --- It would be nice if\nvalgrind had a better summary that said:\n  - leaks: 5\n  - uninitialsed data: 2\n  - Total errors: 7\n\nOr something really easy to consider a pass/fail.\n\nFor example, without Tomi's current fixup patches, this test 'passes'\neven though valgrind has (correctly) identified and reported the use of\nuninitialised variables, and an invalid close(-1) syscall.\n\nIt would be nice if (easily) we could report all valgrind errors here.\nIt feels too obvious - so I must have missed something staring me in the\nface :-(\n\n\n\n> \n>>\n>> https://github.com/adobe/chromium/blob/master/tools/valgrind/memcheck_analyze.py\n>> seems to show an existing tool that parses the valgrind output as one\n>> possible path - but I'm happy to hear of any better ideas too.\n>>\n>>\n> \n> As a last point I wonder if we should create a new binary in tests that \n> iterates over all cameras while using the same CameraManager instance \n> and looper over them 2-3 times. I think that could exercise more code \n> paths then creating a new CamereManger for each run (as we do with cam).  \n> But this can of course be done on top or maybe even in addition.\n\nYes, that sounds like another internal test to add.\n\nMeson does have the facilility to wrap everthing with valgrind too ...\nso perhaps we should also consider that - but I thought this route might\nbe interesting too.\n\n$ meson test --wrap='valgrind --tool=helgrind -v --error-exitcode=1'\n\nOk - so I grabbed that from a search - and look at that\n--error-exitcode=1 ! That looks like what I might have been missing ;-)\n\n\nWell well well ... so I've run meson test --wrap=valgrind and I get:\n\n$ meson test --wrap=valgrind\n14/57 libcamera:ipc / unixsocket                       FAIL\n16/57 libcamera:log / log_process                      FAIL\n22/57 libcamera:process / process_test                 FAIL\n55/57 libcamera / timer                                FAIL\n\n(trimmed of course)\n\nand worse:\n\nmeson test --wrap='valgrind --tool=helgrind -v --error-exitcode=1'\n<trim all the failures>\n 1/57 libcamera:integration / cam-capture-test         OK\n38/57 libcamera / span                                 OK\n\nOk:                 2\nExpected Fail:      0\nFail:               55\nUnexpected Pass:    0\nSkipped:            0\nTimeout:            0\n\nIt seems that might be potentially due to a common failure in the\nCameraTest::CameraTest though.\n\nTime for some digging I guess...\n\n--\nKieran\n\n\n\n> \n>>\n>>  test/cam/capture-test.sh | 71 ++++++++++++++++++++++++++++++++++++++++\n>>  test/cam/meson.build     | 10 ++++++\n>>  test/meson.build         |  1 +\n>>  3 files changed, 82 insertions(+)\n>>  create mode 100755 test/cam/capture-test.sh\n>>  create mode 100644 test/cam/meson.build\n>>\n>> diff --git a/test/cam/capture-test.sh b/test/cam/capture-test.sh\n>> new file mode 100755\n>> index 000000000000..ab808976be72\n>> --- /dev/null\n>> +++ b/test/cam/capture-test.sh\n>> @@ -0,0 +1,71 @@\n>> +#!/bin/sh\n>> +\n>> +# SPDX-License-Identifier: GPL-2.0-or-later\n>> +\n>> +TestPass=0\n>> +TestFail=255\n>> +TestSkip=77\n>> +\n>> +# Initialise success, set for failure.\n>> +ret=$TestPass\n>> +\n>> +ok() {\n>> +\techo \"ok $*\"\n>> +}\n>> +\n>> +nok() {\n>> +\techo \"not ok $*\"\n>> +\tret=$TestFail\n>> +}\n>> +\n>> +valgrind=$(command -v valgrind)\n>> +\n>> +if [ x\"\" = x\"$valgrind\" ] ; then\n>> +\techo \"skip 1 - Valgrind unavailable ...\"\n>> +\texit $TestSkip\n>> +fi\n>> +\n>> +# Tests expect to be run from the meson.project_build_root()\n>> +cam=${1:-src/cam/cam}\n>> +\n>> +if [ ! -e \"$cam\" ] ; then\n>> +\tnok \"1 - failed to find cam utility.\"\n>> +\texit $TestFail\n>> +fi\n>> +\n>> +# Unfortunately, we don't have a 'machine interface', so we rely on parsing the\n>> +# output of cam...\n>> +num_cameras=$(\"$cam\" -l | grep -v \"Available\" | wc -l)\n>> +\n>> +# Enter TAP plan\n>> +echo \"1..$num_cameras\"\n>> +\n>> +for i in $(seq 1 1 \"$num_cameras\");\n>> +do\n>> +\t\"$cam\" -c \"$i\" -C10\n>> +\tret=$?\n>> +\tif [ $ret != 0 ] ; then\n>> +\t\tnok \"$i - $cam returned $ret\"\n>> +\t\tcontinue\n>> +\tfi\n>> +\n>> +\tlog_file=\"valgrind-cam-$i.log\"\n>> +\t\"$valgrind\" \"$cam\" -c \"$i\" -C10 > \"$log_file\" 2>&1\n>> +\tret=$?\n>> +\tif [ $ret != 0 ] ; then\n>> +\t\tnok \"$i - $valgrind returned $ret\"\n>> +\t\tcontinue\n>> +\tfi\n>> +\n>> +\t# I'd prefer a better way of checking there are no leaks, as well as reporting\n>> +\t# the different categories from valgrind as distinct tests.\n>> +\tif ! grep \"no leaks are possible\" \"$log_file\" > /dev/null; then\n>> +\t\tnok \"$i - Valgrind Errors detected\"\n>> +\t\tcat $log_file > /dev/stderr\n>> +\t\tcontinue\n>> +\tfi\n>> +\n>> +\tok \"$i - Camera $i reports no leaks\"\n>> +done;\n>> +\n>> +exit $ret\n>> diff --git a/test/cam/meson.build b/test/cam/meson.build\n>> new file mode 100644\n>> index 000000000000..834c9bcf6b86\n>> --- /dev/null\n>> +++ b/test/cam/meson.build\n>> @@ -0,0 +1,10 @@\n>> +# SPDX-License-Identifier: CC0-1.0\n>> +\n>> +cam_capture = files('capture-test.sh')\n>> +\n>> +test('cam-capture-test', cam_capture,\n>> +    args : cam,\n>> +    suite : 'integration',\n>> +    is_parallel : false,\n>> +    #protocol : 'tap',\n>> +    timeout : 60)\n>> diff --git a/test/meson.build b/test/meson.build\n>> index 0a1d434e3996..d1b24220dc7c 100644\n>> --- a/test/meson.build\n>> +++ b/test/meson.build\n>> @@ -2,6 +2,7 @@\n>>  \n>>  subdir('libtest')\n>>  \n>> +subdir('cam')\n>>  subdir('camera')\n>>  subdir('controls')\n>>  subdir('ipa')\n>> -- \n>> 2.25.1\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 9E5F6BEEE0\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Oct 2020 12:26:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 26BBE60580;\n\tWed,  7 Oct 2020 14:26:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A250960553\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Oct 2020 14:26:43 +0200 (CEST)","from [192.168.0.20]\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 B5C8B9DA;\n\tWed,  7 Oct 2020 14:26:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"slOU6/7x\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602073603;\n\tbh=p70E1cK8YrhuG+INu9LqDijyYdkw2aYfnOGCud0c+bw=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=slOU6/7x3BqVXvftGwdwCcrkHQHfaxy0THQ9TeXMlN1GfNys24quKqwZNxcozUv6H\n\t9WS4ZHXSm7gcrvSQ46TbQtIx7j3UF6KWkbmx73o6HdWiJGe7/TFiY7NwnjlEzkWrL+\n\tyP6g/ia+cqcb5cqiWnRGZOAN3HUgz05GNX5EFLBM=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","References":"<20201007112544.3169293-1-kieran.bingham@ideasonboard.com>\n\t<20201007112544.3169293-2-kieran.bingham@ideasonboard.com>\n\t<20201007120301.vsxgffg7vae2gbkh@oden.dyn.berto.se>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<7a808811-f225-39f1-2255-ffd0c157de4e@ideasonboard.com>","Date":"Wed, 7 Oct 2020 13:26:40 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201007120301.vsxgffg7vae2gbkh@oden.dyn.berto.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 1/1] test: cam tool testing with\n\tvalgrind","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13061,"web_url":"https://patchwork.libcamera.org/comment/13061/","msgid":"<20201007131023.GC3937@pendragon.ideasonboard.com>","date":"2020-10-07T13:10:23","subject":"Re: [libcamera-devel] [RFC PATCH 1/1] test: cam tool testing with\n\tvalgrind","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Oct 07, 2020 at 01:26:40PM +0100, Kieran Bingham wrote:\n> On 07/10/2020 13:03, Niklas Söderlund wrote:\n> > On 2020-10-07 12:25:44 +0100, Kieran Bingham wrote:\n> >> Implement a test which runs cam to capture 10 frames,\n> >> and validates there are no memory leaks with valgrind.\n> >>\n> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >> ---\n> >>\n> >> A few points to consider here:\n> >>\n> >> Duration\n> >> --------\n> >> Unfortuantely this test adds an extra 13 seconds to the test run on my\n> >> system. We could reduce the duration of course, but it will still be a\n> >> reasonably substantial addition.\n> > \n> > I think this is indeed a good tool to have in our arsenal. I'm starting \n> > to wonder if we need to split our test directory in a 'release' and \n> > 'normal' mode. In release we would have tests that takes longer and are \n> > more focused to catch deeper errors such as memory leaks or other issues \n> > that requires a longer runtime. While in 'normal' we would have tests \n> > similar that we have today that focus on incorrect behavior of the API.  \n> > What do you think?\n> > \n> > I think I would be unhappy for this test to run (without any way to \n> > disable it) for every ninja test run as I already think our tests takes \n> > too much time ;-)\n> \n> I'm fine with that, a 'smoke-test' suite, and a full suite. I'll leave\n> the definition of what goes it the smoke-test to later, after we\n> determine if it's possible. But I certainly don't object ;-)\n\nDeciding which category a test will go to will be an interesting issue.\n\n> >> TAP\n> >> ---\n> >> This runs a test for mulitiple cameras, and meson now supports 'tap'\n> >> format tests (multiple tests in one execution). In this patch - the\n> >> 'protocol : \"tap\" is disabled, because meson doesn't like the output of\n> >> the rest of the test being mixed with the tap output. I'm not sure of a\n> >> good way to handle that. The output is useful, so we want it somewhere,\n> >> and ideally in the test log - but we also want the tap parsing to work\n> >> in that case too.\n> >> Anyway, with the tap protocol disabled, it just falls back to the return\n> >> status which is also handled here. Still, this is a good opportunity to\n> >> see how 'tap' could help our tests, and might revive my old patches to\n> >> use common test helpers across the rest of test/\n\nI'd certainly be happy to see improvements in our test infrastructure,\nincluding better logging support that could help with TAP support in a\nway that would be transparent for individual tests. We're not doing that\nbad on the unit test front, but we're slowly accumulated technical debt\nwhich would be very nice to see handled.\n\n> > I'm a huge fan of TAP, do you think it would require a lot of work to \n> > change our current tests output to be valid TAP? If I recall correctly \n> > prepending a # too all non-TAP lines is nought to make any text stream \n> > into a valid (yet not so useful) TAP stream.\n> \n> Ok - prepending sounds like a good route then. Maybe we can throw\n> something together that automatically redirects stdout/stderr to two\n> different prefixes.\n> \n> 1...3\n> #out: a standard out message\n> #err: A standard error mesage/warning\n> not ok - 1 There was an error in this test.\n\nThere's also the option to redirecting the ancillary output to a log\nfile.\n\n> > One thing about TAP I learnt the hard way is that the number of planed \n> > tests (keyword \"1..5\") does not need to go first in the TAP stream, it's \n> > perfectly valid to provide this last. The results should only be \n> \n> Yes, I discovered this recently too. That helps a lot when you don't\n> know how many tests you are going to run until you run them ;-)\n> \n> > considered incomplete if there is no plan provided anywhere. This have \n> > helped me in the past turning things into being TAP compatible.\n> > \n> > Lastly a word of caution of the TAP13 specification, please don't :-)\n> \n> Ack. I haven't looked at TAP13. I'll avoid it ;-)\n> \n> >> Valgrind\n> >> --------\n> >> Parsing valgrind is a real pain. This test *only* checks to see if there\n> >> are any memory leaks, and does not report on any of the other topics\n> >> valgirind might highlight. (They could be added later of course).\n> >> Ideally we would want a machine parsable output from valgrind. It can\n> >> output an xml report ... but ... that's a lot more parsing too ;-)\n> >>\n> >> Thoughts/Suggestions?\n> > \n> > Is it really useful information to parse this out? As long as the \n> > valgrind output is keept in a log file I think a PASS/FAIL of the run is \n> > enough as someone who debugs it would like find more informaiton in the \n> > log then from parsed values presented in the test run.\n> \n> The issue is - It's hard to (easily/simply) identify if there is a\n> reason to consider valgrind a pass/fail.\n> \n> grepping the logs seems fragile and awkward --- It would be nice if\n> valgrind had a better summary that said:\n>   - leaks: 5\n>   - uninitialsed data: 2\n>   - Total errors: 7\n> \n> Or something really easy to consider a pass/fail.\n> \n> For example, without Tomi's current fixup patches, this test 'passes'\n> even though valgrind has (correctly) identified and reported the use of\n> uninitialised variables, and an invalid close(-1) syscall.\n> \n> It would be nice if (easily) we could report all valgrind errors here.\n> It feels too obvious - so I must have missed something staring me in the\n> face :-(\n\nI don't have the answer to that problem, but I also feel there should be\nsomething :-)\n\n> >> https://github.com/adobe/chromium/blob/master/tools/valgrind/memcheck_analyze.py\n> >> seems to show an existing tool that parses the valgrind output as one\n> >> possible path - but I'm happy to hear of any better ideas too.\n> > \n> > As a last point I wonder if we should create a new binary in tests that \n> > iterates over all cameras while using the same CameraManager instance \n> > and looper over them 2-3 times. I think that could exercise more code \n> > paths then creating a new CamereManger for each run (as we do with cam).  \n> > But this can of course be done on top or maybe even in addition.\n> \n> Yes, that sounds like another internal test to add.\n> \n> Meson does have the facilility to wrap everthing with valgrind too ...\n> so perhaps we should also consider that - but I thought this route might\n> be interesting too.\n> \n> $ meson test --wrap='valgrind --tool=helgrind -v --error-exitcode=1'\n> \n> Ok - so I grabbed that from a search - and look at that\n> --error-exitcode=1 ! That looks like what I might have been missing ;-)\n> \n> \n> Well well well ... so I've run meson test --wrap=valgrind and I get:\n> \n> $ meson test --wrap=valgrind\n> 14/57 libcamera:ipc / unixsocket                       FAIL\n> 16/57 libcamera:log / log_process                      FAIL\n> 22/57 libcamera:process / process_test                 FAIL\n> 55/57 libcamera / timer                                FAIL\n> \n> (trimmed of course)\n> \n> and worse:\n> \n> meson test --wrap='valgrind --tool=helgrind -v --error-exitcode=1'\n> <trim all the failures>\n>  1/57 libcamera:integration / cam-capture-test         OK\n> 38/57 libcamera / span                                 OK\n> \n> Ok:                 2\n> Expected Fail:      0\n> Fail:               55\n> Unexpected Pass:    0\n> Skipped:            0\n> Timeout:            0\n> \n> It seems that might be potentially due to a common failure in the\n> CameraTest::CameraTest though.\n> \n> Time for some digging I guess...\n\nWe have leaks flagged for instance due to one-time initialization inside\nlibudev that has no cleanup API. We will need a valgrind suppression\nfile if we want to be able to wrap all tests with valgrind.\n\n> >>  test/cam/capture-test.sh | 71 ++++++++++++++++++++++++++++++++++++++++\n> >>  test/cam/meson.build     | 10 ++++++\n> >>  test/meson.build         |  1 +\n> >>  3 files changed, 82 insertions(+)\n> >>  create mode 100755 test/cam/capture-test.sh\n> >>  create mode 100644 test/cam/meson.build\n> >>\n> >> diff --git a/test/cam/capture-test.sh b/test/cam/capture-test.sh\n> >> new file mode 100755\n> >> index 000000000000..ab808976be72\n> >> --- /dev/null\n> >> +++ b/test/cam/capture-test.sh\n> >> @@ -0,0 +1,71 @@\n> >> +#!/bin/sh\n> >> +\n> >> +# SPDX-License-Identifier: GPL-2.0-or-later\n> >> +\n> >> +TestPass=0\n> >> +TestFail=255\n> >> +TestSkip=77\n> >> +\n> >> +# Initialise success, set for failure.\n> >> +ret=$TestPass\n> >> +\n> >> +ok() {\n> >> +\techo \"ok $*\"\n> >> +}\n> >> +\n> >> +nok() {\n> >> +\techo \"not ok $*\"\n> >> +\tret=$TestFail\n> >> +}\n> >> +\n> >> +valgrind=$(command -v valgrind)\n> >> +\n> >> +if [ x\"\" = x\"$valgrind\" ] ; then\n> >> +\techo \"skip 1 - Valgrind unavailable ...\"\n> >> +\texit $TestSkip\n> >> +fi\n> >> +\n> >> +# Tests expect to be run from the meson.project_build_root()\n> >> +cam=${1:-src/cam/cam}\n> >> +\n> >> +if [ ! -e \"$cam\" ] ; then\n> >> +\tnok \"1 - failed to find cam utility.\"\n> >> +\texit $TestFail\n> >> +fi\n> >> +\n> >> +# Unfortunately, we don't have a 'machine interface', so we rely on parsing the\n> >> +# output of cam...\n> >> +num_cameras=$(\"$cam\" -l | grep -v \"Available\" | wc -l)\n> >> +\n> >> +# Enter TAP plan\n> >> +echo \"1..$num_cameras\"\n> >> +\n> >> +for i in $(seq 1 1 \"$num_cameras\");\n> >> +do\n> >> +\t\"$cam\" -c \"$i\" -C10\n> >> +\tret=$?\n> >> +\tif [ $ret != 0 ] ; then\n> >> +\t\tnok \"$i - $cam returned $ret\"\n> >> +\t\tcontinue\n> >> +\tfi\n> >> +\n> >> +\tlog_file=\"valgrind-cam-$i.log\"\n> >> +\t\"$valgrind\" \"$cam\" -c \"$i\" -C10 > \"$log_file\" 2>&1\n> >> +\tret=$?\n> >> +\tif [ $ret != 0 ] ; then\n> >> +\t\tnok \"$i - $valgrind returned $ret\"\n> >> +\t\tcontinue\n> >> +\tfi\n> >> +\n> >> +\t# I'd prefer a better way of checking there are no leaks, as well as reporting\n> >> +\t# the different categories from valgrind as distinct tests.\n> >> +\tif ! grep \"no leaks are possible\" \"$log_file\" > /dev/null; then\n> >> +\t\tnok \"$i - Valgrind Errors detected\"\n> >> +\t\tcat $log_file > /dev/stderr\n> >> +\t\tcontinue\n> >> +\tfi\n> >> +\n> >> +\tok \"$i - Camera $i reports no leaks\"\n> >> +done;\n> >> +\n> >> +exit $ret\n> >> diff --git a/test/cam/meson.build b/test/cam/meson.build\n> >> new file mode 100644\n> >> index 000000000000..834c9bcf6b86\n> >> --- /dev/null\n> >> +++ b/test/cam/meson.build\n> >> @@ -0,0 +1,10 @@\n> >> +# SPDX-License-Identifier: CC0-1.0\n> >> +\n> >> +cam_capture = files('capture-test.sh')\n> >> +\n> >> +test('cam-capture-test', cam_capture,\n> >> +    args : cam,\n> >> +    suite : 'integration',\n> >> +    is_parallel : false,\n> >> +    #protocol : 'tap',\n> >> +    timeout : 60)\n> >> diff --git a/test/meson.build b/test/meson.build\n> >> index 0a1d434e3996..d1b24220dc7c 100644\n> >> --- a/test/meson.build\n> >> +++ b/test/meson.build\n> >> @@ -2,6 +2,7 @@\n> >>  \n> >>  subdir('libtest')\n> >>  \n> >> +subdir('cam')\n> >>  subdir('camera')\n> >>  subdir('controls')\n> >>  subdir('ipa')","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 74F6EBEEE0\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Oct 2020 13:11:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC35760580;\n\tWed,  7 Oct 2020 15:11:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A9F5C6055E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Oct 2020 15:11:05 +0200 (CEST)","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 1FBB89DA;\n\tWed,  7 Oct 2020 15:11:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Uv9jbmoX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1602076265;\n\tbh=lj2VinI/56aCRClhU4D8fF/X1dpr2aH4ZfdxHeLWIYU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Uv9jbmoXhHAtrIAdTz0zjJHyh0TQyKlcGfZahlGyoC3P8Jn5jRWymIHAPgw2k9M//\n\tfeYczlmrvbPU58E+xdyEQN5vTxob+qgXucSymO/xNawSsp7HdTE5UueZ9ex9eJ6wOS\n\tGE37/N8lM1/sOv42/gwrbgqk2etnabtW1Whgfd+A=","Date":"Wed, 7 Oct 2020 16:10:23 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20201007131023.GC3937@pendragon.ideasonboard.com>","References":"<20201007112544.3169293-1-kieran.bingham@ideasonboard.com>\n\t<20201007112544.3169293-2-kieran.bingham@ideasonboard.com>\n\t<20201007120301.vsxgffg7vae2gbkh@oden.dyn.berto.se>\n\t<7a808811-f225-39f1-2255-ffd0c157de4e@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<7a808811-f225-39f1-2255-ffd0c157de4e@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 1/1] test: cam tool testing with\n\tvalgrind","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]