[libcamera-devel] [PATCH v5 2/2] tests: ipa: add tests to test IPAModule

[Laurent Pinchart]

On Tue, May 21, 2019 at 04:54:29PM -0400, Paul Elder wrote:
> Add tests to test the the IPAModule class, for loading the IPA module
> info from IPA module .so shared objects, with modules written in both C
> and C++.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> Changes in v5:
> - remove non-error test output
> - auto-check if the correct module info was loaded
> - make the meson build for the test shared objects prettier
> 
> No changes in v4
> 
> Changes in v3:
> - remove tests for incorrect bitness
> - make the test IPA module .so a C one and a C++ one
> 
> Changes in v2:
> - added source for test .so
> - updated tests to work with new (v2, see 1/2) IPAModule API
> 
>  test/ipa/ipa_test.cpp    | 79 ++++++++++++++++++++++++++++++++++++++++
>  test/ipa/meson.build     | 21 +++++++++++
>  test/ipa/shared_test.c   |  6 +++
>  test/ipa/shared_test.cpp | 12 ++++++
>  test/meson.build         |  1 +
>  5 files changed, 119 insertions(+)
>  create mode 100644 test/ipa/ipa_test.cpp
>  create mode 100644 test/ipa/meson.build
>  create mode 100644 test/ipa/shared_test.c
>  create mode 100644 test/ipa/shared_test.cpp
> 
> diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp
> new file mode 100644
> index 0000000..fb8eeba
> --- /dev/null
> +++ b/test/ipa/ipa_test.cpp
> @@ -0,0 +1,79 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * load-so.cpp - loading .so tests
> + */
> +
> +#include <iostream>
> +#include <string.h>
> +
> +#include "ipa_module.h"
> +
> +#include "test.h"
> +
> +using namespace std;
> +using namespace libcamera;
> +
> +class IPAModuleTest : public Test
> +{
> +protected:
> +	int init()
> +	{
> +		strcpy(testInfo_.name, "It's over nine thousand!");
> +		testInfo_.version = 9001;
> +
> +		return 0;
> +	}
> +
> +	int runTest(const string &path)
> +	{
> +		int ret = 0;
> +		struct IPAModuleInfo info;
> +
> +		IPAModule *ll = new IPAModule(path);
> +
> +		if (!ll->isValid()) {
> +			cerr << "test IPA module is invalid" << endl;
> +			ret = -1;
> +			goto done;
> +		}
> +
> +		info = ll->info();
> +		// todo validate instead of print

Please validate instead of printing :-) You can pass the expected name
and version number from the caller to make this easier. Or you can use
the same info for both modules.

> +		if (strcmp(info.name, testInfo_.name)) {
> +			cerr << "test IPA module has incorrect name" << endl;
> +			cerr << "expected \"" << testInfo_.name << "\", got \"" << info.name << "\"" << endl;
> +			ret = -1;
> +		}
> +
> +		if (info.version != testInfo_.version) {
> +			cerr << "test IPA module has incorrect version" << endl;
> +			cerr << "expected \"" << testInfo_.version << "\", got \"" << info.version << "\"" << endl;
> +			ret = -1;
> +		}

This will fail for the C implementation.

> +
> +	done:
> +		delete ll;
> +		return ret;
> +	}
> +
> +	int run() override
> +	{
> +		int count = 0;
> +
> +		count += runTest("test/ipa/ipa-dummy.so");
> +
> +		count += runTest("test/ipa/ipa-dummy-cpp.so");
> +
> +		if (count < 0)
> +			return TestFail;
> +
> +		return TestPass;
> +	}
> +
> +private:
> +	struct IPAModuleInfo testInfo_;
> +};
> +
> +TEST_REGISTER(IPAModuleTest)
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> new file mode 100644
> index 0000000..57c6a1c
> --- /dev/null
> +++ b/test/ipa/meson.build
> @@ -0,0 +1,21 @@
> +ipa_modules_sources = [
> +    ['ipa-dummy',     'shared_test.c'],

I'd name this ipa-dummy-c. With this fixed,

> +    ['ipa-dummy-cpp', 'shared_test.cpp'],
> +]
> +
> +foreach m : ipa_modules_sources
> +    shared_library(m, name_prefix: '',
> +                   include_directories: test_includes_public)
> +endforeach
> +
> +ipa_test = [
> +    ['ipa_test', 'ipa_test.cpp'],
> +]
> +
> +foreach t : ipa_test
> +    exe = executable(t[0], t[1],
> +                     link_with : test_libraries,
> +                     include_directories : test_includes_internal)
> +
> +    test(t[0], exe, suite: 'ipa', is_parallel: false)
> +endforeach
> diff --git a/test/ipa/shared_test.c b/test/ipa/shared_test.c
> new file mode 100644
> index 0000000..87d182b
> --- /dev/null
> +++ b/test/ipa/shared_test.c
> @@ -0,0 +1,6 @@
> +#include <libcamera/ipa/ipa_module_info.h>
> +
> +const struct IPAModuleInfo ipaModuleInfo = {
> +	.name = "It's over nine thousand!",
> +	.version = 9001,
> +};
> diff --git a/test/ipa/shared_test.cpp b/test/ipa/shared_test.cpp
> new file mode 100644
> index 0000000..4e5c976
> --- /dev/null
> +++ b/test/ipa/shared_test.cpp
> @@ -0,0 +1,12 @@
> +#include <libcamera/ipa/ipa_module_info.h>
> +
> +namespace libcamera {
> +
> +extern "C" {
> +const struct libcamera::IPAModuleInfo ipaModuleInfo = {
> +	"It's over nine thousand!",
> +	9001,
> +};
> +};
> +
> +}; /* namespace libcamera */
> diff --git a/test/meson.build b/test/meson.build
> index d501f2b..ef41367 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -1,6 +1,7 @@
>  subdir('libtest')
>  
>  subdir('camera')
> +subdir('ipa')
>  subdir('media_device')
>  subdir('pipeline')
>  subdir('v4l2_device')

-- 
Regards,

Laurent Pinchart