[libcamera-devel] [PATCH] test: process: Test Process::kill()

[Laurent Pinchart]

Hi Kieran,

On Mon, Jul 27, 2020 at 02:15:14PM +0100, Kieran Bingham wrote:
> On 27/07/2020 13:53, Laurent Pinchart wrote:
> > Add a test to ensure that Process::kill() on an unstarted process is
> > safe. This aims at ensuring we don't kill other processes unexpectedly.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  test/process/process_test.cpp | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > If you want to try this at home *without* the corresponding fix to the
> > Process class ("libcamera: process: Fix killing innocent processes
> > unexpectedly"), make sure to save all your work first. It's a very
> > effective implementation of an ejection seat.
> > 
> > diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
> > index ce0cc7c972cd..721a7c9d46ff 100644
> > --- a/test/process/process_test.cpp
> > +++ b/test/process/process_test.cpp
> > @@ -51,6 +51,10 @@ protected:
> >  		args.push_back(to_string(exitCode));
> >  		proc_.finished.connect(this, &ProcessTest::procFinished);
> >  
> > +		/* Test that kill() on an unstarted process is safe. */
> > +		proc_.kill();
> > +
> 
> Perhaps this is one test where we should merge the fix *first* :-)

It's merged already :-)

> Do we need to do anything to actually make sure no other process is
> killed? I presume it will become quite self evident, as probably all of
> the test environment might disappear to succinctly fail the test ;-)

Manually checking that no other process was impacted would be difficult.
I think making sure that we don't kill(-1) is enough. That one is very
noticeable :-)

> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> (As long as I don't have to provide a Tested-by: tag...)
> 
> > +		/* Test starting the process and retrieving the exit code. */
> >  		int ret = proc_.start("/proc/self/exe", args);
> >  		if (ret) {
> >  			cerr << "failed to start process" << endl;

-- 
Regards,

Laurent Pinchart