[libcamera-devel,v2,2/2] test: add logging API test

1654 diff mbox series

Test that setting the log file and log levels works from an application
point of view. The test uses the internal logging mechanism as well,
just to write to the log file.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
Changes in v2:
- added more test cases to catch if log level changes fail

 test/log/log_api.cpp | 75 ++++++++++++++++++++++++++++++++++++++++++++
 test/log/meson.build | 12 +++++++
 test/meson.build     |  1 +
 3 files changed, 88 insertions(+)
 create mode 100644 test/log/log_api.cpp
 create mode 100644 test/log/meson.build

Hi Paul,

Thank you for the patch.

On Thu, Jul 11, 2019 at 07:24:18PM +0900, Paul Elder wrote:
> Test that setting the log file and log levels works from an application
> point of view. The test uses the internal logging mechanism as well,
> just to write to the log file.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> Changes in v2:
> - added more test cases to catch if log level changes fail
> 
>  test/log/log_api.cpp | 75 ++++++++++++++++++++++++++++++++++++++++++++
>  test/log/meson.build | 12 +++++++
>  test/meson.build     |  1 +

There's no need to create a directory for a single test, you can move it
to test/, and I would even call it log.cpp for now.

>  3 files changed, 88 insertions(+)
>  create mode 100644 test/log/log_api.cpp
>  create mode 100644 test/log/meson.build
> 
> diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp
> new file mode 100644
> index 0000000..395b857
> --- /dev/null
> +++ b/test/log/log_api.cpp
> @@ -0,0 +1,75 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * log_api.cpp - log API test
> + */
> +
> +#include <algorithm>
> +#include <fcntl.h>
> +#include <iostream>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include <libcamera/logging.h>
> +
> +#include "log.h"
> +#include "test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(LogAPITest)
> +
> +class LogAPITest : public Test
> +{
> +protected:
> +	int run() override
> +	{
> +		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[PATH_MAX];

That's a bit long, 32 will largely suffice.

> +		snprintf(path, PATH_MAX, "/proc/self/fd/%d", fd);

s/PATH_MAX/sizeof(path)/
s/%d/%u/ as we know the number is positive

> +
> +		logSetFile(path);
> +
> +		logSetLevel("LogAPITest", "DEBUG");
> +		LOG(LogAPITest, Info) << "asdf";
> +
> +		logSetLevel("LogAPITest", "WARN");
> +		LOG(LogAPITest, Info) << "asdf";
> +
> +		logSetLevel("LogAPITest", "WARN");
> +		LOG(LogAPITest, Info) << "asdf";

Those two lines don't appear to do anything more than the two previous
ones.

> +		LOG(LogAPITest, Debug) << "asdf";
> +
> +		logSetLevel("LogAPITest", "ERROR");
> +		LOG(LogAPITest, Error) << "asdf";
> +		LOG(LogAPITest, Info) << "asdf";
> +
> +		logSetLevel("LogAPITest", "WARN");
> +		LOG(LogAPITest, Warning) << "asdf";
> +		LOG(LogAPITest, Info) << "asdf";

I would print a sequence of numbers instead of "asdf", and verify that
the expected numbers are printed. This will be easier if you read the
whole file (in a bigger array than 200 bytes) and wrap the buffer in an
istringstream
(https://en.cppreference.com/w/cpp/io/basic_istringstream). You can then
call getline() in a loop and get the last character of each line.

> +
> +		char buf[200];
> +		lseek(fd, 0, SEEK_SET);
> +		read(fd, buf, 1000);
> +		close(fd);
> +
> +		std::string s(buf);
> +		int n = count(s.begin(), s.end(), '\n');
> +		if (n == 3)
> +			return TestPass;
> +
> +		return TestFail;
> +	}
> +};
> +
> +TEST_REGISTER(LogAPITest)
> diff --git a/test/log/meson.build b/test/log/meson.build
> new file mode 100644
> index 0000000..35ea553
> --- /dev/null
> +++ b/test/log/meson.build
> @@ -0,0 +1,12 @@
> +log_api_test = [
> +    ['log_api', 'log_api.cpp'],
> +]
> +
> +foreach t : log_api_test
> +    exe = executable(t[0], t[1],
> +                     dependencies : libcamera_dep,
> +                     link_with : test_libraries,
> +                     include_directories : test_includes_internal)
> +
> +    test(t[0], exe, suite : 'log')
> +endforeach
> diff --git a/test/meson.build b/test/meson.build
> index 60ce960..44d8495 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -4,6 +4,7 @@ subdir('camera')
>  subdir('controls')
>  subdir('ipa')
>  subdir('ipc')
> +subdir('log')
>  subdir('media_device')
>  subdir('pipeline')
>  subdir('stream')