| Message ID | 20141003205833.6deb3569@zion |
|---|---|
| State | New |
| Headers | show |
The dup2 hack is either cute or horrible (likely both). I don't think we should do anything like that in one random test. Rather, we should settle on a uniform policy for tests and using assert. It could be that we never use assert. Or it could be that we use assert freely and just make test-skeleton.c always do the dup2 hack itself. Or it could be something else I haven't thought of off hand. But let's get a plan. If you'd like to convert these tests to use test-skeleton.c first as a separate change before we figure out how to address the assert issue, that could go in right away. Thanks, Roland
> The dup2 hack is either cute or horrible (likely both). I don't think we > should do anything like that in one random test. Okay. > Rather, we should settle > on a uniform policy for tests and using assert. It could be that we never > use assert. Or it could be that we use assert freely and just make > test-skeleton.c always do the dup2 hack itself. Or it could be something > else I haven't thought of off hand. But let's get a plan. One thing we cannot do is use `dup2' in the skeleton. It might confuse tests that specifically test for output written to stdout/stderr. One such example is 'posix/tst-getopt_long1.c'. > If you'd like to convert these tests to use test-skeleton.c first as a > separate change before we figure out how to address the assert issue, > that could go in right away. Okay. And I see that this itself is a task in the master TODO [1]. I will get on it more thoroughly than just these two tests. In the meanwhile, there are also a lot of tests writing to stderr [2]. If we have the policy on that wrapped up, I will work on it next. I am sort of new here, by the way. Hello, glibc hackers! Cheers, Arjun [1] https://sourceware.org/glibc/wiki/Development_Todo/Master#Use_test-skeleton.c [2] http://fpaste.org/139008/
> One thing we cannot do is use `dup2' in the skeleton. It might confuse tests > that specifically test for output written to stdout/stderr. One such example > is 'posix/tst-getopt_long1.c'. If need be, we could make it depend on some macros you define before including the file. In the one case you cite, it would not break anything. That test uses freopen to redirect stderr at the stdio level, so it never writes to fd 2 at all. So it might well work as a blanket change, but certainly we should think about it more first. > > If you'd like to convert these tests to use test-skeleton.c first as a > > separate change before we figure out how to address the assert issue, > > that could go in right away. > > Okay. And I see that this itself is a task in the master TODO [1]. I will > get on it more thoroughly than just these two tests. That would be a very fine contribution and there shouldn't be much of anything to figure out or discuss to get it done. > In the meanwhile, there are also a lot of tests writing to stderr [2]. If > we have the policy on that wrapped up, I will work on it next. > I am sort of new here, by the way. Hello, glibc hackers! Welcome! We very much appreciate new contributors. Have you completed your copyright assignment paperwork for future contributions to libc? I don't see your name in copyright.list. See https://sourceware.org/glibc/wiki/Contribution%20checklist for everything you should know. Thanks, Roland
On 4 October 2014 02:47, Roland McGrath <roland@hack.frob.com> wrote: > Welcome! We very much appreciate new contributors. Have you completed > your copyright assignment paperwork for future contributions to libc? > I don't see your name in copyright.list. > See https://sourceware.org/glibc/wiki/Contribution%20checklist for > everything you should know. Arjun works for Red Hat, so the blanket copyright assignment ought to cover him. Siddhesh
On 3 October 2014 20:36, Roland McGrath <roland@hack.frob.com> wrote: > The dup2 hack is either cute or horrible (likely both). I don't think we > should do anything like that in one random test. Rather, we should settle > on a uniform policy for tests and using assert. It could be that we never > use assert. Or it could be that we use assert freely and just make > test-skeleton.c always do the dup2 hack itself. Or it could be something > else I haven't thought of off hand. But let's get a plan. One option could be to use a hand-rolled assert function which is what most unit testing libraries do. This would also give scope for adding more esoteric asserts that can output more detail about the failure.
On Mon, Oct 06, 2014 at 09:33:33AM +0100, Will Newton wrote: > On 3 October 2014 20:36, Roland McGrath <roland@hack.frob.com> wrote: > > The dup2 hack is either cute or horrible (likely both). I don't think we > > should do anything like that in one random test. Rather, we should settle > > on a uniform policy for tests and using assert. It could be that we never > > use assert. Or it could be that we use assert freely and just make > > test-skeleton.c always do the dup2 hack itself. Or it could be something > > else I haven't thought of off hand. But let's get a plan. > > One option could be to use a hand-rolled assert function which is what > most unit testing libraries do. This would also give scope for adding > more esoteric asserts that can output more detail about the failure. Agreed, but to be clear, the hand-rolled assert should always exit normally, i.e. not with an abort. An abort precludes any cleanups that may be necessary for a test. In fact asserts that result in such abnormal termination should be actively discouraged. On whether we should redirect stderr to stdout, I am not very excited about it. I don't want to exclude the possibility of wanting to use stderr some time in future for the tests. Siddhesh
diff --git a/wcsmbs/tst-mbrtowc.c b/wcsmbs/tst-mbrtowc.c index 3e1eb72..43601cb 100644 --- a/wcsmbs/tst-mbrtowc.c +++ b/wcsmbs/tst-mbrtowc.c @@ -24,6 +24,7 @@ #include <stdlib.h> #include <string.h> #include <wchar.h> +#include <unistd.h> static int check_ascii (const char *locname); @@ -139,7 +140,7 @@ utf8_test (void) if (!setlocale (LC_CTYPE, locale)) { - fprintf (stderr, "locale '%s' not available!\n", locale); + printf ("locale '%s' not available!\n", locale); exit (1); } @@ -151,11 +152,14 @@ utf8_test (void) } -int -main (void) +static int +do_test (void) { int result = 0; + /* Failed `assert' messages should go to stdout, not stderr. */ + dup2 (STDOUT_FILENO, STDERR_FILENO); + /* Check mapping of ASCII range for some character sets which have ASCII as a subset. For those the wide char generated must have the same value. */ @@ -230,3 +234,6 @@ check_ascii (const char *locname) return res != 0; } + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/wcsmbs/tst-wcrtomb.c b/wcsmbs/tst-wcrtomb.c index 3f052f3..cad343e 100644 --- a/wcsmbs/tst-wcrtomb.c +++ b/wcsmbs/tst-wcrtomb.c @@ -26,8 +26,8 @@ static int check_ascii (const char *locname); -int -main (void) +static int +do_test (void) { int result = 0; @@ -92,3 +92,6 @@ check_ascii (const char *locname) return res != 0; } + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c"