[Avocado-devel] Subprocess termination

[Lucas Meneghel Rodrigues]

I would avoid keeping track of special resources by the test runner itself.
It's the sort of thing that the test writer would be expected to implement
on cleanup procedures.

Implementing said track of subprocesses would be justifiable if we look at
the following perspectives:

1) We want to reduce the amount of boilerplate code in tests. This would
hold true if Radek's use case was very common, like in many occasions you
want to spawn a process in the background and forget about it. It doesn't
seem it is.
2) We want avocado to be very good in avoid leaking resources regardless of
decisions made by test writers.

I can see the point 2) as valid, but again, it's introducing a lot of code
to handle certain resources transparently for users that might not need
that on a regular basis.

On Fri, Feb 17, 2017 at 11:55 AM Amador Pahim <apahim at redhat.com> wrote:

> On Fri, Feb 17, 2017 at 9:42 AM, Radek Duda <rduda at redhat.com> wrote:
> > Good morning folks,
> > Andrei thanks  for this line of code I've forgotten to include it. So the
> > code reads:
> >
> > def run(vt_test, test_params, env):
> >   cmd = "nc -l %s" % test_params['some_port']
> >   nc_process = process.SubProcess(cmd)
> >   nc_process_pid = nc_process.start()
> >   return
> >
> > Amador thanks for your advice, but it doesn't work for me. I need the
> > process to run in the background and not block test flow. That's why I
> used
> > start() method. I tried run() method as well, but the process does not
> go to
> > background and blocked test flow. I think the process should be killed
> > automatically after test finishes.
>
> Oh, now I see what you need.
> Well, I'd recommend to finish the process in tearDown(). Not sure if
> we should make the runner to track the sub-processes created by the
> test... let me open a card to discuss that.
>
>
> >
> > Radek
> >
> > On Thu, Feb 16, 2017 at 5:09 PM, Amador Pahim <apahim at redhat.com> wrote:
> >>
> >> On Thu, Feb 16, 2017 at 5:02 PM, Andrei Stepanov <astepano at redhat.com>
> >> wrote:
> >> > I think
> >> >
> >> > nc_process_pid = nc_process.start()
> >>
> >> In that case, the missing part is the `wait()` (and the possible
> >> timeout handling) which are both present in `run()`. You can call it
> >> directly (with `process.run(cmd)`) instead of keeping the SubProcess
> >> object. Unless you have other reasons to have that object.
> >>
> >> >
> >> > On Thu, Feb 16, 2017 at 4:58 PM, Amador Pahim <apahim at redhat.com>
> wrote:
> >> >>
> >> >> On Thu, Feb 16, 2017 at 4:38 PM, Radek Duda <rduda at redhat.com>
> wrote:
> >> >> > Dear avocado users and developers,
> >> >> > I have made a testcase in which is executed nc process in run
> >> >> > function :
> >> >> > (simplified version):
> >> >> >
> >> >> > from avocado.utils import process
> >> >> >
> >> >> > def run(vt_test, test_params, env):
> >> >> >   cmd = "nc -l %s" % test_params['some_port']
> >> >> >   nc_process = process.SubProcess(cmd)
> >> >> >   return
> >> >> >
> >> >> > after testcase is executed, nc does not terminate and is still
> >> >> > present.
> >> >> > To
> >> >> > avoid this I have to kill the process e.g. by
> >> >> > process.safe_kill(nc_process_pid, signal.SIGKILL)
> >> >>
> >> >> Are you running the process with `nc_process.run()`? It's expected to
> >> >> wait for the process to finish.
> >> >>
> >> >>
> >> >>
> >> >>
> https://github.com/avocado-framework/avocado/blob/master/avocado/utils/process.py#L596-L643
> >> >>
> >> >>
> >> >> >
> >> >> > It is pretty awkward to close the process manually particularly in
> >> >> > case
> >> >> > of
> >> >> > complex testcase code
> >> >> > Shouldn't be the subprocess killed automatically after test exits?
> >> >> > After all its called SubProcess
> >> >> >
> >> >> >
> >> >> > regards,
> >> >> >
> >> >> > Radek Duda
> >> >>
> >> >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20170217/594505ff/attachment.htm>