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

222 diff mbox series

Provide Class object to return the test status, and perform any result reporting.

Signed-off-by: Kieran Bingham <kieran.bingham@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

Hi Kieran,

On Mon, Jan 14, 2019 at 09:54:16AM +0000, Kieran Bingham wrote:
> Provide Class object to return the test status, and perform any result reporting.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@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
>
> diff --git a/test/libtest/meson.build b/test/libtest/meson.build
> index e0893b70c3d4..a9e67d8d7e6c 100644
> --- a/test/libtest/meson.build
> +++ b/test/libtest/meson.build
> @@ -1,5 +1,6 @@
>  libtest_sources = files([
>      'test.cpp',
> +    'test_status.cpp',
>  ])
>
>  libtest = static_library('libtest', libtest_sources)
> diff --git a/test/libtest/test.h b/test/libtest/test.h
> index ec08bf97c03d..d816cf15aaf0 100644
> --- a/test/libtest/test.h
> +++ b/test/libtest/test.h
> @@ -9,6 +9,8 @@
>
>  #include <sstream>
>
> +#include "test_status.h"
> +
>  enum TestStatus {
>  	TestPass = 0,
>  	TestFail = -1,

When you will replace this, please move it to test_status.h

> diff --git a/test/libtest/test_status.cpp b/test/libtest/test_status.cpp
> new file mode 100644
> index 000000000000..c0daef866740
> --- /dev/null
> +++ b/test/libtest/test_status.cpp
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * test_status.cpp - libcamera test result management
> + */
> +
> +#include "test.h"
> +
> +static unsigned int test_number = 0;
> +
> +TestStatusBase::TestStatusBase()
> +	: value(-99)

-99 ?

> +{
> +	test_number++;
> +};
> +
> +TestStatusBase::~TestStatusBase()
> +{
> +	std::cout << prefix << test_number << " " << message << std::endl;

cout or cerr? This has been asked already bu never finalized

> +};
> +
> +TestStatusPass::TestStatusPass(const std::string &m)
> +{
> +	value = ValuePass;
> +	prefix = "ok ";
> +	message = m;
> +};
> +
> +TestStatusFail::TestStatusFail(const std::string &m)
> +{
> +	value = ValueFail;
> +	prefix = "not ok ";
> +	message = m;
> +};
> +
> +TestStatusSkip::TestStatusSkip(const std::string &m)
> +{
> +	value = ValueSkip;
> +	prefix = "skip ";
> +	message = m;
> +};

Use the base constructor, as noted below in a comment to the header
file

> 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

Make this and the .cpp one the same

> + */
> +#ifndef __TEST_TEST_STATUS_H__
> +#define __TEST_TEST_STATUS_H__
> +
> +#include <iostream>

You can move this to the cpp file

> +#include <string>
> +
> +class TestStatusBase
> +{
> +public:
> +	TestStatusBase();
> +	TestStatusBase(const std::string &m);

TestStatusBase() is not used and shall be deleted
TestStatusBase(const std::string m) should be protected as this base
class shall not be instantiated directly

> +	~TestStatusBase();
> +
> +	operator int() { return value; };
> +
> +	enum ReturnValues {
> +		ValuePass = 0,
> +		ValueFail = -1,
> +		ValueSkip = 77,
> +	};

This and TestStatus can be unified

> +
> +protected:
> +	int value;
> +	std::string prefix;
> +	std::string message;

class member end with _
> +};
> +
> +class TestStatusPass : public TestStatusBase

All of these shall use their base constructor
> +{
> +public:
> +	TestStatusPass(const std::string &m);
> +};
> +
> +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));})
> +
> +#endif /* __TEST_TEST_STATUS_H__ */

Here it is a patch on top that addresses most of my comments and that
produces this output when run through the test_status_test

