[libcamera-devel] [PATCH RFC 1/2] test: Add TestStatus classes

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Monday, 14 January 2019 11:54:16 EET Kieran Bingham wrote:
> Provide Class object to return the test status, and perform any result
> reporting.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  test/libtest/meson.build     |  1 +
>  test/libtest/test.h          |  2 ++
>  test/libtest/test_status.cpp | 42 +++++++++++++++++++++++++++
>  test/libtest/test_status.h   | 55 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 100 insertions(+)
>  create mode 100644 test/libtest/test_status.cpp
>  create mode 100644 test/libtest/test_status.h

[snip]

> diff --git a/test/libtest/test_status.h b/test/libtest/test_status.h
> new file mode 100644
> index 000000000000..2e4713ad3f6d
> --- /dev/null
> +++ b/test/libtest/test_status.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * test_status.h - libcamera test status class
> + */
> +#ifndef __TEST_TEST_STATUS_H__
> +#define __TEST_TEST_STATUS_H__
> +
> +#include <iostream>
> +#include <string>
> +
> +class TestStatusBase
> +{
> +public:
> +	TestStatusBase();
> +	TestStatusBase(const std::string &m);
> +	~TestStatusBase();
> +
> +	operator int() { return value; };
> +
> +	enum ReturnValues {
> +		ValuePass = 0,
> +		ValueFail = -1,
> +		ValueSkip = 77,
> +	};
> +
> +protected:
> +	int value;
> +	std::string prefix;
> +	std::string message;
> +};
> +
> +class TestStatusPass : public TestStatusBase
> +{
> +public:
> +	TestStatusPass(const std::string &m);

I think we need a way to log more complex messages with parameters.

> +};
> +
> +class TestStatusFail : public TestStatusBase
> +{
> +public:
> +	TestStatusFail(const std::string &m);
> +};
> +
> +class TestStatusSkip : public TestStatusBase
> +{
> +public:
> +	TestStatusSkip(const std::string &m);
> +};
> +
> +#define is(a, b, m) ({((a) == (b)) ? TestStatusPass((m)) :
> TestStatusFail((m));})
> +#define isnt(a, b, m) ({((a) != (b)) ? TestStatusPass((m)) :
> TestStatusFail((m));})

I would prefer names along the lines of assertEqual() and assertNotEqual(), as 
that would make it easier to add other helpers such as assertSmallerThan() for 
instance (whether we will want to do so remains to be decided).

I'm also concerned by the is() and isnt() macros closing a test (as in 
returning either pass or fail), as a single test may require multiple checks 
to decide on the result. I think this boils down to defining the test-related 
concepts we want to use (test case, test suite, unit test, ...) as explained 
in my other reply to this series, and then building on top of that. If we try 
to write code without clearly defining the direction we want to take it will 
be painful at best.

> +
> +#endif /* __TEST_TEST_STATUS_H__ */

-- 
Regards,

Laurent Pinchart