[libcamera-devel] [PATCH] tests: Add a base Test class

[Kieran Bingham]

Hi Laurent,

On 20/12/2018 18:53, Niklas Söderlund wrote:
> Hi Laurent,
> 
> Thanks for your work.
> 
> On 2018-12-19 11:48:30 +0200, Laurent Pinchart wrote:
>> The base Test class is meant to provide infrastructure common to all
>> tests. It is very limited for now, and should be extended with at least
>> logging and assertion handling.
> 
> I agree it's a bit limited now but can serve as a good base to build 
> upon. I have used this to write tests for device enumerator work.
> 

I've also used this as a base to rework my tests on top of, and I've now
refactored my v4l2device tests which does indeed reduce quite a bit of
code duplication.

>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> 
>> ---
>>  test/meson.build |  6 ++++++
>>  test/test.cpp    | 28 ++++++++++++++++++++++++++++
>>  test/test.h      | 28 ++++++++++++++++++++++++++++
>>  3 files changed, 62 insertions(+)
>>  create mode 100644 test/test.cpp
>>  create mode 100644 test/test.h
>>
>> diff --git a/test/meson.build b/test/meson.build
>> index 24d1927f977c..fc9c124927d2 100644
>> --- a/test/meson.build
>> +++ b/test/meson.build
>> @@ -1,3 +1,9 @@
>> +libtest_sources = files([
>> +    'test.cpp',
>> +])
>> +
>> +libtest = static_library('libtest', libtest_sources)

I wonder if libtest should get it's own directory as test/libtest to
separate it out. Especially as it will likely expand with logging and
other helpers.

>> +
>>  test_init = executable('test_init', 'init.cpp',
>>                         link_with : libcamera,
>>                         include_directories : libcamera_includes)
>> diff --git a/test/test.cpp b/test/test.cpp
>> new file mode 100644
>> index 000000000000..4e7779e750d5
>> --- /dev/null
>> +++ b/test/test.cpp
>> @@ -0,0 +1,28 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (C) 2018, Google Inc.
>> + *
>> + * test.cpp - libcamera test base class
>> + */
>> +
>> +#include "test.h"
>> +
>> +Test::Test()
>> +{
>> +}
>> +
>> +Test::~Test()
>> +{
>> +	cleanup();
>> +}
>> +
>> +int Test::execute()
>> +{
>> +	int ret;
>> +
>> +	ret = init();
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return run();
>> +}
>> diff --git a/test/test.h b/test/test.h
>> new file mode 100644
>> index 000000000000..2464fc5cb607
>> --- /dev/null
>> +++ b/test/test.h
>> @@ -0,0 +1,28 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (C) 2018, Google Inc.
>> + *
>> + * test.h - libcamera test base class
>> + */

Shouldn't we have an include guard here?

>> +
>> +#include <sstream>
>> +
>> +class Test
>> +{
>> +public:
>> +	Test();
>> +	virtual ~Test();
>> +
>> +	int execute();
>> +
>> +protected:
>> +	virtual int init() { return 0; }
>> +	virtual int run() = 0;
>> +	virtual void cleanup() { }
>> +};
>> +
>> +#define TEST_REGISTER(klass)						\
>> +int main(int argc, char *argv[])					\
>> +{									\
>> +	return klass().execute();					\
>> +}
>> -- 
>> Regards,
>>
>> Laurent Pinchart
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
> 

-- 
Regards
--
Kieran