/home/jmondi/project/libcamera/libcamera.git/build/test/test_status
--- stdout ---
Test: 1: ok - [Verify TestStatusPass]
Test: 2: not ok - [Verify TestStatusFail]
Test: 3: skip - [Verify TestStatusSkip]
Test: 4: ok - [Good is return check]
Test: 5: ok - [Good isn't return check]
Test: 6: not ok - [Bad Is Check]
Test: 7: not ok - [Bad Isn't check]
Test: 8: ok - TestStatus validations
-------

diff --git a/test/libtest/test_status.cpp b/test/libtest/test_status.cpp
index c0daef8..be84556 100644
--- a/test/libtest/test_status.cpp
+++ b/test/libtest/test_status.cpp
@@ -5,38 +5,37 @@
  * test_status.cpp - libcamera test result management
  */

+#include <iostream>
+#include <string>
+
 #include "test.h"

 static unsigned int test_number = 0;

-TestStatusBase::TestStatusBase()
-       : value(-99)
+TestStatusBase::TestStatusBase(enum ReturnValues result,
+                              const std::string &prefix,
+                              const std::string &message)
+       : result_(result), prefix_(prefix), message_(message)
 {
        test_number++;
 };

 TestStatusBase::~TestStatusBase()
 {
-       std::cout << prefix << test_number << " " << message << std::endl;
+       std::cout << "Test: " << test_number << ": " << prefix_ << " - "
+                 << message_ << std::endl;

 };

-TestStatusPass::TestStatusPass(const std::string &m)
+TestStatusPass::TestStatusPass(const std::string &m) :
+       TestStatusBase(ValuePass, "ok", m)
 {
-       value = ValuePass;
-       prefix = "ok ";
-       message = m;
 };

-TestStatusFail::TestStatusFail(const std::string &m)
+TestStatusFail::TestStatusFail(const std::string &m) :
+       TestStatusBase(ValueFail, "not ok", m)
 {
-       value = ValueFail;
-       prefix = "not ok ";
-       message = m;
 };

-TestStatusSkip::TestStatusSkip(const std::string &m)
+TestStatusSkip::TestStatusSkip(const std::string &m) :
+       TestStatusBase(ValueSkip, "skip", m)
 {
-       value = ValueSkip;
-       prefix = "skip ";
-       message = m;
 };
diff --git a/test/libtest/test_status.h b/test/libtest/test_status.h
index 2e4713a..7f3bb26 100644
--- a/test/libtest/test_status.h
+++ b/test/libtest/test_status.h
@@ -7,17 +7,14 @@
 #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; };
+       operator int() { return result_; };

        enum ReturnValues {
                ValuePass = 0,
@@ -26,9 +23,12 @@ public:
        };

 protected:
-       int value;
-       std::string prefix;
 public:
-       TestStatusBase();
-       TestStatusBase(const std::string &m);
        ~TestStatusBase();

-       operator int() { return value; };
+       operator int() { return result_; };

        enum ReturnValues {
                ValuePass = 0,
@@ -26,9 +23,12 @@ public:
        };

 protected:
-       int value;
-       std::string prefix;
-       std::string message;
+       TestStatusBase() = delete;
+       TestStatusBase(enum ReturnValues result, const std::string &prefix,
+                      const std::string &m);
+       enum ReturnValues result_;
+       std::string prefix_;
+       std::string message_;
 };

 class TestStatusPass : public TestStatusBase


> --
> 2.17.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Jacopo,

Thanks - Shall I take your detailed review comments as a good indicator
that you like this direction?

I'm sorry it was not so polished, (hence the RFC) - I wanted to gauge
feel on if this is a good way forwards as much as showing the
implementation.

That said - you've fixed up the initialiser for the three classes for me :)

So now I can use:

TestPass::TestPass(const std::string &m) :
     TestStatus(ValuePass, "ok", m) {}



On 14/01/2019 10:46, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Mon, Jan 14, 2019 at 09:54:16AM +0000, Kieran Bingham wrote:
>> Provide Class object to return the test status, and perform any result reporting.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham@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
>>
>> diff --git a/test/libtest/meson.build b/test/libtest/meson.build
>> index e0893b70c3d4..a9e67d8d7e6c 100644
>> --- a/test/libtest/meson.build
>> +++ b/test/libtest/meson.build
>> @@ -1,5 +1,6 @@
>>  libtest_sources = files([
>>      'test.cpp',
>> +    'test_status.cpp',
>>  ])
>>
>>  libtest = static_library('libtest', libtest_sources)
>> diff --git a/test/libtest/test.h b/test/libtest/test.h
>> index ec08bf97c03d..d816cf15aaf0 100644
>> --- a/test/libtest/test.h
>> +++ b/test/libtest/test.h
>> @@ -9,6 +9,8 @@
>>
>>  #include <sstream>
>>
>> +#include "test_status.h"
>> +
>>  enum TestStatus {
>>  	TestPass = 0,
>>  	TestFail = -1,
> 
> When you will replace this, please move it to test_status.h

They already are - they are there as ValuePass. I aim to rename the
class TestStatusPass() to class TestPass() and thus this enum TestStatus
shall then be removed completely.

I've kept it during this RFC to allow both to live side by side for
trying out the new style - and providing comments :)

> 
>> diff --git a/test/libtest/test_status.cpp b/test/libtest/test_status.cpp
>> new file mode 100644
>> index 000000000000..c0daef866740
>> --- /dev/null
>> +++ b/test/libtest/test_status.cpp
>> @@ -0,0 +1,42 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (C) 2019, Google Inc.
>> + *
>> + * test_status.cpp - libcamera test result management
>> + */
>> +
>> +#include "test.h"
>> +
>> +static unsigned int test_number = 0;
>> +
>> +TestStatusBase::TestStatusBase()
>> +	: value(-99)
> 
> -99 ?

I needed a random non-defined number. TestStatusBase shouldn't be
instantiated.


>> +{
>> +	test_number++;
>> +};
>> +
>> +TestStatusBase::~TestStatusBase()
>> +{
>> +	std::cout << prefix << test_number << " " << message << std::endl;
> 
> cout or cerr? This has been asked already bu never finalized

Well the best part about this series is that there will be only one
place to change when we decide :)

cout will be fine for the moment I think.


> 
>> +};
>> +
>> +TestStatusPass::TestStatusPass(const std::string &m)
>> +{
>> +	value = ValuePass;
>> +	prefix = "ok ";
>> +	message = m;
>> +};
>> +
>> +TestStatusFail::TestStatusFail(const std::string &m)
>> +{
>> +	value = ValueFail;
>> +	prefix = "not ok ";
>> +	message = m;
>> +};
>> +
>> +TestStatusSkip::TestStatusSkip(const std::string &m)
>> +{
>> +	value = ValueSkip;
>> +	prefix = "skip ";
>> +	message = m;
>> +};
> 
> Use the base constructor, as noted below in a comment to the header
> file

