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

[Laurent Pinchart]

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 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

-- 
Regards,

Laurent Pinchart