support: Introduce new subdirectory for test infrastructure

On 12/02/2016 08:21 PM, Carlos O'Donell wrote:

> My only subconscious fear is to stay way from anything that looks as
> complicated as TET/VSXGen from the Open Group. Your additional infrastructure
> is nowhere near that point, but we should be conscious of an overall design
> goal which is to remain as small as possible.

But we need something to view changes in subtest failures more easily.

> Have you tested your changes with --enable-hardcoded-path-in-tests and made
> sure nothing breaks?

Ugh.  There was some breakages, fixed in the attached patch.  I had 
dropped the ld.so override accidentally.  I don't see any regressions 
anymore.

> (3) Implementation details.
>
> You need a README.txt which:
>
> * Describes the goals of the infrastructure, this will provide guidance
>   for what we should or should not consider adding.

I think it's premature for that.

> * It should also give an example minimal template for a test that users the
>   infrastructure.

I added a dummy test for that.

> * [optional] List _all_ functions that are part of the support infrastructure
>   in order to give a broad overview of what's available without having to
>   crawl all the headers.

I don't like this because it will introduce additional merge conflicts 
while backporting new tests.  I'm just listing the header files.

> Notes:
>
> Is there any way to avoid needing to list $(libsupport) as a dependency
> of the hand-built DSOs in the test framework? This is not a blocker, just
> a question about how we might make it easier to use these functions from
> all such DSOs without developers having to remember to add it as a dependency.

I don't know where the pattern rule is located which builds test DSOs.

> (b) ChangeLog typo.

Fixed.

> (c) Why do you set errno in xpthread_check_return?

It's needed for %m.  Most libpthread functions do not set errno.

> (d) I noticed that the waitpid in signal_handler has a timeout that is
>     not in any way a function of TIMEOUTFACTOR, and it probably should be.
>     Nothing to do with your changes but I wanted to mention it. If you're
>     already refactoring and running tests it might be nice to make it
>     something that scales with TIMEOUTFACTOR.

I'm not sure the timeout factor should apply in this case.

There are non-async-signal-safe functions in there, too.  If the process 
is stuck in a deadlock, calling exit (not _exit) from the signal handler 
is probably what we want to do.

But I would avoid that scope creep here.

> (e) xpthread_join should take a parameter "canceled" to indicate it should
>     verify the result of pthread_join was PTHREAD_CANCELED, and then this
>     should be used when refactoring to tighten up the expected cancellation
>     results e.g. tst-cancel7.c should be adjusted to use this.

But very few callers will need this, so I don't think it makes sense. 
Maybe we can aim for something like this in the future:

   TEST_VERIFY (xpthread_join (thr) == PTHREAD_CANCELED);

It's slightly more verbose, but you don't need to wonder what the 
Boolean argument to xpthread_join does.

>> +int
>> +create_temp_file (const char *base, char **filename)
>> +{
>> +  char *fname;
>> +  int fd;
>> +
>> +  fname = (char *) xmalloc (strlen (test_dir) + 1 + strlen (base)
>> +			    + sizeof ("XXXXXX"));
>> +  strcpy (stpcpy (stpcpy (stpcpy (fname, test_dir), "/"), base), "XXXXXX");
>
> OK, but ugly IMO. I prefer a more verbose version. #bikeshed

It was this way in the original.  We could just use xasprintf nowadays.

>> +/* This test skeleton is to support running existing tests.  New tests
>> +   should use <support/test-driver.c> instead; see the documentation
>> +   in that file for instructions.  */
>
> OK. Should reference support/README.txt.

I've added a reference to README-testing.c.

I realized that I had to convert another test case (tst-secure-getenv). 
In the attached verion, I renamed signal.h to xsignal.h and thread.h to 
xthread.h, to avoid potential header name conflicts.

Thanks,
Florian

support: Introduce new subdirectory for test infrastructure

Commit Message

Comments

Patch