Thank you for the patch -  I tried to do this through the initialiser
list - and couldn't get it to work correctly.

Now I see I was missing the wrapping for the TestBase() - thanks I'll
update.



> 
>> 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
> 
> Make this and the .cpp one the same
> 
>> + */
>> +#ifndef __TEST_TEST_STATUS_H__
>> +#define __TEST_TEST_STATUS_H__
>> +
>> +#include <iostream>
> 
> You can move this to the cpp file
> 
>> +#include <string>
>> +
>> +class TestStatusBase
>> +{
>> +public:
>> +	TestStatusBase();
>> +	TestStatusBase(const std::string &m);
> 
> TestStatusBase() is not used and shall be deleted
> TestStatusBase(const std::string m) should be protected as this base
> class shall not be instantiated directly

Correct :)


> 
>> +	~TestStatusBase();
>> +
>> +	operator int() { return value; };
>> +
>> +	enum ReturnValues {
>> +		ValuePass = 0,
>> +		ValueFail = -1,
>> +		ValueSkip = 77,
>> +	};
> 
> This and TestStatus can be unified

Not quite - I want to use the TestPass TestSkip, TestFail namings for
the actual object names.


> 
>> +
>> +protected:
>> +	int value;
>> +	std::string prefix;
>> +	std::string message;
> 
> class member end with _
>> +};
>> +
>> +class TestStatusPass : public TestStatusBase
> 
> All of these shall use their base constructor
>> +{
>> +public:
>> +	TestStatusPass(const std::string &m);
>> +};
>> +
>> +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));})
>> +
>> +#endif /* __TEST_TEST_STATUS_H__ */
> 
> Here it is a patch on top that addresses most of my comments and that
> produces this output when run through the test_status_test
> 
> /home/jmondi/project/libcamera/libcamera.git/build/test/test_status
> --- stdout ---
> Test: 1: ok - [Verify TestStatusPass]
> Test: 2: not ok - [Verify TestStatusFail]
> Test: 3: skip - [Verify TestStatusSkip]
> Test: 4: ok - [Good is return check]
> Test: 5: ok - [Good isn't return check]
> Test: 6: not ok - [Bad Is Check]
> Test: 7: not ok - [Bad Isn't check]
> Test: 8: ok - TestStatus validations
> -------
> 
> diff --git a/test/libtest/test_status.cpp b/test/libtest/test_status.cpp
> index c0daef8..be84556 100644
> --- a/test/libtest/test_status.cpp
> +++ b/test/libtest/test_status.cpp
> @@ -5,38 +5,37 @@
>   * test_status.cpp - libcamera test result management
>   */
> 
> +#include <iostream>
> +#include <string>
> +
>  #include "test.h"
> 
>  static unsigned int test_number = 0;
> 
> -TestStatusBase::TestStatusBase()
> -       : value(-99)
> +TestStatusBase::TestStatusBase(enum ReturnValues result,
> +                              const std::string &prefix,
> +                              const std::string &message)
> +       : result_(result), prefix_(prefix), message_(message)
>  {
>         test_number++;
>  };
> 
>  TestStatusBase::~TestStatusBase()
>  {
> -       std::cout << prefix << test_number << " " << message << std::endl;
> +       std::cout << "Test: " << test_number << ": " << prefix_ << " - "
> +                 << message_ << std::endl;
> 
>  };
> 
> -TestStatusPass::TestStatusPass(const std::string &m)
> +TestStatusPass::TestStatusPass(const std::string &m) :
> +       TestStatusBase(ValuePass, "ok", m)
>  {
> -       value = ValuePass;
> -       prefix = "ok ";
> -       message = m;
>  };
> 
> -TestStatusFail::TestStatusFail(const std::string &m)
> +TestStatusFail::TestStatusFail(const std::string &m) :
> +       TestStatusBase(ValueFail, "not ok", m)
>  {
> -       value = ValueFail;
> -       prefix = "not ok ";
> -       message = m;
>  };
> 
> -TestStatusSkip::TestStatusSkip(const std::string &m)
> +TestStatusSkip::TestStatusSkip(const std::string &m) :
> +       TestStatusBase(ValueSkip, "skip", m)
>  {
> -       value = ValueSkip;
> -       prefix = "skip ";
> -       message = m;
>  };
> diff --git a/test/libtest/test_status.h b/test/libtest/test_status.h
> index 2e4713a..7f3bb26 100644
> --- a/test/libtest/test_status.h
> +++ b/test/libtest/test_status.h
> @@ -7,17 +7,14 @@
>  #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; };
> +       operator int() { return result_; };
> 
>         enum ReturnValues {
>                 ValuePass = 0,
> @@ -26,9 +23,12 @@ public:
>         };
> 
>  protected:
> -       int value;
> -       std::string prefix;
>  public:
> -       TestStatusBase();
> -       TestStatusBase(const std::string &m);
>         ~TestStatusBase();
> 
> -       operator int() { return value; };
> +       operator int() { return result_; };
> 
>         enum ReturnValues {
>                 ValuePass = 0,
> @@ -26,9 +23,12 @@ public:
>         };
> 
>  protected:
> -       int value;
> -       std::string prefix;
> -       std::string message;
> +       TestStatusBase() = delete;
> +       TestStatusBase(enum ReturnValues result, const std::string &prefix,
> +                      const std::string &m);

> +       enum ReturnValues result_;

Ah - yes - correct value type is a good call too :)

> +       std::string prefix_;
> +       std::string message_;
>  };
> 
>  class TestStatusPass : public TestStatusBase
> 
> 
>> --
>> 2.17.1
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel@lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Kieran,

