[{"id":2254,"web_url":"https://patchwork.libcamera.org/comment/2254/","msgid":"<20190714100753.GC6043@pendragon.ideasonboard.com>","date":"2019-07-14T10:07:53","subject":"Re: [libcamera-devel] [PATCH 2/4] test: logging: add logSetStream\n\ttest","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Sat, Jul 13, 2019 at 05:16:18AM +0900, Paul Elder wrote:\n> Test the new logSetStream loggin API call. Reorganize the logging API\n\ns/loggin/logging/\n\n> tests at the same time.\n\nAs for 1/4, it would probably have been easier to review if you had\nsplit the rework into a preparatory patch.\n\n> logSetTarget is not tested since the nowhere and syslog logging\n> destinations do not really need to be tested.\n\nI would test logSetTarget(LoggingTargetNone) and verify logging a\nmessage doesn't crash, and then that logSetTarget(LoggingTargetFile) and\nlogSetTarget(LoggingTargetStream) both fail.\n\n> Signed-off-by: Paul Elder \n> ---\n> test/log.cpp | 82 +++++++++++++++++++++++++++++++++++++---------------\n> 1 file changed, 58 insertions(+), 24 deletions(-)\n> \n> diff --git a/test/log.cpp b/test/log.cpp\n> index 89fb5ca..c1446bd 100644\n> --- a/test/log.cpp\n> +++ b/test/log.cpp\n> @@ -29,19 +29,8 @@ LOG_DEFINE_CATEGORY(LogAPITest)\n> class LogAPITest : public Test\n> {\n> protected:\n> -\tint run() override\n> +\tvoid doLogging()\n> \t{\n> -\t\tint fd = open(\"/tmp\", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);\n> -\t\tif (fd < 0) {\n> -\t\t\tcerr << \"Failed to open tmp log file\" << endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> -\t\tchar path[32];\n> -\t\tsnprintf(path, sizeof(path), \"/proc/self/fd/%u\", fd);\n> -\n> -\t\tlogSetFile(path);\n> -\n> \t\tlogSetLevel(\"LogAPITest\", \"DEBUG\");\n> \t\tLOG(LogAPITest, Info) << \"good 1\";\n> \n> @@ -55,19 +44,13 @@ protected:\n> \t\tlogSetLevel(\"LogAPITest\", \"WARN\");\n> \t\tLOG(LogAPITest, Warning) << \"good 5\";\n> \t\tLOG(LogAPITest, Info) << \"bad\";\n> +\t}\n> \n> -\t\tchar buf[1000];\n> -\t\tmemset(buf, 0, sizeof(buf));\n> -\t\tlseek(fd, 0, SEEK_SET);\n> -\t\tif (read(fd, buf, sizeof(buf)) < 0) {\n> -\t\t\tcerr << \"Failed to read tmp log file\" << endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\t\tclose(fd);\n> -\n> -\t\tstd::list goodList = { 1, 3, 5 };\n> -\t\tstd::basic_istringstream iss((std::string(buf)));\n> -\t\tstd::string line;\n> +\tint verifyOutput(string &str)\n> +\t{\n> +\t\tlist goodList = { 1, 3, 5 };\n> +\t\tbasic_istringstream iss(str);\n\nYou can use istringstream instead of basic_istringstream.\n\n> +\t\tstring line;\n> \t\twhile (getline(iss, line)) {\n> \t\t\tif (goodList.empty()) {\n> \t\t\t\tcout << \"Too many log lines\" << endl;\n> @@ -90,6 +73,57 @@ protected:\n> \n> \t\treturn TestPass;\n> \t}\n> +\n> +\tint testFile()\n> +\t{\n> +\t\tint fd = open(\"/tmp\", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);\n> +\t\tif (fd < 0) {\n> +\t\t\tcerr << \"Failed to open tmp log file\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\tchar path[32];\n> +\t\tsnprintf(path, sizeof(path), \"/proc/self/fd/%u\", fd);\n> +\n> +\t\tlogSetFile(path);\n\nYou should return an error if logSetFile() fails (and print a message).\n\n> +\n> +\t\tdoLogging();\n> +\n> +\t\tchar buf[1000];\n> +\t\tmemset(buf, 0, sizeof(buf));\n> +\t\tlseek(fd, 0, SEEK_SET);\n> +\t\tif (read(fd, buf, sizeof(buf)) < 0) {\n> +\t\t\tcerr << \"Failed to read tmp log file\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\t\tclose(fd);\n> +\n> +\t\tstring str(buf);\n> +\t\treturn verifyOutput(str);\n> +\t}\n> +\n> +\tint testStream()\n> +\t{\n> +\t\tostringstream log;\n> +\t\tlogSetStream(log);\n> +\n> +\t\tdoLogging();\n> +\n> +\t\tstring str(log.str());\n\nHow about passing istream to verifyOutput() ? You could then avoid\nturning the string into an istringstream internally. You will need to\nturn ostringstream into a stringstream. For the file case you would\ncreate the istringstream in testFile().\n\n> +\t\treturn verifyOutput(str);\n> +\t}\n> +\n> +\tint run() override\n> +\t{\n> +\t\tint ret1 = testFile();\n> +\n> +\t\tint ret2 = testStream();\n> +\n> +\t\tif (ret1 == TestPass && ret2 == TestPass)\n> +\t\t\treturn TestPass;\n\nI think you can return failures as soon as they occur.\n\n> +\n> +\t\treturn TestFail;\n\nLet's revert the condition to return TestSuccess at the end, like all\nother tests.\n\n> +\t}\n> };\n> \n> TEST_REGISTER(LogAPITest)","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 1414361572\n\tfor ;\n\tSun, 14 Jul 2019 12:08:23 +0200 (CEST)","from pendragon.ideasonboard.com (softbank126159224182.bbtec.net\n\t[126.159.224.182])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 85BDE2B2;\n\tSun, 14 Jul 2019 12:08:21 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1563098902;\n\tbh=3cYeh45qYtBIIEqaqXzijV+a7fW4+yJ3Vg0z+WXEJS4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hSsL71HgNpmItBfdQd8URI4+ibxn07xhCRIPPUBHKgkrPj/ZDGz3RPUS5H3uJMACT\n\tOpObI9TxWEyUra58IZ8MH7ykfRIpubepOUXy94DwqqKNm5J4LD4D9AhvAQmnQ1akgu\n\taZXDlA8UZgowCJIZDjN8zqbN75rC+6hTw+ybGpWI=","Date":"Sun, 14 Jul 2019 13:07:53 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190714100753.GC6043@pendragon.ideasonboard.com>","References":"<20190712201620.30457-1-paul.elder@ideasonboard.com>\n\t<20190712201620.30457-2-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190712201620.30457-2-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/4] test: logging: add logSetStream\n\ttest","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, 14 Jul 2019 10:08:23 -0000"}}]