testsuite process helpers (was: RE: string methods thread safety)

[Farid Zaripov <Fa...@kyiv.vdiweb.com>]

 > -----Original Message-----
 > From: Martin Sebor [mailto:sebor@roguewave.com]
 > Sent: Friday, July 21, 2006 3:34 AM
 > To: stdcxx-dev@incubator.apache.org
 > Subject: Re: string methods thread safety
 >
 > I would move these two functions to a different header/source
 > file, say rw_process.h and process.cpp. We can also move
 > rw_system() to the same files and get rid of system.h and system.cpp.
   Done.

 > > // returns pid of the created process
 > > // returns -1 if failed
 > > // note: argv[0] should be equal to path _TEST_EXPORT rw_pid_t
 > > rw_process_create (const char* path, char* const argv []);
 >
 > I'm not sure about the second argument. Is it the most
 > convenient interface? Wouldn't something closer to
 > rw_system() be easier to use? (If so, you might want to move
 > rw_process_create to the .cpp file and make it static and
 > call it from the new vararg function).
   For a while I implemented the two overloads: the argv overload
(used by string mt test), and the vararg overload. Should we keep
the both overloads? The advantages of the argv overload for string mt 
test I have described in the previous letter (Sent: Wednesday, July 26, 
2006 7:09 PM; Subject: RE: string methods thread safety).

 > > // returns pid of the specified process // returns -1 if failed //
 > > result is a pointer to a buffer where the result code // of the
 > > specified process will be stored, or NULL _TEST_EXPORT rw_pid_t
 > > rw_process_join (rw_pid_t pid, int* result);
 >
 > Okay, this is close to waitpid() that I would suggest to
 > rename it to simply rw_waitpid() :)
   Done.

   The test exercising the rw_process_create() overloads and rw_waitpid()
implemented also (0.process.cpp). This test executes the self with some 
parameters, then the executed copy compares the parameters with expected 
and returns the result. The one problem is confusing rw_error() output 
when I test the expected errno value in cases when rw_process_create() 
and rw_waitpid() should fail. Maybe that cases are not necessary?

   The files are attached.

Farid.

Re: testsuite process helpers (was: RE: string methods thread safety)

[Martin Sebor <se...@roguewave.com>]

Farid Zaripov wrote:
>  > -----Original Message-----
>  > From: Martin Sebor [mailto:sebor@roguewave.com]
>  > Sent: Friday, July 21, 2006 3:34 AM
>  > To: stdcxx-dev@incubator.apache.org
>  > Subject: Re: string methods thread safety
>  >
>  > I would move these two functions to a different header/source
>  > file, say rw_process.h and process.cpp. We can also move
>  > rw_system() to the same files and get rid of system.h and system.cpp.
>   Done.
> 
>  > > // returns pid of the created process
>  > > // returns -1 if failed
>  > > // note: argv[0] should be equal to path _TEST_EXPORT rw_pid_t
>  > > rw_process_create (const char* path, char* const argv []);
>  >
>  > I'm not sure about the second argument. Is it the most
>  > convenient interface? Wouldn't something closer to
>  > rw_system() be easier to use? (If so, you might want to move
>  > rw_process_create to the .cpp file and make it static and
>  > call it from the new vararg function).
>   For a while I implemented the two overloads: the argv overload
> (used by string mt test), and the vararg overload. Should we keep
> the both overloads? The advantages of the argv overload for string mt 
> test I have described in the previous letter (Sent: Wednesday, July 26, 
> 2006 7:09 PM; Subject: RE: string methods thread safety).

Sure, let's keep them both for now and see which one's easier to use.
We can always remove one or the other.

I made a few small changes to process.cpp (e.g., I made the error
messages a bit more descriptive by including the arguments of each
failed call), and committed the result here:
http://svn.apache.org/viewvc?rev=426343&view=rev

Btw., I noticed your RCS $Id$ keywords were missing the closing $
so I added them. Without the trailing $ subversion will not expand
them. Also, we need to remove system.cpp (and also the system.h
header, although that's not causing any problems). I did that
(belatedly) in http://svn.apache.org/viewvc?rev=426354&view=rev
(it should have been done in the same commit as adding the
definition in process.cpp).

> 
>  > > // returns pid of the specified process // returns -1 if failed //
>  > > result is a pointer to a buffer where the result code // of the
>  > > specified process will be stored, or NULL _TEST_EXPORT rw_pid_t
>  > > rw_process_join (rw_pid_t pid, int* result);
>  >
>  > Okay, this is close to waitpid() that I would suggest to
>  > rename it to simply rw_waitpid() :)
>   Done.
> 
>   The test exercising the rw_process_create() overloads and rw_waitpid()
> implemented also (0.process.cpp). This test executes the self with some 
> parameters, then the executed copy compares the parameters with expected 
> and returns the result. The one problem is confusing rw_error() output 
> when I test the expected errno value in cases when rw_process_create() 
> and rw_waitpid() should fail. Maybe that cases are not necessary?

I think it's important to exercise this functionality. We'll need
to come up with a way to silence the errors in these tests. Maybe
via some global variable or something like that.

There is another problem with the test: we cannot assume that 1234
(or any other number we pick out of a hat) is an invalid process id.
If there is a process with that pid our test might never finish (or
fail with a false negative). Which brings up another point: we need
to be able to kill the (child) process in case it hangs, and we need
to exercise the ability to correctly report the signal the child
process exited with, or more generally, its exit status.

Incidentally, there is going to be quite a bit of overlap between
the new exec utility (the replacement for the runall.sh script)
that Andrew has been working on and this functionality, including
the tests for both (since Andrew will be implementing a self test
feature for exec). See
http://svn.apache.org/viewvc?view=rev&revision=425242. You guys
might want to think about and discuss how to reduce the amount of
code duplicated between the two.

Also, as a general comment on the test, it helps to give each test
function a descriptive name (rather than test1, test2, etc. It's
also nice to print out some information (using rw_info()) about
what's being tested.

Martin