Message ID | 20190521004059.7750-2-paul.elder@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Paul, Thank you for the patch. On Mon, May 20, 2019 at 08:40:59PM -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@ideasonboard.com> > --- > 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 | 65 ++++++++++++++++++++++++++++++++++++++++ > test/ipa/meson.build | 29 ++++++++++++++++++ > test/ipa/shared_test.c | 6 ++++ > test/ipa/shared_test.cpp | 12 ++++++++ > test/meson.build | 1 + > 5 files changed, 113 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..74854df > --- /dev/null > +++ b/test/ipa/ipa_test.cpp > @@ -0,0 +1,65 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * load-so.cpp - loading .so tests > + */ > + > +#include <iostream> > + > +#include "ipa_module.h" > + > +#include "test.h" > + > +using namespace std; > +using namespace libcamera; > + > +class IPAModuleTest : public Test > +{ > +protected: > + int init() > + { > + return 0; > + } No need to override init() if it's empty (same for cleanup() below). > + > + int run_test(const string path) s/run_test/runTest/ of better, name it testModuleValid() or similar ? const string &path, there's no need to make a copy. > + { > + cout << "running lib loader test" << endl; > + > + IPAModule *ll = new IPAModule(path); > + > + if (!ll->isValid()) { > + cout << "failed to load" << endl; cout << "IPA module " << path << " is not valid" << endl; You're leaking the ll pointer. > + return TestFail; > + } > + > + struct IPAModuleInfo info = ll->info(); > + cout << "loaded!" << endl; > + cout << "name = " << info.name << ", version = " << info.version << endl; You should validate the information here instead of printing. > + > + delete ll; > + return TestPass; > + } > + > + int run() int run() override > + { > + int count = 0; > + > + cout << "testing C IPAModule" << endl; We usually don't print messages in tests other than failure messages. > + count += run_test("test/ipa/ipa-dummy.so"); > + > + cout << "testing C++ IPAModule" << endl; > + count += run_test("test/ipa/ipa-dummy-cpp.so"); > + > + if (count < 0) This relies on TestPass being 0 and TestFail being -1, and on run_test() not returning TestSkip. Is this safe ? > + return TestFail; > + > + return TestPass; > + } > + > + void cleanup() > + { > + } > +}; > + > +TEST_REGISTER(IPAModuleTest) > diff --git a/test/ipa/meson.build b/test/ipa/meson.build > new file mode 100644 > index 0000000..dc8fbdd > --- /dev/null > +++ b/test/ipa/meson.build > @@ -0,0 +1,29 @@ > +ipa_test = [ > + ['ipa_test', 'ipa_test.cpp'], meson.build files use spaces instead of tabs for indentation (here and below). > +] > + > +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 > + > +ipa_dummy_sources = files([ > + 'shared_test.c' > +]) > + > +ipa_dummy_sources_cpp = files([ > + 'shared_test.cpp' > +]) > + > +ipa_dummy = shared_library('ipa-dummy', > + ipa_dummy_sources, > + name_prefix: '', > + include_directories: test_includes_public) > + > +ipa_dummy_cpp = shared_library('ipa-dummy-cpp', > + ipa_dummy_sources_cpp, > + name_prefix: '', > + include_directories: test_includes_public) How about the (untested) following ? ipa_modules = [] ipa_modules_sources = [ [ipa_dummy_c, files('shared_test.c')], [ipa_dummy_cpp, files('shared_test.cpp')], ] foreach m : ipa_modules_sources ipa_modules += shared_library(m[0], m[1], 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 Ideally, we should specify the modules as dependencies for the tests: test(t[0], exe, suite: 'ipa', is_parallel: false, depends: ipa_modules) but that was added in meson 0.46, which isn't available in Ubuntu 18.04 LTS, and I fear that would be an issue. > diff --git a/test/ipa/shared_test.c b/test/ipa/shared_test.c > new file mode 100644 > index 0000000..4959a03 > --- /dev/null > +++ b/test/ipa/shared_test.c > @@ -0,0 +1,6 @@ > +#include <libcamera/ipa/ipa_module_info.h> > + > +const struct IPAModuleInfo ipaModuleInfo = { > + .name = "Answer to the Ultimate Question of Life, the Universe, and Everything", > + .version = 42, > +}; > 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')
diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp new file mode 100644 index 0000000..74854df --- /dev/null +++ b/test/ipa/ipa_test.cpp @@ -0,0 +1,65 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * load-so.cpp - loading .so tests + */ + +#include <iostream> + +#include "ipa_module.h" + +#include "test.h" + +using namespace std; +using namespace libcamera; + +class IPAModuleTest : public Test +{ +protected: + int init() + { + return 0; + } + + int run_test(const string path) + { + cout << "running lib loader test" << endl; + + IPAModule *ll = new IPAModule(path); + + if (!ll->isValid()) { + cout << "failed to load" << endl; + return TestFail; + } + + struct IPAModuleInfo info = ll->info(); + cout << "loaded!" << endl; + cout << "name = " << info.name << ", version = " << info.version << endl; + + delete ll; + return TestPass; + } + + int run() + { + int count = 0; + + cout << "testing C IPAModule" << endl; + count += run_test("test/ipa/ipa-dummy.so"); + + cout << "testing C++ IPAModule" << endl; + count += run_test("test/ipa/ipa-dummy-cpp.so"); + + if (count < 0) + return TestFail; + + return TestPass; + } + + void cleanup() + { + } +}; + +TEST_REGISTER(IPAModuleTest) diff --git a/test/ipa/meson.build b/test/ipa/meson.build new file mode 100644 index 0000000..dc8fbdd --- /dev/null +++ b/test/ipa/meson.build @@ -0,0 +1,29 @@ +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 + +ipa_dummy_sources = files([ + 'shared_test.c' +]) + +ipa_dummy_sources_cpp = files([ + 'shared_test.cpp' +]) + +ipa_dummy = shared_library('ipa-dummy', + ipa_dummy_sources, + name_prefix: '', + include_directories: test_includes_public) + +ipa_dummy_cpp = shared_library('ipa-dummy-cpp', + ipa_dummy_sources_cpp, + name_prefix: '', + include_directories: test_includes_public) diff --git a/test/ipa/shared_test.c b/test/ipa/shared_test.c new file mode 100644 index 0000000..4959a03 --- /dev/null +++ b/test/ipa/shared_test.c @@ -0,0 +1,6 @@ +#include <libcamera/ipa/ipa_module_info.h> + +const struct IPAModuleInfo ipaModuleInfo = { + .name = "Answer to the Ultimate Question of Life, the Universe, and Everything", + .version = 42, +}; 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')
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@ideasonboard.com> --- 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 | 65 ++++++++++++++++++++++++++++++++++++++++ test/ipa/meson.build | 29 ++++++++++++++++++ test/ipa/shared_test.c | 6 ++++ test/ipa/shared_test.cpp | 12 ++++++++ test/meson.build | 1 + 5 files changed, 113 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