[libcamera-devel,2/4] test: logging: add logSetStream test

1676 diff mbox series

Test the new logSetStream loggin API call. Reorganize the logging API
tests at the same time.

logSetTarget is not tested since the nowhere and syslog logging
destinations do not really need to be tested.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 test/log.cpp | 82 +++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 24 deletions(-)

Hi Paul,

Thank you for the patch.

On Sat, Jul 13, 2019 at 05:16:18AM +0900, Paul Elder wrote:
> Test the new logSetStream loggin API call. Reorganize the logging API

s/loggin/logging/

> tests at the same time.

As for 1/4, it would probably have been easier to review if you had
split the rework into a preparatory patch.

> logSetTarget is not tested since the nowhere and syslog logging
> destinations do not really need to be tested.

I would test logSetTarget(LoggingTargetNone) and verify logging a
message doesn't crash, and then that logSetTarget(LoggingTargetFile) and
logSetTarget(LoggingTargetStream) both fail.

> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  test/log.cpp | 82 +++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 58 insertions(+), 24 deletions(-)
> 
> diff --git a/test/log.cpp b/test/log.cpp
> index 89fb5ca..c1446bd 100644
> --- a/test/log.cpp
> +++ b/test/log.cpp
> @@ -29,19 +29,8 @@ LOG_DEFINE_CATEGORY(LogAPITest)
>  class LogAPITest : public Test
>  {
>  protected:
> -	int run() override
> +	void doLogging()
>  	{
> -		int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);
> -		if (fd < 0) {
> -			cerr << "Failed to open tmp log file" << endl;
> -			return TestFail;
> -		}
> -
> -		char path[32];
> -		snprintf(path, sizeof(path), "/proc/self/fd/%u", fd);
> -
> -		logSetFile(path);
> -
>  		logSetLevel("LogAPITest", "DEBUG");
>  		LOG(LogAPITest, Info) << "good 1";
>  
> @@ -55,19 +44,13 @@ protected:
>  		logSetLevel("LogAPITest", "WARN");
>  		LOG(LogAPITest, Warning) << "good 5";
>  		LOG(LogAPITest, Info) << "bad";
> +	}
>  
> -		char buf[1000];
> -		memset(buf, 0, sizeof(buf));
> -		lseek(fd, 0, SEEK_SET);
> -		if (read(fd, buf, sizeof(buf)) < 0) {
> -			cerr << "Failed to read tmp log file" << endl;
> -			return TestFail;
> -		}
> -		close(fd);
> -
> -		std::list<int> goodList = { 1, 3, 5 };
> -		std::basic_istringstream<char> iss((std::string(buf)));
> -		std::string line;
> +	int verifyOutput(string &str)
> +	{
> +		list<int> goodList = { 1, 3, 5 };
> +		basic_istringstream<char> iss(str);

You can use istringstream instead of basic_istringstream<char>.

> +		string line;
>  		while (getline(iss, line)) {
>  			if (goodList.empty()) {
>  				cout << "Too many log lines" << endl;
> @@ -90,6 +73,57 @@ protected:
>  
>  		return TestPass;
>  	}
> +
> +	int testFile()
> +	{
> +		int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);
> +		if (fd < 0) {
> +			cerr << "Failed to open tmp log file" << endl;
> +			return TestFail;
> +		}
> +
> +		char path[32];
> +		snprintf(path, sizeof(path), "/proc/self/fd/%u", fd);
> +
> +		logSetFile(path);

You should return an error if logSetFile() fails (and print a message).

> +
> +		doLogging();
> +
> +		char buf[1000];
> +		memset(buf, 0, sizeof(buf));
> +		lseek(fd, 0, SEEK_SET);
> +		if (read(fd, buf, sizeof(buf)) < 0) {
> +			cerr << "Failed to read tmp log file" << endl;
> +			return TestFail;
> +		}
> +		close(fd);
> +
> +		string str(buf);
> +		return verifyOutput(str);
> +	}
> +
> +	int testStream()
> +	{
> +		ostringstream log;
> +		logSetStream(log);
> +
> +		doLogging();
> +
> +		string str(log.str());

How about passing istream to verifyOutput() ? You could then avoid
turning the string into an istringstream internally. You will need to
turn ostringstream into a stringstream. For the file case you would
create the istringstream in testFile().

> +		return verifyOutput(str);
> +	}
> +
> +	int run() override
> +	{
> +		int ret1 = testFile();
> +
> +		int ret2 = testStream();
> +
> +		if (ret1 == TestPass && ret2 == TestPass)
> +			return TestPass;

I think you can return failures as soon as they occur.

> +
> +		return TestFail;

Let's revert the condition to return TestSuccess at the end, like all
other tests.

> +	}
>  };
>  
>  TEST_REGISTER(LogAPITest)