On Mon, Jan 14, 2019 at 01:24:49PM +0000, Kieran Bingham wrote:
> Hi Jacopo,
>
> Thanks - Shall I take your detailed review comments as a good indicator
> that you like this direction?
>

Yes I do :)

> I'm sorry it was not so polished, (hence the RFC) - I wanted to gauge
> feel on if this is a good way forwards as much as showing the
> implementation.
>
> That said - you've fixed up the initialiser for the three classes for me :)
>
> So now I can use:
>
> TestPass::TestPass(const std::string &m) :
>      TestStatus(ValuePass, "ok", m) {}

great, but please not my below patches do not respect the "TAP" output
format, I just found out what was about after sending this.

How do we plan to use the TAP formatted output?

Thanks
   j

>
>
>
> On 14/01/2019 10:46, Jacopo Mondi wrote:
> > Hi Kieran,
> >
> > On Mon, Jan 14, 2019 at 09:54:16AM +0000, Kieran Bingham wrote:
> >> Provide Class object to return the test status, and perform any result reporting.
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham@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
> >>
> >> diff --git a/test/libtest/meson.build b/test/libtest/meson.build
> >> index e0893b70c3d4..a9e67d8d7e6c 100644
> >> --- a/test/libtest/meson.build
> >> +++ b/test/libtest/meson.build
> >> @@ -1,5 +1,6 @@
> >>  libtest_sources = files([
> >>      'test.cpp',
> >> +    'test_status.cpp',
> >>  ])
> >>
> >>  libtest = static_library('libtest', libtest_sources)
> >> diff --git a/test/libtest/test.h b/test/libtest/test.h
> >> index ec08bf97c03d..d816cf15aaf0 100644
> >> --- a/test/libtest/test.h
> >> +++ b/test/libtest/test.h
> >> @@ -9,6 +9,8 @@
> >>
> >>  #include <sstream>
> >>
> >> +#include "test_status.h"
> >> +
> >>  enum TestStatus {
> >>  	TestPass = 0,
> >>  	TestFail = -1,
> >
> > When you will replace this, please move it to test_status.h
>
> They already are - they are there as ValuePass. I aim to rename the
> class TestStatusPass() to class TestPass() and thus this enum TestStatus
> shall then be removed completely.
>
> I've kept it during this RFC to allow both to live side by side for
> trying out the new style - and providing comments :)
>
> >
> >> diff --git a/test/libtest/test_status.cpp b/test/libtest/test_status.cpp
> >> new file mode 100644
> >> index 000000000000..c0daef866740
> >> --- /dev/null
> >> +++ b/test/libtest/test_status.cpp
> >> @@ -0,0 +1,42 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> +/*
> >> + * Copyright (C) 2019, Google Inc.
> >> + *
> >> + * test_status.cpp - libcamera test result management
> >> + */
> >> +
> >> +#include "test.h"
> >> +
> >> +static unsigned int test_number = 0;
> >> +
> >> +TestStatusBase::TestStatusBase()
> >> +	: value(-99)
> >
> > -99 ?
>
> I needed a random non-defined number. TestStatusBase shouldn't be
> instantiated.
>
>
> >> +{
> >> +	test_number++;
> >> +};
> >> +
> >> +TestStatusBase::~TestStatusBase()
> >> +{
> >> +	std::cout << prefix << test_number << " " << message << std::endl;
> >
> > cout or cerr? This has been asked already bu never finalized
>
> Well the best part about this series is that there will be only one
> place to change when we decide :)
>
> cout will be fine for the moment I think.
>
>
> >
> >> +};
> >> +
> >> +TestStatusPass::TestStatusPass(const std::string &m)
> >> +{
> >> +	value = ValuePass;
> >> +	prefix = "ok ";
> >> +	message = m;
> >> +};
> >> +
> >> +TestStatusFail::TestStatusFail(const std::string &m)
> >> +{
> >> +	value = ValueFail;
> >> +	prefix = "not ok ";
> >> +	message = m;
> >> +};
> >> +
> >> +TestStatusSkip::TestStatusSkip(const std::string &m)
> >> +{
> >> +	value = ValueSkip;
> >> +	prefix = "skip ";
> >> +	message = m;
> >> +};
> >
> > Use the base constructor, as noted below in a comment to the header
> > file
>
> Thank you for the patch -  I tried to do this through the initialiser
> list - and couldn't get it to work correctly.
>
> Now I see I was missing the wrapping for the TestBase() - thanks I'll
> update.
>
>
>
> >
> >> 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
> >
> > Make this and the .cpp one the same
> >
> >> + */
> >> +#ifndef __TEST_TEST_STATUS_H__
> >> +#define __TEST_TEST_STATUS_H__
> >> +
> >> +#include <iostream>
> >
> > You can move this to the cpp file
> >
> >> +#include <string>
> >> +
> >> +class TestStatusBase
> >> +{
> >> +public:
> >> +	TestStatusBase();
> >> +	TestStatusBase(const std::string &m);
> >
> > TestStatusBase() is not used and shall be deleted
> > TestStatusBase(const std::string m) should be protected as this base
> > class shall not be instantiated directly
>
> Correct :)
>
>
> >
> >> +	~TestStatusBase();
> >> +
> >> +	operator int() { return value; };
> >> +
> >> +	enum ReturnValues {
> >> +		ValuePass = 0,
> >> +		ValueFail = -1,
> >> +		ValueSkip = 77,
> >> +	};
> >
> > This and TestStatus can be unified
>
> Not quite - I want to use the TestPass TestSkip, TestFail namings for
> the actual object names.
>
>
> >
> >> +
> >> +protected:
> >> +	int value;
> >> +	std::string prefix;
> >> +	std::string message;
> >
> > class member end with _
> >> +};
> >> +
> >> +class TestStatusPass : public TestStatusBase
> >
> > All of these shall use their base constructor
> >> +{
> >> +public:
> >> +	TestStatusPass(const std::string &m);
> >> +};
> >> +
> >> +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));})
> >> +
> >> +#endif /* __TEST_TEST_STATUS_H__ */
> >
> > Here it is a patch on top that addresses most of my comments and that
> > produces this output when run through the test_status_test
> >
> > /home/jmondi/project/libcamera/libcamera.git/build/test/test_status
> > --- stdout ---
> > Test: 1: ok - [Verify TestStatusPass]
> > Test: 2: not ok - [Verify TestStatusFail]
> > Test: 3: skip - [Verify TestStatusSkip]
> > Test: 4: ok - [Good is return check]
> > Test: 5: ok - [Good isn't return check]
> > Test: 6: not ok - [Bad Is Check]
> > Test: 7: not ok - [Bad Isn't check]
> > Test: 8: ok - TestStatus validations
> > -------
> >
> > diff --git a/test/libtest/test_status.cpp b/test/libtest/test_status.cpp
> > index c0daef8..be84556 100644
> > --- a/test/libtest/test_status.cpp
> > +++ b/test/libtest/test_status.cpp
> > @@ -5,38 +5,37 @@
> >   * test_status.cpp - libcamera test result management
> >   */
> >
> > +#include <iostream>
> > +#include <string>
> > +
> >  #include "test.h"
> >
> >  static unsigned int test_number = 0;
> >
> > -TestStatusBase::TestStatusBase()
> > -       : value(-99)
> > +TestStatusBase::TestStatusBase(enum ReturnValues result,
> > +                              const std::string &prefix,
> > +                              const std::string &message)
> > +       : result_(result), prefix_(prefix), message_(message)
> >  {
> >         test_number++;
> >  };
> >
> >  TestStatusBase::~TestStatusBase()
> >  {
> > -       std::cout << prefix << test_number << " " << message << std::endl;
> > +       std::cout << "Test: " << test_number << ": " << prefix_ << " - "
> > +                 << message_ << std::endl;
> >
> >  };
> >
> > -TestStatusPass::TestStatusPass(const std::string &m)
> > +TestStatusPass::TestStatusPass(const std::string &m) :
> > +       TestStatusBase(ValuePass, "ok", m)
> >  {
> > -       value = ValuePass;
> > -       prefix = "ok ";
> > -       message = m;
> >  };
> >
> > -TestStatusFail::TestStatusFail(const std::string &m)
> > +TestStatusFail::TestStatusFail(const std::string &m) :
> > +       TestStatusBase(ValueFail, "not ok", m)
> >  {
> > -       value = ValueFail;
> > -       prefix = "not ok ";
> > -       message = m;
> >  };
> >
> > -TestStatusSkip::TestStatusSkip(const std::string &m)
> > +TestStatusSkip::TestStatusSkip(const std::string &m) :
> > +       TestStatusBase(ValueSkip, "skip", m)
> >  {
> > -       value = ValueSkip;
> > -       prefix = "skip ";
> > -       message = m;
> >  };
> > diff --git a/test/libtest/test_status.h b/test/libtest/test_status.h
> > index 2e4713a..7f3bb26 100644
> > --- a/test/libtest/test_status.h
> > +++ b/test/libtest/test_status.h
> > @@ -7,17 +7,14 @@
> >  #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; };
> > +       operator int() { return result_; };
> >
> >         enum ReturnValues {
> >                 ValuePass = 0,
> > @@ -26,9 +23,12 @@ public:
> >         };
> >
> >  protected:
> > -       int value;
> > -       std::string prefix;
> >  public:
> > -       TestStatusBase();
> > -       TestStatusBase(const std::string &m);
> >         ~TestStatusBase();
> >
> > -       operator int() { return value; };
> > +       operator int() { return result_; };
> >
> >         enum ReturnValues {
> >                 ValuePass = 0,
> > @@ -26,9 +23,12 @@ public:
> >         };
> >
> >  protected:
> > -       int value;
> > -       std::string prefix;
> > -       std::string message;
> > +       TestStatusBase() = delete;
> > +       TestStatusBase(enum ReturnValues result, const std::string &prefix,
> > +                      const std::string &m);
>
> > +       enum ReturnValues result_;
>
> Ah - yes - correct value type is a good call too :)
>
> > +       std::string prefix_;
> > +       std::string message_;
> >  };
> >
> >  class TestStatusPass : public TestStatusBase
> >
> >
> >> --
> >> 2.17.1
> >>
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel@lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards
> --
> Kieran

