[{"id":16243,"web_url":"https://patchwork.libcamera.org/comment/16243/","msgid":"","date":"2021-04-13T20:51:31","subject":"Re: [libcamera-devel] [PATCH 1/2] test: span: Add tests for begin()\n\tand rend()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 13/04/2021 01:27, Laurent Pinchart wrote:\n> Verify that the begin() and rend() iterators (and their const version)\n> reference the correct values. The end() and rbegin() iterators can't be\n> tested the same way as they're not dereferenceable.\n\nCan you instead verify that the item preceding the end() or after\nrbegin() are correct?\n\nOr is there no real value in that anyway...\n\n\nBut your statement here doesn't seem to match the code.\n\nYou add tests for begin and cbegin, and rbegin and crbegin.\n\nSo perhaps you mean you can't test end(), cend(), rend() and crend()\nbecause they are not dereferencable..\n\nPresumably here, with the commit message clarified, as I expect the code\nto have been compile tested:\n\nReviewed-by: Kieran Bingham \n\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> test/span.cpp | 40 ++++++++++++++++++++++++++++++++--------\n> 1 file changed, 32 insertions(+), 8 deletions(-)\n> \n> diff --git a/test/span.cpp b/test/span.cpp\n> index d60b769c9877..ca037c8f02fa 100644\n> --- a/test/span.cpp\n> +++ b/test/span.cpp\n> @@ -72,12 +72,24 @@ protected:\n> \n> \t\tstaticSpan = Span{ v };\n> \n> -\t\tstaticSpan.begin();\n> -\t\tstaticSpan.cbegin();\n> +\t\tif (*staticSpan.begin() != 1) {\n> +\t\t\tstd::cout << \"Span::begin() failed\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t\tif (*staticSpan.cbegin() != 1) {\n> +\t\t\tstd::cout << \"Span::cbegin() failed\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> \t\tstaticSpan.end();\n> \t\tstaticSpan.cend();\n> -\t\tstaticSpan.rbegin();\n> -\t\tstaticSpan.crbegin();\n> +\t\tif (*staticSpan.rbegin() != 4) {\n> +\t\t\tstd::cout << \"Span::rbegin() failed\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t\tif (*staticSpan.crbegin() != 4) {\n> +\t\t\tstd::cout << \"Span::crbegin() failed\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> \t\tstaticSpan.rend();\n> \t\tstaticSpan.crend();\n> \n> @@ -141,12 +153,24 @@ protected:\n> \n> \t\tdynamicSpan = Span{ a };\n> \n> -\t\tdynamicSpan.begin();\n> -\t\tdynamicSpan.cbegin();\n> +\t\tif (*dynamicSpan.begin() != 1) {\n> +\t\t\tstd::cout << \"Span::begin() failed\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t\tif (*dynamicSpan.cbegin() != 1) {\n> +\t\t\tstd::cout << \"Span::cbegin() failed\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> \t\tdynamicSpan.end();\n> \t\tdynamicSpan.cend();\n> -\t\tdynamicSpan.rbegin();\n> -\t\tdynamicSpan.crbegin();\n> +\t\tif (*dynamicSpan.rbegin() != 4) {\n> +\t\t\tstd::cout << \"Span::rbegin() failed\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t\tif (*dynamicSpan.crbegin() != 4) {\n> +\t\t\tstd::cout << \"Span::crbegin() failed\" << std::endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> \t\tdynamicSpan.rend();\n> \t\tdynamicSpan.crend();\n> \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 24F06BD224\n\tfor ;\n\tTue, 13 Apr 2021 20:51:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8BADA68800;\n\tTue, 13 Apr 2021 22:51:35 +0200 (CEST)","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 004F3602D1\n\tfor ;\n\tTue, 13 Apr 2021 22:51:34 +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 585AE9F0;\n\tTue, 13 Apr 2021 22:51:34 +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=\"kn3FA6Ud\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1618347094;\n\tbh=OWnOXfBlLsXNPsGDVTvSMcdwLGUyqcfewaRxXATagKk=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=kn3FA6UdGH2BuHLOv9a2t/X7gDqgHFoRRSRXKfEXfmZriKZ4e/9hbCZ+hDwW5GPni\n\tk/6V+QOBLhYV4JcilkaGmWLm4H87Eom20bSMqUIjNw5kd5vMgRUqWp9sC91/tDrvdN\n\tVz095CcgYk0Tlzi6l0eku7VEc2CY7V6+SCT7CyvE=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210413002731.25653-1-laurent.pinchart@ideasonboard.com>\n\t<20210413002731.25653-2-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","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":"","Date":"Tue, 13 Apr 2021 21:51:31 +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":"<20210413002731.25653-2-laurent.pinchart@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/2] test: span: Add tests for begin()\n\tand rend()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16245,"web_url":"https://patchwork.libcamera.org/comment/16245/","msgid":"","date":"2021-04-13T21:16:30","subject":"Re: [libcamera-devel] [PATCH 1/2] test: span: Add tests for begin()\n\tand rend()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Apr 13, 2021 at 09:51:31PM +0100, Kieran Bingham wrote:\n> On 13/04/2021 01:27, Laurent Pinchart wrote:\n> > Verify that the begin() and rend() iterators (and their const version)\n> > reference the correct values. The end() and rbegin() iterators can't be\n> > tested the same way as they're not dereferenceable.\n> \n> Can you instead verify that the item preceding the end() or after\n> rbegin() are correct?\n> \n> Or is there no real value in that anyway...\n\nI think more tests for the Span class make sense, including iterating\nover the whole container, but that's an exercise left to the reader :-)\n\n> But your statement here doesn't seem to match the code.\n> \n> You add tests for begin and cbegin, and rbegin and crbegin.\n> \n> So perhaps you mean you can't test end(), cend(), rend() and crend()\n> because they are not dereferencable..\n\nIndeed, it should have read\n\nVerify that the begin() and rbegin() iterators (and their const version)\nreference the correct values. The end() and rend() iterators can't be\ntested the same way as they're not dereferenceable.\n\n> Presumably here, with the commit message clarified, as I expect the code\n> to have been compile tested:\n> \n> Reviewed-by: Kieran Bingham \n> \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > test/span.cpp | 40 ++++++++++++++++++++++++++++++++--------\n> > 1 file changed, 32 insertions(+), 8 deletions(-)\n> > \n> > diff --git a/test/span.cpp b/test/span.cpp\n> > index d60b769c9877..ca037c8f02fa 100644\n> > --- a/test/span.cpp\n> > +++ b/test/span.cpp\n> > @@ -72,12 +72,24 @@ protected:\n> > \n> > \t\tstaticSpan = Span{ v };\n> > \n> > -\t\tstaticSpan.begin();\n> > -\t\tstaticSpan.cbegin();\n> > +\t\tif (*staticSpan.begin() != 1) {\n> > +\t\t\tstd::cout << \"Span::begin() failed\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\t\tif (*staticSpan.cbegin() != 1) {\n> > +\t\t\tstd::cout << \"Span::cbegin() failed\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > \t\tstaticSpan.end();\n> > \t\tstaticSpan.cend();\n> > -\t\tstaticSpan.rbegin();\n> > -\t\tstaticSpan.crbegin();\n> > +\t\tif (*staticSpan.rbegin() != 4) {\n> > +\t\t\tstd::cout << \"Span::rbegin() failed\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\t\tif (*staticSpan.crbegin() != 4) {\n> > +\t\t\tstd::cout << \"Span::crbegin() failed\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > \t\tstaticSpan.rend();\n> > \t\tstaticSpan.crend();\n> > \n> > @@ -141,12 +153,24 @@ protected:\n> > \n> > \t\tdynamicSpan = Span{ a };\n> > \n> > -\t\tdynamicSpan.begin();\n> > -\t\tdynamicSpan.cbegin();\n> > +\t\tif (*dynamicSpan.begin() != 1) {\n> > +\t\t\tstd::cout << \"Span::begin() failed\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\t\tif (*dynamicSpan.cbegin() != 1) {\n> > +\t\t\tstd::cout << \"Span::cbegin() failed\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > \t\tdynamicSpan.end();\n> > \t\tdynamicSpan.cend();\n> > -\t\tdynamicSpan.rbegin();\n> > -\t\tdynamicSpan.crbegin();\n> > +\t\tif (*dynamicSpan.rbegin() != 4) {\n> > +\t\t\tstd::cout << \"Span::rbegin() failed\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\t\tif (*dynamicSpan.crbegin() != 4) {\n> > +\t\t\tstd::cout << \"Span::crbegin() failed\" << std::endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > \t\tdynamicSpan.rend();\n> > \t\tdynamicSpan.crend();\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2A01ABD224\n\tfor ;\n\tTue, 13 Apr 2021 21:17:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D396687FE;\n\tTue, 13 Apr 2021 23:17:23 +0200 (CEST)","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 77407602D1\n\tfor ;\n\tTue, 13 Apr 2021 23:17:22 +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 7DBD39F0;\n\tTue, 13 Apr 2021 23:17:21 +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=\"sRNEOw5/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1618348641;\n\tbh=y9Z4W16HiqBm+vtUT0K+GA9kFeFRxrTNlHpr1OG/PCM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sRNEOw5/Zlo0uNw6knd+IpTrv6rOFHUr94fRnwRdMNy5Fwf21E2S7oC/tdabJZMW0\n\tfnyXkHu05ps73dt7RzzOVZ1ONttUViF4XrB7EAsTQQnbPQLe1f8IHjfks+etBFnwy3\n\tghH8FgeXGDYY6qRsIIHlb8yllTUIJkPwOTMVpLLs=","Date":"Wed, 14 Apr 2021 00:16:30 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210413002731.25653-1-laurent.pinchart@ideasonboard.com>\n\t<20210413002731.25653-2-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/2] test: span: Add tests for begin()\n\tand rend()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]