[{"id":1978,"web_url":"https://patchwork.libcamera.org/comment/1978/","msgid":"<20190623153957.GF32581@bigcity.dyn.berto.se>","date":"2019-06-23T15:39:57","subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nThanks for your work.\n\nOn 2019-06-21 17:13:59 +0100, Kieran Bingham wrote:\n> Extend the Controls tests with specific testing of the ControlList\n> infrastructure and public APIs.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> test/controls.cpp | 99 +++++++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 99 insertions(+)\n> \n> diff --git a/test/controls.cpp b/test/controls.cpp\n> index 94e89f270108..c4145b7a0543 100644\n> --- a/test/controls.cpp\n> +++ b/test/controls.cpp\n> @@ -25,6 +25,101 @@ protected:\n> \t\treturn TestPass;\n> \t}\n> \n> +\tint testControlList()\n> +\t{\n> +\t\tControlList list;\n> +\n> +\t\tif (list.contains(ManualGain)) {\n> +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n\nIf this error was returned I would not know what went wrong without \nlooking at the source ;-)\n\nThis is a test case so I think it's fine,\n\nReviewed-by: Niklas Söderlund \n\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (list.size() != 0) {\n> +\t\t\tcout << \"List should contain zero elements\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (!list.empty()) {\n> +\t\t\tcout << \"List expected to be empty\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Set a control */\n> +\t\tlist[ManualBrightness] = 255;\n> +\n> +\t\t/* Contains should still not find a non set control */\n> +\t\tif (list.contains(ManualGain)) {\n> +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (list.size() != 1) {\n> +\t\t\tcout << \"List should contain one element\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (list.empty()) {\n> +\t\t\tcout << \"List not expected to be empty\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Finally lets find that Gain */\n> +\t\tlist[ManualGain] = 128;\n> +\t\tif (!list.contains(ManualGain)) {\n> +\t\t\tcout << \"Couldn't identify an in-list control\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Validate value retrieval */\n> +\t\tif (list[ManualGain].getInt() != 128 ||\n> +\t\t list[ManualBrightness].getInt() != 255) {\n> +\t\t\tcout << \"Failed to retrieve control value\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Update using ControlInfo */\n> +\t\tControlInfo gainInfo(ManualGain);\n> +\t\tlist.update(gainInfo, 200);\n> +\t\tif (list[ManualGain].getInt() != 200) {\n> +\t\t\tcout << \"Failed to update control value\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\t/* Create a new list from an existing one */\n> +\t\tControlList newList;\n> +\n> +\t\tnewList.update(list);\n> +\n> +\t\t/*\n> +\t\t * Clear down the original list and assert items are still in\n> +\t\t * the new list\n> +\t\t */\n> +\t\tlist.reset();\n> +\n> +\t\tif (list.size() != 0) {\n> +\t\t\tcout << \"Old List should contain zero elements\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (!list.empty()) {\n> +\t\t\tcout << \"Old List expected to be empty\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (newList.size() != 2) {\n> +\t\t\tcout << \"New list with incorrect size\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tif (!(newList.contains(ManualGain) &&\n> +\t\t newList.contains(ManualBrightness))) {\n> +\t\t\tcout << \"New list with incorrect items\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> \tint run()\n> \t{\n> \t\tint ret;\n> @@ -33,6 +128,10 @@ protected:\n> \t\tif (ret)\n> \t\t\treturn ret;\n> \n> +\t\tret = testControlList();\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\n> \t\treturn TestPass;\n> \t}\n> };\n> -- \n> 2.20.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":"","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 21A046157E\n\tfor ;\n\tSun, 23 Jun 2019 17:40:00 +0200 (CEST)","by mail-lj1-x244.google.com with SMTP id h10so10243313ljg.0\n\tfor ;\n\tSun, 23 Jun 2019 08:40:00 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\ta17sm1349080lfk.0.2019.06.23.08.39.58\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tSun, 23 Jun 2019 08:39:58 -0700 (PDT)"],"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\t:user-agent; bh=JR1+5n409bEloV6OZVSk5lv9LiYDwJqNOou7qEBvaow=;\n\tb=ZOuEdkR0CqOsN6uLW6UzUr2+Cg7b6Gg9LIQxK+Zz7RQLKTuFKAJ5sUpOWr4IBd2Gh/\n\tSJLsU5/facw+O3Lx8j/k5Tn9tuR9OltMR+sHVTyz8juFWd23D1Mmd2R9SVK+qPo37jSm\n\t02or06Ts5wUwre/cE2YWze3rqO+FJmQa82bAqDs7LEq7N6/7UTx+t+HI2cRSrJZsK1A3\n\tKSqy98/mfWRfkKbW57ChbtPxkpDFQ7Rtm/4tvNnHVOevfqah5+iz3mkQTG3OcJ0DJy2u\n\tWvmBkud9HbYUtd4shvadU249+hq/l6wVtilISE9Z5aWNZ5LXnj9poauqQP56x78VgjFM\n\t0KVw==","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:user-agent;\n\tbh=JR1+5n409bEloV6OZVSk5lv9LiYDwJqNOou7qEBvaow=;\n\tb=XNAIs3Q7LP3SLkt0hyyWq94LtfT8BaGoZQYgSL76d9utcZNLbrlcLN+ABXC4YTzBkq\n\twwlP/PLsGpApQINJQTm+aIpZbRwrldKpkDzUw0nfkTcqxNurodNItB4x6R0v0r+p3MM6\n\twCCXqk9/BZpptG7vxJugRwsIlSeuHBgfDOT5CNDLWZ+BRoGaAMuPHSslKZLNBPECL2De\n\tYKdUU1GigT58YdUZxnhidD28KAwHaRrt32NJ04PIVHh0186thFxZAHC5r+Fm1SVMeeFU\n\tAnt+r+qjpXCqZmCXdDXAYE0YwOG63NfKtJWTkJPtiXfXTOjgWdgmuqziDWuCWeltcy4P\n\tq4ng==","X-Gm-Message-State":"APjAAAWAxh9nOvVM8PP9LJTSIo0uWLjt3UqfJxk/nwk06a9J0SmthJMw\n\t1V/s42p3Q9mgZUpjNpQrRA7q01wRIAc=","X-Google-Smtp-Source":"APXvYqx1lR0JTl9NznvRhGrL0wgczYV2EACYhbn4OTDg0hXWOVMZ6nXydgFxgMVGJ736FR+4z5GObg==","X-Received":"by 2002:a2e:8583:: with SMTP id\n\tb3mr60664974lji.171.1561304399621; \n\tSun, 23 Jun 2019 08:39:59 -0700 (PDT)","Date":"Sun, 23 Jun 2019 17:39:57 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kieran Bingham ","Cc":"LibCamera Devel ","Message-ID":"<20190623153957.GF32581@bigcity.dyn.berto.se>","References":"<20190621161401.28337-1-kieran.bingham@ideasonboard.com>\n\t<20190621161401.28337-8-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190621161401.28337-8-kieran.bingham@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 23 Jun 2019 15:40:00 -0000"}},{"id":1984,"web_url":"https://patchwork.libcamera.org/comment/1984/","msgid":"<20190623224403.GG6124@pendragon.ideasonboard.com>","date":"2019-06-23T22:44:03","subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThanks for your work.\n\nOn Sun, Jun 23, 2019 at 05:39:57PM +0200, Niklas Söderlund wrote:\n> On 2019-06-21 17:13:59 +0100, Kieran Bingham wrote:\n> > Extend the Controls tests with specific testing of the ControlList\n> > infrastructure and public APIs.\n> > \n> > Signed-off-by: Kieran Bingham \n> > ---\n> > test/controls.cpp | 99 +++++++++++++++++++++++++++++++++++++++++++++++\n> > 1 file changed, 99 insertions(+)\n> > \n> > diff --git a/test/controls.cpp b/test/controls.cpp\n> > index 94e89f270108..c4145b7a0543 100644\n> > --- a/test/controls.cpp\n> > +++ b/test/controls.cpp\n> > @@ -25,6 +25,101 @@ protected:\n> > \t\treturn TestPass;\n> > \t}\n> > \n> > +\tint testControlList()\n> > +\t{\n> > +\t\tControlList list;\n> > +\n> > +\t\tif (list.contains(ManualGain)) {\n> > +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n> \n> If this error was returned I would not know what went wrong without \n> looking at the source ;-)\n> \n> This is a test case so I think it's fine,\n> \n> Reviewed-by: Niklas Söderlund \n> \n> > +\t\t\treturn TestFail;\n> > +\t\t}\n\nIsn't this a bit too random ? :-) I don't really see how this could be\nthe case, and I don't really see what value this test brings. Same for\nmost of the ones below. I think we should try to write test that focus\non use cases, on expected problems, and on API contracts. Sure, a\nControlList being empty after construction is an API contract, but if\nyou go there you can as well test that it contains none of the defined\ncontrols. Or that calling size() twice in a row will return the same\nvalue.\n\nAn example of a potentially useful test, in the current implementation,\nwould be that the has function operates correctly on the id() contained\nin the ControlInfo. It would insert two items with different\nControlInfo that have the same ID, and verify that the second one\nreplaces the first one (both for the key and the value).\n\nIn other words, let's not go too trivial.\n\nAnd another very useful test would operate at the request level with the\nVIMC pipeline handler, and verify that controls are correctly applied.\nThat's more work though :-)\n\n> > +\n> > +\t\tif (list.size() != 0) {\n> > +\t\t\tcout << \"List should contain zero elements\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (!list.empty()) {\n> > +\t\t\tcout << \"List expected to be empty\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Set a control */\n> > +\t\tlist[ManualBrightness] = 255;\n> > +\n> > +\t\t/* Contains should still not find a non set control */\n> > +\t\tif (list.contains(ManualGain)) {\n> > +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (list.size() != 1) {\n> > +\t\t\tcout << \"List should contain one element\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (list.empty()) {\n> > +\t\t\tcout << \"List not expected to be empty\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Finally lets find that Gain */\n> > +\t\tlist[ManualGain] = 128;\n> > +\t\tif (!list.contains(ManualGain)) {\n> > +\t\t\tcout << \"Couldn't identify an in-list control\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Validate value retrieval */\n> > +\t\tif (list[ManualGain].getInt() != 128 ||\n> > +\t\t list[ManualBrightness].getInt() != 255) {\n> > +\t\t\tcout << \"Failed to retrieve control value\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Update using ControlInfo */\n> > +\t\tControlInfo gainInfo(ManualGain);\n> > +\t\tlist.update(gainInfo, 200);\n> > +\t\tif (list[ManualGain].getInt() != 200) {\n> > +\t\t\tcout << \"Failed to update control value\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\t/* Create a new list from an existing one */\n> > +\t\tControlList newList;\n> > +\n> > +\t\tnewList.update(list);\n> > +\n> > +\t\t/*\n> > +\t\t * Clear down the original list and assert items are still in\n> > +\t\t * the new list\n> > +\t\t */\n> > +\t\tlist.reset();\n> > +\n> > +\t\tif (list.size() != 0) {\n> > +\t\t\tcout << \"Old List should contain zero elements\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (!list.empty()) {\n> > +\t\t\tcout << \"Old List expected to be empty\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (newList.size() != 2) {\n> > +\t\t\tcout << \"New list with incorrect size\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\tif (!(newList.contains(ManualGain) &&\n> > +\t\t newList.contains(ManualBrightness))) {\n> > +\t\t\tcout << \"New list with incorrect items\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > \tint run()\n> > \t{\n> > \t\tint ret;\n> > @@ -33,6 +128,10 @@ protected:\n> > \t\tif (ret)\n> > \t\t\treturn ret;\n> > \n> > +\t\tret = testControlList();\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\n> > \t\treturn TestPass;\n> > \t}\n> > };","headers":{"Return-Path":"","Received":["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 7FAA16157E\n\tfor ;\n\tMon, 24 Jun 2019 00:46:33 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EF750500;\n\tMon, 24 Jun 2019 00:46:32 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561329993;\n\tbh=CaPtqeqH4JWE5kJuMLX48UqyAR0Epmk9jK6UAnnZEX0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PNHPSwFS+l07JPer2KAyGBpwXjNu9xPv/SjJgcjfr3WgpuMQDIF7FCnfxqvI9wRz/\n\t9/BWz6FrJhP+eylizRGUgkoad9s8tF1jIaHQX2PKIjPntecTptr71CULVJibF1WnEA\n\tQvKAH/V3uqNxEVgZE7yitmldOoAZry5CeYYJOsdc=","Date":"Mon, 24 Jun 2019 01:44:03 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Niklas =?utf-8?q?S=C3=B6derlund?= ,\n\tLibCamera Devel ","Message-ID":"<20190623224403.GG6124@pendragon.ideasonboard.com>","References":"<20190621161401.28337-1-kieran.bingham@ideasonboard.com>\n\t<20190621161401.28337-8-kieran.bingham@ideasonboard.com>\n\t<20190623153957.GF32581@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190623153957.GF32581@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 23 Jun 2019 22:46:33 -0000"}},{"id":2001,"web_url":"https://patchwork.libcamera.org/comment/2001/","msgid":"","date":"2019-06-24T12:16:04","subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 23/06/2019 23:44, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thanks for your work.\n> \n> On Sun, Jun 23, 2019 at 05:39:57PM +0200, Niklas Söderlund wrote:\n>> On 2019-06-21 17:13:59 +0100, Kieran Bingham wrote:\n>>> Extend the Controls tests with specific testing of the ControlList\n>>> infrastructure and public APIs.\n>>>\n>>> Signed-off-by: Kieran Bingham \n>>> ---\n>>> test/controls.cpp | 99 +++++++++++++++++++++++++++++++++++++++++++++++\n>>> 1 file changed, 99 insertions(+)\n>>>\n>>> diff --git a/test/controls.cpp b/test/controls.cpp\n>>> index 94e89f270108..c4145b7a0543 100644\n>>> --- a/test/controls.cpp\n>>> +++ b/test/controls.cpp\n>>> @@ -25,6 +25,101 @@ protected:\n>>> \t\treturn TestPass;\n>>> \t}\n>>> \n>>> +\tint testControlList()\n>>> +\t{\n>>> +\t\tControlList list;\n>>> +\n>>> +\t\tif (list.contains(ManualGain)) {\n>>> +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n>>\n>> If this error was returned I would not know what went wrong without \n>> looking at the source ;-)\n>>\n>> This is a test case so I think it's fine,\n>>\n>> Reviewed-by: Niklas Söderlund \n>>\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n> \n> Isn't this a bit too random ? :-) I don't really see how this could be\n> the case, and I don't really see what value this test brings. Same for\n> most of the ones below. I think we should try to write test that focus\n\nI put in the tongue in cheek \"Unexpected item in the bagging area\"\nbecause I have no expectation of this firing - unless someone starts\nscrewing about with the internals of the ControlList.\n\nBut that doesn't mean I don't think it has a value in the test-suite. At\nthat point - if they are doing that - they are already looking at the\nappropriate code - This message just states that someone has broken the\ninternals. It should be noticed immediately by that person (or they\naren't running the tests)\n\n\n> on use cases, on expected problems, and on API contracts. Sure, a\n> ControlList being empty after construction is an API contract, but if\n> you go there you can as well test that it contains none of the defined\n> controls. Or that calling size() twice in a row will return the same\n> value.\n\n\nI'm trying to exercise each exposed call from the *application public*\nAPI of the ControlList.\n\n\nShould we leave public API functions untested just because they are\n'trivial' (i.e. size, should return the size....)\n\n\nIf this was a libcamera internal object I would be more likely to agree\nthat the tests are overkill. But this is an interface which applications\nwill be able to use.\n\n\n> An example of a potentially useful test, in the current implementation,\n> would be that the has function operates correctly on the id() contained\n> in the ControlInfo. It would insert two items with different\n> ControlInfo that have the same ID, and verify that the second one\n> replaces the first one (both for the key and the value).\n\n\nso I've now added this (psuedocode):\n\nControlInfo gainInfo(ManualGain);\nControlInfo secondGainInfo(ManualGain);\n\nlistSize = list.size();\nlist.update(gainInfo, 200);\n\tassert(list[ManualGain] == 200);\nlist.update(secondGainInfo, 201);\n\tassert(list[ManualGain] == 201);\nassert(list.size()==listSize);\n\n\n> In other words, let's not go too trivial.\n\nControlList is a container. And more (*much more*) importantly, this is\na container exposed to libcamera applications.\n\nThis is testing/enforcing/guaranteeing that container API.\n\nShould someone shift the underlying storage mechanism, I would expect\nthese calls to still function. That's the simple point of the unit tests\nright?\n\n\nHow else should we enforce the API contract designs if not through\nadding tests?\n\n\n> And another very useful test would operate at the request level with the\n> VIMC pipeline handler, and verify that controls are correctly applied.\n> That's more work though :-)\n\nSure - but that's not testing the ControlList, that's testing Controls\nand V4L2Controls.\n\nUseful to add - but these were to validate and verify the function of\nthe container object.\n\nThis test-set is \"testControlList\".\n\nTesting the controls themselves is a different layer and while they\nmight be in this 'controls.cpp file', they would be in a function called\ntestControls().\n\nTesting Controls require the controls to be clearly defined, and have an\nimplementation for managing the controls in the VIMC (or required)\npipeline handler.\n\nIMO Those should be added, but once the pipeline handlers are updated to\nprovide that required functionality.\n\n\n\n>>> +\n>>> +\t\tif (list.size() != 0) {\n>>> +\t\t\tcout << \"List should contain zero elements\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\tif (!list.empty()) {\n>>> +\t\t\tcout << \"List expected to be empty\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\t/* Set a control */\n>>> +\t\tlist[ManualBrightness] = 255;\n>>> +\n>>> +\t\t/* Contains should still not find a non set control */\n>>> +\t\tif (list.contains(ManualGain)) {\n>>> +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\tif (list.size() != 1) {\n>>> +\t\t\tcout << \"List should contain one element\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\tif (list.empty()) {\n>>> +\t\t\tcout << \"List not expected to be empty\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\t/* Finally lets find that Gain */\n>>> +\t\tlist[ManualGain] = 128;\n>>> +\t\tif (!list.contains(ManualGain)) {\n>>> +\t\t\tcout << \"Couldn't identify an in-list control\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\t/* Validate value retrieval */\n>>> +\t\tif (list[ManualGain].getInt() != 128 ||\n>>> +\t\t list[ManualBrightness].getInt() != 255) {\n>>> +\t\t\tcout << \"Failed to retrieve control value\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\t/* Update using ControlInfo */\n>>> +\t\tControlInfo gainInfo(ManualGain);\n>>> +\t\tlist.update(gainInfo, 200);\n>>> +\t\tif (list[ManualGain].getInt() != 200) {\n>>> +\t\t\tcout << \"Failed to update control value\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\t/* Create a new list from an existing one */\n>>> +\t\tControlList newList;\n>>> +\n>>> +\t\tnewList.update(list);\n>>> +\n>>> +\t\t/*\n>>> +\t\t * Clear down the original list and assert items are still in\n>>> +\t\t * the new list\n>>> +\t\t */\n>>> +\t\tlist.reset();\n>>> +\n>>> +\t\tif (list.size() != 0) {\n>>> +\t\t\tcout << \"Old List should contain zero elements\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\tif (!list.empty()) {\n>>> +\t\t\tcout << \"Old List expected to be empty\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\tif (newList.size() != 2) {\n>>> +\t\t\tcout << \"New list with incorrect size\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\tif (!(newList.contains(ManualGain) &&\n>>> +\t\t newList.contains(ManualBrightness))) {\n>>> +\t\t\tcout << \"New list with incorrect items\" << endl;\n>>> +\t\t\treturn TestFail;\n>>> +\t\t}\n>>> +\n>>> +\t\treturn TestPass;\n>>> +\t}\n>>> +\n>>> \tint run()\n>>> \t{\n>>> \t\tint ret;\n>>> @@ -33,6 +128,10 @@ protected:\n>>> \t\tif (ret)\n>>> \t\t\treturn ret;\n>>> \n>>> +\t\tret = testControlList();\n>>> +\t\tif (ret)\n>>> +\t\t\treturn ret;\n>>> +\n>>> \t\treturn TestPass;\n>>> \t}\n>>> };\n>","headers":{"Return-Path":"","Received":["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 D1081600EB\n\tfor ;\n\tMon, 24 Jun 2019 14:16:07 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 54AB7323;\n\tMon, 24 Jun 2019 14:16:07 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561378567;\n\tbh=9bTqLK/7QfZP8kV0NTyUw01E4uze9OUMsq70Djk0ovs=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=UcvrcWWP86RCxRvNAS0rv/omZkffpkL1LwneVTP2XyOmqh9RHE4eTUjvM4qsCi48l\n\tSNwqe9E1lkRxHRQx310lL3BwuSxizBUtzlywU24r0A9nGRVw3EIFDa35hlRKIEouCX\n\tMstk2cMEpaAIjpCDKCNZudokHfJMfCPOoboOrj8M=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ","Cc":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tLibCamera Devel ","References":"<20190621161401.28337-1-kieran.bingham@ideasonboard.com>\n\t<20190621161401.28337-8-kieran.bingham@ideasonboard.com>\n\t<20190623153957.GF32581@bigcity.dyn.berto.se>\n\t<20190623224403.GG6124@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"","Date":"Mon, 24 Jun 2019 13:16:04 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190623224403.GG6124@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 24 Jun 2019 12:16:08 -0000"}},{"id":2002,"web_url":"https://patchwork.libcamera.org/comment/2002/","msgid":"<20190624123626.GI5737@pendragon.ideasonboard.com>","date":"2019-06-24T12:36:26","subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Jun 24, 2019 at 01:16:04PM +0100, Kieran Bingham wrote:\n> On 23/06/2019 23:44, Laurent Pinchart wrote:\n> > On Sun, Jun 23, 2019 at 05:39:57PM +0200, Niklas Söderlund wrote:\n> >> On 2019-06-21 17:13:59 +0100, Kieran Bingham wrote:\n> >>> Extend the Controls tests with specific testing of the ControlList\n> >>> infrastructure and public APIs.\n> >>>\n> >>> Signed-off-by: Kieran Bingham \n> >>> ---\n> >>> test/controls.cpp | 99 +++++++++++++++++++++++++++++++++++++++++++++++\n> >>> 1 file changed, 99 insertions(+)\n> >>>\n> >>> diff --git a/test/controls.cpp b/test/controls.cpp\n> >>> index 94e89f270108..c4145b7a0543 100644\n> >>> --- a/test/controls.cpp\n> >>> +++ b/test/controls.cpp\n> >>> @@ -25,6 +25,101 @@ protected:\n> >>> \t\treturn TestPass;\n> >>> \t}\n> >>> \n> >>> +\tint testControlList()\n> >>> +\t{\n> >>> +\t\tControlList list;\n> >>> +\n> >>> +\t\tif (list.contains(ManualGain)) {\n> >>> +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n> >>\n> >> If this error was returned I would not know what went wrong without \n> >> looking at the source ;-)\n> >>\n> >> This is a test case so I think it's fine,\n> >>\n> >> Reviewed-by: Niklas Söderlund \n> >>\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> > \n> > Isn't this a bit too random ? :-) I don't really see how this could be\n> > the case, and I don't really see what value this test brings. Same for\n> > most of the ones below. I think we should try to write test that focus\n> \n> I put in the tongue in cheek \"Unexpected item in the bagging area\"\n> because I have no expectation of this firing - unless someone starts\n> screwing about with the internals of the ControlList.\n> \n> But that doesn't mean I don't think it has a value in the test-suite. At\n> that point - if they are doing that - they are already looking at the\n> appropriate code - This message just states that someone has broken the\n> internals. It should be noticed immediately by that person (or they\n> aren't running the tests)\n\nI'd say that if it gets that far it should even be noticed without the\ntest suite. We don't have a test checking if something has broken the\nfundamental laws of physics, it will be noticeable when planets start\ncrashing into each other :-)\n\n> > on use cases, on expected problems, and on API contracts. Sure, a\n> > ControlList being empty after construction is an API contract, but if\n> > you go there you can as well test that it contains none of the defined\n> > controls. Or that calling size() twice in a row will return the same\n> > value.\n> \n> I'm trying to exercise each exposed call from the *application public*\n> API of the ControlList.\n> \n> Should we leave public API functions untested just because they are\n> 'trivial' (i.e. size, should return the size....)\n> \n> If this was a libcamera internal object I would be more likely to agree\n> that the tests are overkill. But this is an interface which applications\n> will be able to use.\n\nWe shouldn't leave them untested, we should implement meaningful tests\n:-)\n\n> > An example of a potentially useful test, in the current implementation,\n> > would be that the has function operates correctly on the id() contained\n> > in the ControlInfo. It would insert two items with different\n> > ControlInfo that have the same ID, and verify that the second one\n> > replaces the first one (both for the key and the value).\n> \n> so I've now added this (psuedocode):\n> \n> ControlInfo gainInfo(ManualGain);\n> ControlInfo secondGainInfo(ManualGain);\n> \n> listSize = list.size();\n> list.update(gainInfo, 200);\n> \tassert(list[ManualGain] == 200);\n> list.update(secondGainInfo, 201);\n> \tassert(list[ManualGain] == 201);\n> assert(list.size()==listSize);\n\nAnd you've also tested the size() method :-)\n\nNote that list.update(gainInfo, 200) should add a control to the list,\nso the size will change.\n\n> > In other words, let's not go too trivial.\n> \n> ControlList is a container. And more (*much more*) importantly, this is\n> a container exposed to libcamera applications.\n> \n> This is testing/enforcing/guaranteeing that container API.\n> \n> Should someone shift the underlying storage mechanism, I would expect\n> these calls to still function. That's the simple point of the unit tests\n> right?\n> \n> How else should we enforce the API contract designs if not through\n> adding tests?\n\nAgain, this isn't about not adding tests, it's about adding meaningful\ntests :-)\n\nYou test the size() method by making sure that it returns 1 after adding\none element. What if someone modifies the storage mechanism and\nincorrectly returns !empty() for the size() implementation ? Your test\nwill succeed, but size() won't behave correctly with two elements.\nThere's always a knowledge of the implementation hardcoded in tests, as\neven if you tested for size() in the 0, 1 and 2 cases, it could still\nfail for other values.\n\nMy point is that the test shouldn't be random, but should meaningfully\ntest for problems we foresee (or problems we have experienced). In this\ncase I would write a test suite that essentially tests the underlying\ncontainers, I would concentrate on the added value of the ControlList\nclass.\n\nIf you want to test the underlying container then I think you'll have to\ntest it function by function, including the iterators.\n\n> > And another very useful test would operate at the request level with the\n> > VIMC pipeline handler, and verify that controls are correctly applied.\n> > That's more work though :-)\n> \n> Sure - but that's not testing the ControlList, that's testing Controls\n> and V4L2Controls.\n\nIt would still indirectly test the control list :-) But I agree that\nindirect tests don't necessarily provide the coverage we need.\n\n> Useful to add - but these were to validate and verify the function of\n> the container object.\n> \n> This test-set is \"testControlList\".\n> \n> Testing the controls themselves is a different layer and while they\n> might be in this 'controls.cpp file', they would be in a function called\n> testControls().\n> \n> Testing Controls require the controls to be clearly defined, and have an\n> implementation for managing the controls in the VIMC (or required)\n> pipeline handler.\n> \n> IMO Those should be added, but once the pipeline handlers are updated to\n> provide that required functionality.\n\nYes, that will be done in a separate step.\n\n> >>> +\n> >>> +\t\tif (list.size() != 0) {\n> >>> +\t\t\tcout << \"List should contain zero elements\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\tif (!list.empty()) {\n> >>> +\t\t\tcout << \"List expected to be empty\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\t/* Set a control */\n> >>> +\t\tlist[ManualBrightness] = 255;\n> >>> +\n> >>> +\t\t/* Contains should still not find a non set control */\n> >>> +\t\tif (list.contains(ManualGain)) {\n> >>> +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\tif (list.size() != 1) {\n> >>> +\t\t\tcout << \"List should contain one element\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\tif (list.empty()) {\n> >>> +\t\t\tcout << \"List not expected to be empty\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\t/* Finally lets find that Gain */\n> >>> +\t\tlist[ManualGain] = 128;\n> >>> +\t\tif (!list.contains(ManualGain)) {\n> >>> +\t\t\tcout << \"Couldn't identify an in-list control\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\t/* Validate value retrieval */\n> >>> +\t\tif (list[ManualGain].getInt() != 128 ||\n> >>> +\t\t list[ManualBrightness].getInt() != 255) {\n> >>> +\t\t\tcout << \"Failed to retrieve control value\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\t/* Update using ControlInfo */\n> >>> +\t\tControlInfo gainInfo(ManualGain);\n> >>> +\t\tlist.update(gainInfo, 200);\n> >>> +\t\tif (list[ManualGain].getInt() != 200) {\n> >>> +\t\t\tcout << \"Failed to update control value\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\t/* Create a new list from an existing one */\n> >>> +\t\tControlList newList;\n> >>> +\n> >>> +\t\tnewList.update(list);\n> >>> +\n> >>> +\t\t/*\n> >>> +\t\t * Clear down the original list and assert items are still in\n> >>> +\t\t * the new list\n> >>> +\t\t */\n> >>> +\t\tlist.reset();\n> >>> +\n> >>> +\t\tif (list.size() != 0) {\n> >>> +\t\t\tcout << \"Old List should contain zero elements\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\tif (!list.empty()) {\n> >>> +\t\t\tcout << \"Old List expected to be empty\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\tif (newList.size() != 2) {\n> >>> +\t\t\tcout << \"New list with incorrect size\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\tif (!(newList.contains(ManualGain) &&\n> >>> +\t\t newList.contains(ManualBrightness))) {\n> >>> +\t\t\tcout << \"New list with incorrect items\" << endl;\n> >>> +\t\t\treturn TestFail;\n> >>> +\t\t}\n> >>> +\n> >>> +\t\treturn TestPass;\n> >>> +\t}\n> >>> +\n> >>> \tint run()\n> >>> \t{\n> >>> \t\tint ret;\n> >>> @@ -33,6 +128,10 @@ protected:\n> >>> \t\tif (ret)\n> >>> \t\t\treturn ret;\n> >>> \n> >>> +\t\tret = testControlList();\n> >>> +\t\tif (ret)\n> >>> +\t\t\treturn ret;\n> >>> +\n> >>> \t\treturn TestPass;\n> >>> \t}\n> >>> };","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE220600EB\n\tfor ;\n\tMon, 24 Jun 2019 14:36:45 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2B08B323;\n\tMon, 24 Jun 2019 14:36:45 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561379805;\n\tbh=0XvrAxLhrpuwML81JdQPc9j3nVRwdpUzeX+jfpHwQQA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QDeKbaJ/2UrRYtZ7EY6rDBAXY4jtqF/hlnqpatRKvOhbQmq+jrKkWuHueBSsgNhvG\n\tvTOq34wEg/EZ7uJQD2c23+fDF/i6wudJIak9BwDI/zEs2C7py8YPpAN0o4uOOYEE38\n\tzN+gmYL28aHj1QQzAGamBWnhTLwcXy+A0RuWcJgk=","Date":"Mon, 24 Jun 2019 15:36:26 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Niklas =?utf-8?q?S=C3=B6derlund?= ,\n\tLibCamera Devel ","Message-ID":"<20190624123626.GI5737@pendragon.ideasonboard.com>","References":"<20190621161401.28337-1-kieran.bingham@ideasonboard.com>\n\t<20190621161401.28337-8-kieran.bingham@ideasonboard.com>\n\t<20190623153957.GF32581@bigcity.dyn.berto.se>\n\t<20190623224403.GG6124@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 24 Jun 2019 12:36:45 -0000"}},{"id":2017,"web_url":"https://patchwork.libcamera.org/comment/2017/","msgid":"<091a8b05-17bf-a10b-bb50-9b0125435fba@ideasonboard.com>","date":"2019-06-24T16:57:50","subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 24/06/2019 13:36, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Mon, Jun 24, 2019 at 01:16:04PM +0100, Kieran Bingham wrote:\n>> On 23/06/2019 23:44, Laurent Pinchart wrote:\n>>> On Sun, Jun 23, 2019 at 05:39:57PM +0200, Niklas Söderlund wrote:\n>>>> On 2019-06-21 17:13:59 +0100, Kieran Bingham wrote:\n>>>>> Extend the Controls tests with specific testing of the ControlList\n>>>>> infrastructure and public APIs.\n>>>>>\n>>>>> Signed-off-by: Kieran Bingham \n>>>>> ---\n>>>>> test/controls.cpp | 99 +++++++++++++++++++++++++++++++++++++++++++++++\n>>>>> 1 file changed, 99 insertions(+)\n>>>>>\n>>>>> diff --git a/test/controls.cpp b/test/controls.cpp\n>>>>> index 94e89f270108..c4145b7a0543 100644\n>>>>> --- a/test/controls.cpp\n>>>>> +++ b/test/controls.cpp\n>>>>> @@ -25,6 +25,101 @@ protected:\n>>>>> \t\treturn TestPass;\n>>>>> \t}\n>>>>> \n>>>>> +\tint testControlList()\n>>>>> +\t{\n>>>>> +\t\tControlList list;\n>>>>> +\n>>>>> +\t\tif (list.contains(ManualGain)) {\n>>>>> +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n>>>>\n>>>> If this error was returned I would not know what went wrong without \n>>>> looking at the source ;-)\n>>>>\n>>>> This is a test case so I think it's fine,\n>>>>\n>>>> Reviewed-by: Niklas Söderlund \n>>>>\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>\n>>> Isn't this a bit too random ? :-) I don't really see how this could be\n>>> the case, and I don't really see what value this test brings. Same for\n>>> most of the ones below. I think we should try to write test that focus\n>>\n>> I put in the tongue in cheek \"Unexpected item in the bagging area\"\n>> because I have no expectation of this firing - unless someone starts\n>> screwing about with the internals of the ControlList.\n\nLike by changing the class to a simple using-statement. ... I wonder if\nthese tests will all still work when I do :-)\n\n\n\n>>\n>> But that doesn't mean I don't think it has a value in the test-suite. At\n>> that point - if they are doing that - they are already looking at the\n>> appropriate code - This message just states that someone has broken the\n>> internals. It should be noticed immediately by that person (or they\n>> aren't running the tests)\n> \n> I'd say that if it gets that far it should even be noticed without the\n> test suite. We don't have a test checking if something has broken the\n> fundamental laws of physics, it will be noticeable when planets start\n> crashing into each other :-)\n> \n>>> on use cases, on expected problems, and on API contracts. Sure, a\n>>> ControlList being empty after construction is an API contract, but if\n>>> you go there you can as well test that it contains none of the defined\n>>> controls. Or that calling size() twice in a row will return the same\n>>> value.\n>>\n>> I'm trying to exercise each exposed call from the *application public*\n>> API of the ControlList.\n>>\n>> Should we leave public API functions untested just because they are\n>> 'trivial' (i.e. size, should return the size....)\n>>\n>> If this was a libcamera internal object I would be more likely to agree\n>> that the tests are overkill. But this is an interface which applications\n>> will be able to use.\n> \n> We shouldn't leave them untested, we should implement meaningful tests\n> :-)\n> \n>>> An example of a potentially useful test, in the current implementation,\n>>> would be that the has function operates correctly on the id() contained\n>>> in the ControlInfo. It would insert two items with different\n>>> ControlInfo that have the same ID, and verify that the second one\n>>> replaces the first one (both for the key and the value).\n>>\n>> so I've now added this (psuedocode):\n>>\n>> ControlInfo gainInfo(ManualGain);\n>> ControlInfo secondGainInfo(ManualGain);\n>>\n>> listSize = list.size();\n>> list.update(gainInfo, 200);\n>> \tassert(list[ManualGain] == 200);\n>> list.update(secondGainInfo, 201);\n>> \tassert(list[ManualGain] == 201);\n>> assert(list.size()==listSize);\n> \n> And you've also tested the size() method :-)\n\nYes, and I (in the non-pseudo code) removed the basic size != 1 test,\nand replaced it with size == 2 after both controls are now added.\n\n> \n> Note that list.update(gainInfo, 200) should add a control to the list,\n> so the size will change.\n> \n\nYes, that was psuedo code to describe. Sorry it was incorrect, the\nimplementation is correct. (for what it actually does).\n\nAlternatively I could put:\n\n\n>> list.update(gainInfo, 200);\n>> \tassert(list[ManualGain] == 200);\n\n>> listSize = list.size();\n\n>> list.update(secondGainInfo, 201);\n>> \tassert(list[ManualGain] == 201);\n>> assert(list.size()==listSize);\n\nAnd actually test that it doesn't change, but I don't want to rely on\nthe return value of list.size() being the same twice - I have hardcoded\nthe return value I expect (2) to ensure that a faulty size() always\nreturning 0 doesn't cause an unexpected test pass.\n\n\n>>> In other words, let's not go too trivial.\n>>\n>> ControlList is a container. And more (*much more*) importantly, this is\n>> a container exposed to libcamera applications.\n>>\n>> This is testing/enforcing/guaranteeing that container API.\n>>\n>> Should someone shift the underlying storage mechanism, I would expect\n>> these calls to still function. That's the simple point of the unit tests\n>> right?\n>>\n>> How else should we enforce the API contract designs if not through\n>> adding tests?\n> \n> Again, this isn't about not adding tests, it's about adding meaningful\n> tests :-)\n> \n> You test the size() method by making sure that it returns 1 after adding\n> one element. What if someone modifies the storage mechanism and\n> incorrectly returns !empty() for the size() implementation ? Your test\n> will succeed, but size() won't behave correctly with two elements.\n> There's always a knowledge of the implementation hardcoded in tests, as\n> even if you tested for size() in the 0, 1 and 2 cases, it could still\n> fail for other values.\n\nI also already test the size() returns 2 at the end of this test procedure.\n\n\n\n> \n> My point is that the test shouldn't be random, but should meaningfully\n> test for problems we foresee (or problems we have experienced). In this\n> case I would write a test suite that essentially tests the underlying\n> containers, I would concentrate on the added value of the ControlList\n> class.\n> \n> If you want to test the underlying container then I think you'll have to\n> test it function by function, including the iterators.\n\n\nI don't want to test the *underlying* container (I don't care if it\nchanges underneath). I want to guarantee that the ControlList API\ncontracts are held to account.\n\n\nRight - except now I've finally got here, I have now seen your\nsuggestion of changing the ControlList to just be a 'using' statement to\ndirectly bring in the unordered_map.\n\nThat would then remove the wrapper, and my desire to test the wrapper.\n\nI believe I added the wrapper class with an intention to add extra\nchecks etc, so I'm not yet sure if it can be removed - but if it does -\nit removes a lot of these tests. So win-win right...\n\n\n\n\n>>> And another very useful test would operate at the request level with the\n>>> VIMC pipeline handler, and verify that controls are correctly applied.\n>>> That's more work though :-)\n>>\n>> Sure - but that's not testing the ControlList, that's testing Controls\n>> and V4L2Controls.\n> \n> It would still indirectly test the control list :-) But I agree that\n> indirect tests don't necessarily provide the coverage we need.\n> \n>> Useful to add - but these were to validate and verify the function of\n>> the container object.\n>>\n>> This test-set is \"testControlList\".\n>>\n>> Testing the controls themselves is a different layer and while they\n>> might be in this 'controls.cpp file', they would be in a function called\n>> testControls().\n>>\n>> Testing Controls require the controls to be clearly defined, and have an\n>> implementation for managing the controls in the VIMC (or required)\n>> pipeline handler.\n>>\n>> IMO Those should be added, but once the pipeline handlers are updated to\n>> provide that required functionality.\n> \n> Yes, that will be done in a separate step.\n> \n>>>>> +\n>>>>> +\t\tif (list.size() != 0) {\n>>>>> +\t\t\tcout << \"List should contain zero elements\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\tif (!list.empty()) {\n>>>>> +\t\t\tcout << \"List expected to be empty\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\t/* Set a control */\n>>>>> +\t\tlist[ManualBrightness] = 255;\n>>>>> +\n>>>>> +\t\t/* Contains should still not find a non set control */\n>>>>> +\t\tif (list.contains(ManualGain)) {\n>>>>> +\t\t\tcout << \"Unexpected item in the bagging area\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\tif (list.size() != 1) {\n>>>>> +\t\t\tcout << \"List should contain one element\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\tif (list.empty()) {\n>>>>> +\t\t\tcout << \"List not expected to be empty\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\t/* Finally lets find that Gain */\n>>>>> +\t\tlist[ManualGain] = 128;\n>>>>> +\t\tif (!list.contains(ManualGain)) {\n>>>>> +\t\t\tcout << \"Couldn't identify an in-list control\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\t/* Validate value retrieval */\n>>>>> +\t\tif (list[ManualGain].getInt() != 128 ||\n>>>>> +\t\t list[ManualBrightness].getInt() != 255) {\n>>>>> +\t\t\tcout << \"Failed to retrieve control value\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\t/* Update using ControlInfo */\n>>>>> +\t\tControlInfo gainInfo(ManualGain);\n>>>>> +\t\tlist.update(gainInfo, 200);\n>>>>> +\t\tif (list[ManualGain].getInt() != 200) {\n>>>>> +\t\t\tcout << \"Failed to update control value\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\t/* Create a new list from an existing one */\n>>>>> +\t\tControlList newList;\n>>>>> +\n>>>>> +\t\tnewList.update(list);\n>>>>> +\n>>>>> +\t\t/*\n>>>>> +\t\t * Clear down the original list and assert items are still in\n>>>>> +\t\t * the new list\n>>>>> +\t\t */\n>>>>> +\t\tlist.reset();\n>>>>> +\n>>>>> +\t\tif (list.size() != 0) {\n>>>>> +\t\t\tcout << \"Old List should contain zero elements\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\tif (!list.empty()) {\n>>>>> +\t\t\tcout << \"Old List expected to be empty\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\tif (newList.size() != 2) {\n>>>>> +\t\t\tcout << \"New list with incorrect size\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\tif (!(newList.contains(ManualGain) &&\n>>>>> +\t\t newList.contains(ManualBrightness))) {\n>>>>> +\t\t\tcout << \"New list with incorrect items\" << endl;\n>>>>> +\t\t\treturn TestFail;\n>>>>> +\t\t}\n>>>>> +\n>>>>> +\t\treturn TestPass;\n>>>>> +\t}\n>>>>> +\n>>>>> \tint run()\n>>>>> \t{\n>>>>> \t\tint ret;\n>>>>> @@ -33,6 +128,10 @@ protected:\n>>>>> \t\tif (ret)\n>>>>> \t\t\treturn ret;\n>>>>> \n>>>>> +\t\tret = testControlList();\n>>>>> +\t\tif (ret)\n>>>>> +\t\t\treturn ret;\n>>>>> +\n>>>>> \t\treturn TestPass;\n>>>>> \t}\n>>>>> };\n>","headers":{"Return-Path":"","Received":["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 AA7BC61580\n\tfor ;\n\tMon, 24 Jun 2019 18:57:54 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D78962E7;\n\tMon, 24 Jun 2019 18:57:53 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561395474;\n\tbh=D8XNBWfKH2bQ5R2sVtIjqDe7vrdtwiZpduWPxHEMPzs=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=a8h6Rw+WcR9rgOiLoerljCWstLxsEeknb6bv8Mkl7IrCNigRaZNBxTxFfz+sX9Eef\n\tfl59l3aZcuiH6MOrITqrOOrCqEvosiiGnpANW2/Bw65F8Vlj0qtPIg9YpVH/8BfHq7\n\tLf5YJLzpY8T3dLAuaGZ29Qp8eRJ+dGxKAwqoN95k=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ","Cc":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tLibCamera Devel ","References":"<20190621161401.28337-1-kieran.bingham@ideasonboard.com>\n\t<20190621161401.28337-8-kieran.bingham@ideasonboard.com>\n\t<20190623153957.GF32581@bigcity.dyn.berto.se>\n\t<20190623224403.GG6124@pendragon.ideasonboard.com>\n\t\n\t<20190624123626.GI5737@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","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\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<091a8b05-17bf-a10b-bb50-9b0125435fba@ideasonboard.com>","Date":"Mon, 24 Jun 2019 17:57:50 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190624123626.GI5737@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC PATCH v2 7/9] libcamera: test: Add\n\tControlList tests","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 24 Jun 2019 16:57:54 -0000"}}]