Hi Jacopo,


On 14/01/2019 13:35, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Mon, Jan 14, 2019 at 01:24:49PM +0000, Kieran Bingham wrote:
>> Hi Jacopo,
>>
>> Thanks - Shall I take your detailed review comments as a good indicator
>> that you like this direction?
>>
> 
> Yes I do :)

Great...

> 
>> I'm sorry it was not so polished, (hence the RFC) - I wanted to gauge
>> feel on if this is a good way forwards as much as showing the
>> implementation.
>>
>> That said - you've fixed up the initialiser for the three classes for me :)
>>
>> So now I can use:
>>
>> TestPass::TestPass(const std::string &m) :
>>      TestStatus(ValuePass, "ok", m) {}
> 
> great, but please not my below patches do not respect the "TAP" output
> format, I just found out what was about after sending this.
> 
> How do we plan to use the TAP formatted output?

Not entirely sure yet - we're still in RFC :)

Laurent and Niklas were keen to move in that direction - and either way
- this RFC moves towards making sure each test states what fails when
something goes wrong and introduces a way to start outputting some form
of Logging ... so it's got merit even without something parsing the
results at the end yet....

I assume there are a whole bunch of tools that read TAP.
  (it's not complex anyway)


--
Kieran

> 
> Thanks
>    j
> 
>>
>>
>>
>> On 14/01/2019 10:46, Jacopo Mondi wrote:
>>> Hi Kieran,
>>>
>>> On Mon, Jan 14, 2019 at 09:54:16AM +0000, Kieran Bingham wrote:
>>>> Provide Class object to return the test status, and perform any result reporting.
>>>>
>>>> Signed-off-by: Kieran Bingham <kieran.bingham@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
>>>>
>>>> diff --git a/test/libtest/meson.build b/test/libtest/meson.build
>>>> index e0893b70c3d4..a9e67d8d7e6c 100644
>>>> --- a/test/libtest/meson.build
>>>> +++ b/test/libtest/meson.build
>>>> @@ -1,5 +1,6 @@
>>>>  libtest_sources = files([
>>>>      'test.cpp',
>>>> +    'test_status.cpp',
>>>>  ])
>>>>
>>>>  libtest = static_library('libtest', libtest_sources)
>>>> diff --git a/test/libtest/test.h b/test/libtest/test.h
>>>> index ec08bf97c03d..d816cf15aaf0 100644
>>>> --- a/test/libtest/test.h
>>>> +++ b/test/libtest/test.h
>>>> @@ -9,6 +9,8 @@
>>>>
>>>>  #include <sstream>
>>>>
>>>> +#include "test_status.h"
>>>> +
>>>>  enum TestStatus {
>>>>  	TestPass = 0,
>>>>  	TestFail = -1,
>>>
>>> When you will replace this, please move it to test_status.h
>>
>> They already are - they are there as ValuePass. I aim to rename the
>> class TestStatusPass() to class TestPass() and thus this enum TestStatus
>> shall then be removed completely.
>>
>> I've kept it during this RFC to allow both to live side by side for
>> trying out the new style - and providing comments :)
>>
>>>
>>>> diff --git a/test/libtest/test_status.cpp b/test/libtest/test_status.cpp
>>>> new file mode 100644
>>>> index 000000000000..c0daef866740
>>>> --- /dev/null
>>>> +++ b/test/libtest/test_status.cpp
>>>> @@ -0,0 +1,42 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>>> +/*
>>>> + * Copyright (C) 2019, Google Inc.
>>>> + *
>>>> + * test_status.cpp - libcamera test result management
>>>> + */
>>>> +
>>>> +#include "test.h"
>>>> +
>>>> +static unsigned int test_number = 0;
>>>> +
>>>> +TestStatusBase::TestStatusBase()
>>>> +	: value(-99)
>>>
>>> -99 ?
>>
>> I needed a random non-defined number. TestStatusBase shouldn't be
>> instantiated.
>>
>>
>>>> +{
>>>> +	test_number++;
>>>> +};
>>>> +
>>>> +TestStatusBase::~TestStatusBase()
>>>> +{
>>>> +	std::cout << prefix << test_number << " " << message << std::endl;
>>>
>>> cout or cerr? This has been asked already bu never finalized
>>
>> Well the best part about this series is that there will be only one
>> place to change when we decide :)
>>
>> cout will be fine for the moment I think.
>>
>>
>>>
>>>> +};
>>>> +
>>>> +TestStatusPass::TestStatusPass(const std::string &m)
>>>> +{
>>>> +	value = ValuePass;
>>>> +	prefix = "ok ";
>>>> +	message = m;
>>>> +};
>>>> +
>>>> +TestStatusFail::TestStatusFail(const std::string &m)
>>>> +{
>>>> +	value = ValueFail;
>>>> +	prefix = "not ok ";
>>>> +	message = m;
>>>> +};
>>>> +
>>>> +TestStatusSkip::TestStatusSkip(const std::string &m)
>>>> +{
>>>> +	value = ValueSkip;
>>>> +	prefix = "skip ";
>>>> +	message = m;
>>>> +};
>>>
>>> Use the base constructor, as noted below in a comment to the header
>>> file
>>
>> Thank you for the patch -  I tried to do this through the initialiser
>> list - and couldn't get it to work correctly.
>>
>> Now I see I was missing the wrapping for the TestBase() - thanks I'll
>> update.
>>
>>
>>
>>>
>>>> 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
>>>
>>> Make this and the .cpp one the same
>>>
>>>> + */
>>>> +#ifndef __TEST_TEST_STATUS_H__
>>>> +#define __TEST_TEST_STATUS_H__
>>>> +
>>>> +#include <iostream>
>>>
>>> You can move this to the cpp file
>>>
>>>> +#include <string>
>>>> +
>>>> +class TestStatusBase
>>>> +{
>>>> +public:
>>>> +	TestStatusBase();
>>>> +	TestStatusBase(const std::string &m);
>>>
>>> TestStatusBase() is not used and shall be deleted
>>> TestStatusBase(const std::string m) should be protected as this base
>>> class shall not be instantiated directly
>>
>> Correct :)
>>
>>
>>>
>>>> +	~TestStatusBase();
>>>> +
>>>> +	operator int() { return value; };
>>>> +
>>>> +	enum ReturnValues {
>>>> +		ValuePass = 0,
>>>> +		ValueFail = -1,
>>>> +		ValueSkip = 77,
>>>> +	};
>>>
>>> This and TestStatus can be unified
>>
>> Not quite - I want to use the TestPass TestSkip, TestFail namings for
>> the actual object names.
>>
>>
>>>
>>>> +
>>>> +protected:
>>>> +	int value;
>>>> +	std::string prefix;
>>>> +	std::string message;
>>>
>>> class member end with _
>>>> +};
>>>> +
>>>> +class TestStatusPass : public TestStatusBase
>>>
>>> All of these shall use their base constructor
>>>> +{
>>>> +public:
>>>> +	TestStatusPass(const std::string &m);
>>>> +};
>>>> +
>>>> +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));})
>>>> +
>>>> +#endif /* __TEST_TEST_STATUS_H__ */
>>>
>>> Here it is a patch on top that addresses most of my comments and that
>>> produces this output when run through the test_status_test
>>>
>>> /home/jmondi/project/libcamera/libcamera.git/build/test/test_status
>>> --- stdout ---
>>> Test: 1: ok - [Verify TestStatusPass]
>>> Test: 2: not ok - [Verify TestStatusFail]
>>> Test: 3: skip - [Verify TestStatusSkip]
>>> Test: 4: ok - [Good is return check]
>>> Test: 5: ok - [Good isn't return check]
>>> Test: 6: not ok - [Bad Is Check]
>>> Test: 7: not ok - [Bad Isn't check]
>>> Test: 8: ok - TestStatus validations
>>> -------
>>>
>>> diff --git a/test/libtest/test_status.cpp b/test/libtest/test_status.cpp
>>> index c0daef8..be84556 100644
>>> --- a/test/libtest/test_status.cpp
>>> +++ b/test/libtest/test_status.cpp
>>> @@ -5,38 +5,37 @@
>>>   * test_status.cpp - libcamera test result management
>>>   */
>>>
>>> +#include <iostream>
>>> +#include <string>
>>> +
>>>  #include "test.h"
>>>
>>>  static unsigned int test_number = 0;
>>>
>>> -TestStatusBase::TestStatusBase()
>>> -       : value(-99)
>>> +TestStatusBase::TestStatusBase(enum ReturnValues result,
>>> +                              const std::string &prefix,
>>> +                              const std::string &message)
>>> +       : result_(result), prefix_(prefix), message_(message)
>>>  {
>>>         test_number++;
>>>  };
>>>
>>>  TestStatusBase::~TestStatusBase()
>>>  {
>>> -       std::cout << prefix << test_number << " " << message << std::endl;
>>> +       std::cout << "Test: " << test_number << ": " << prefix_ << " - "
>>> +                 << message_ << std::endl;
>>>
>>>  };
>>>
>>> -TestStatusPass::TestStatusPass(const std::string &m)
>>> +TestStatusPass::TestStatusPass(const std::string &m) :
>>> +       TestStatusBase(ValuePass, "ok", m)
>>>  {
>>> -       value = ValuePass;
>>> -       prefix = "ok ";
>>> -       message = m;
>>>  };
>>>
>>> -TestStatusFail::TestStatusFail(const std::string &m)
>>> +TestStatusFail::TestStatusFail(const std::string &m) :
>>> +       TestStatusBase(ValueFail, "not ok", m)
>>>  {
>>> -       value = ValueFail;
>>> -       prefix = "not ok ";
>>> -       message = m;
>>>  };
>>>
>>> -TestStatusSkip::TestStatusSkip(const std::string &m)
>>> +TestStatusSkip::TestStatusSkip(const std::string &m) :
>>> +       TestStatusBase(ValueSkip, "skip", m)
>>>  {
>>> -       value = ValueSkip;
>>> -       prefix = "skip ";
>>> -       message = m;
>>>  };
>>> diff --git a/test/libtest/test_status.h b/test/libtest/test_status.h
>>> index 2e4713a..7f3bb26 100644
>>> --- a/test/libtest/test_status.h
>>> +++ b/test/libtest/test_status.h
>>> @@ -7,17 +7,14 @@
>>>  #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; };
>>> +       operator int() { return result_; };
>>>
>>>         enum ReturnValues {
>>>                 ValuePass = 0,
>>> @@ -26,9 +23,12 @@ public:
>>>         };
>>>
>>>  protected:
>>> -       int value;
>>> -       std::string prefix;
>>>  public:
>>> -       TestStatusBase();
>>> -       TestStatusBase(const std::string &m);
>>>         ~TestStatusBase();
>>>
>>> -       operator int() { return value; };
>>> +       operator int() { return result_; };
>>>
>>>         enum ReturnValues {
>>>                 ValuePass = 0,
>>> @@ -26,9 +23,12 @@ public:
>>>         };
>>>
>>>  protected:
>>> -       int value;
>>> -       std::string prefix;
>>> -       std::string message;
>>> +       TestStatusBase() = delete;
>>> +       TestStatusBase(enum ReturnValues result, const std::string &prefix,
>>> +                      const std::string &m);
>>
>>> +       enum ReturnValues result_;
>>
>> Ah - yes - correct value type is a good call too :)
>>
>>> +       std::string prefix_;
>>> +       std::string message_;
>>>  };
>>>
>>>  class TestStatusPass : public TestStatusBase
>>>
>>>
>>>> --
>>>> 2.17.1
>>>>
>>>> _______________________________________________
>>>> libcamera-devel mailing list
>>>> libcamera-devel@lists.libcamera.org
>>>> https://lists.libcamera.org/listinfo/libcamera-devel
>>
>> --
>> Regards
>> --
>> Kieran

