[libcamera-devel] [PATCH 6/6] test: Add test case for signal delivery across threads

[Laurent Pinchart]

Hi Niklas,

On Thu, Jul 11, 2019 at 02:53:47PM +0900, Niklas Söderlund wrote:
> On 2019-07-10 22:17:08 +0300, Laurent Pinchart wrote:
> > The test case creates a receiver inheriting from Object, connects a
> > signal to one of its slot, moves the receiver to a different thread,
> > emits the signal and verifies that it gets delivered in the correct
> > thread with the expected value.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  test/meson.build        |   1 +
> >  test/signal-threads.cpp | 125 ++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 126 insertions(+)
> >  create mode 100644 test/signal-threads.cpp
> > 
> > diff --git a/test/meson.build b/test/meson.build
> > index 1f87319aeb65..60ce9601cc55 100644
> > --- a/test/meson.build
> > +++ b/test/meson.build
> > @@ -22,6 +22,7 @@ public_tests = [
> >  internal_tests = [
> >      ['camera-sensor',                   'camera-sensor.cpp'],
> >      ['message',                         'message.cpp'],
> > +    ['signal-threads',                  'signal-threads.cpp'],
> >      ['threads',                         'threads.cpp'],
> >  ]
> >  
> > diff --git a/test/signal-threads.cpp b/test/signal-threads.cpp
> > new file mode 100644
> > index 000000000000..c21f32ae0c20
> > --- /dev/null
> > +++ b/test/signal-threads.cpp
> > @@ -0,0 +1,125 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * signal-threads.cpp - Cross-thread signal delivery test
> > + */
> > +
> > +#include <chrono>
> > +#include <iostream>
> > +#include <thread>
> > +
> > +#include "message.h"
> > +#include "thread.h"
> > +#include "test.h"
> > +#include "utils.h"
> > +
> > +using namespace std;
> > +using namespace libcamera;
> > +
> > +class SignalReceiver : public Object
> > +{
> > +public:
> > +	enum Status {
> > +		NoSignal,
> > +		InvalidThread,
> > +		SignalReceived,
> > +	};
> > +
> > +	SignalReceiver()
> > +		: status_(NoSignal)
> > +	{
> > +	}
> > +
> > +	Status status() const { return status_; }
> > +	int value() const { return value_; }
> > +	void reset()
> > +	{
> > +		status_ = NoSignal;
> > +		value_ = 0;
> > +	}
> > +
> > +	void slot(int value)
> > +	{
> > +		if (Thread::current() != thread())
> > +			status_ = InvalidThread;
> > +		else
> > +			status_ = SignalReceived;
> > +
> > +		value_ = value;
> > +	}
> > +
> > +private:
> > +	Status status_;
> > +	int value_;
> > +};
> > +
> > +class SignalThreadsTest : public Test
> > +{
> > +protected:
> > +	int run()
> > +	{
> > +		SignalReceiver receiver;
> > +		signal_.connect(&receiver, &SignalReceiver::slot);
> > +
> > +		/* Test that a signal is received in the main thread. */
> > +		signal_.emit(42);
> > +
> > +		switch (receiver.status()) {
> > +		case SignalReceiver::NoSignal:
> > +			cout << "No signal received for direct connection" << endl;
> > +			return TestFail;
> > +		case SignalReceiver::InvalidThread:
> > +			cout << "Signal received in incorrect thread "
> > +				"for direct connection" << endl;
> > +			return TestFail;
> > +		default:
> > +			break;
> > +		}
> 
> It's verified in other Signal tests that the argument(s) are delivered 
> but for the sake of us with OCD you might consider adding a 
> receiver.value() != 42 check here ;-)

The purpose of this first test is to make sure that the signal gets
delivered directly when the receiver lives in the same thread as the
sender. More extended value checks are done in the signal.cpp test. The
test below, however, tests that values can be carried across threads, so
it's needed.

> With this fixed,
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> 
> > +
> > +		/*
> > +		 * Move the object to a thread and verify that the signal is
> > +		 * correctly delivered, with the correct data.
> > +		 */
> > +		receiver.reset();
> > +		receiver.moveToThread(&thread_);
> > +
> > +		thread_.start();
> > +
> > +		signal_.emit(42);
> > +
> > +		this_thread::sleep_for(chrono::milliseconds(100));
> > +
> > +		switch (receiver.status()) {
> > +		case SignalReceiver::NoSignal:
> > +			cout << "No signal received for message connection" << endl;
> > +			return TestFail;
> > +		case SignalReceiver::InvalidThread:
> > +			cout << "Signal received in incorrect thread "
> > +				"for message connection" << endl;
> > +			return TestFail;
> > +		default:
> > +			break;
> > +		}
> > +
> > +		if (receiver.value() != 42) {
> > +			cout << "Signal received with incorrect value" << endl;
> > +			return TestFail;
> > +		}
> > +
> > +		return TestPass;
> > +	}
> > +
> > +	void cleanup()
> > +	{
> > +		thread_.exit(0);
> > +		thread_.wait();
> > +	}
> > +
> > +private:
> > +	Thread thread_;
> > +
> > +	Signal<int> signal_;
> > +};
> > +
> > +TEST_REGISTER(SignalThreadsTest)

-- 
Regards,

Laurent Pinchart