Hi Kieran,

Thank you for the patch.

On Monday, 14 January 2019 15:24:49 EET Kieran Bingham wrote:
> Hi Jacopo,
> 
> Thanks - Shall I take your detailed review comments as a good indicator
> that you like this direction?
> 
> I'm sorry it was not so polished, (hence the RFC) - I wanted to gauge
> feel on if this is a good way forwards as much as showing the
> implementation.
> 
> That said - you've fixed up the initialiser for the three classes for me :)
> 
> So now I can use:
> 
> TestPass::TestPass(const std::string &m) :
>      TestStatus(ValuePass, "ok", m) {}

That looks much better to me too.

While there is interesting code in this series, it starts to blur the line 
between different test-related concepts without defining them. The Test class 
was originally intended to model a test, and now you introduce some kind of 
sub-test concept. I think this should be documented in order to discuss the 
design, with a clear explanation of what tests, test suites, test cases, unit 
tests, test sets and any other concept we may need is. It's hard to review 
this series without knowing exactly what you have in mind, and I think that 
ad-hoc extensions to the test infrastructure will build a technical debt we'll 
have a hard time repaying.

> On 14/01/2019 10:46, Jacopo Mondi wrote:
> > On Mon, Jan 14, 2019 at 09:54:16AM +0000, Kieran Bingham wrote:
> >> Provide Class object to return the test status, and perform any result
> >> reporting.
> >> 
> >> Signed-off-by: Kieran Bingham <kieran.bingham@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

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@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__ */