Re: warnings in unit tests

[Jim Meyering]

On Mon, Jun 7, 2021 at 7:41 PM Bruno Haible <bruno@clisp.org> wrote:
> Hi Jim,
>
> > > > > @@ -183,6 +183,9 @@ main ()
> > > > >        *(volatile int *) (page + 0x678) = 42;
> > > > >        break;
> > > > >      case 3:
> > > > > +#if 6 < __GNUC__
> > > > > +# pragma GCC diagnostic ignored "-Wnull-dereference"
> > > > > +#endif
> > > > >        *(volatile int *) 0 = 42;
> > > > >        break;
> > > > >      case 4:
> > > >
> > > > We shouldn't spend time eliminating warnings from test code.
> > > >
> > > > The goal is to have a good coverage of the lib/* code with unit tests.
> > > > That means, we need to
> > > >   - make it easy to write unit tests,
> > > >   - not make it time-consuming to maintain them.
> > > >
> > > > Eliminating warnings from lib/* code is useful, to avoid bugs in the
> > > > programs. But eliminating warnings from tests/* code goes against the
> > > > goal of increasing test coverage.
> >
> > Hi Bruno,
> > Isn't this code exceptional enough to merit three lines of warning 
> > suppression?
>
> No. Good unit tests do exceptional things, like calling close(-1), passing
> NULL pointers to various functions, and other things that e.g. Coverity
> warns about.
>
> > I would like to continue to use -Werror with most warning options even
> > in test code,
> > as long as the cost is low. This feels like a very low one-time cost.
>
> No, the cost is not low. Dmitry's patch also changed the linkage of 11
> functions. He did so to silence warnings (-Wmissing-prototypes) which are
>   1) not part of '-Wall',
>   2) just pointless for tests [my opinion as maintainer of most of these
>      tests].
>
> I don't want the maintainers of packages that use Gnulib to push costs
> onto Gnulib, when it is just for their personal preference.
>
> The tests in Gnulib are there to evaluate the reliability of Gnulib's
> modules on a particular platform. The more test coverage we have, the
> better.
>
> Warnings in the unit tests are not problems. Test *failures* are problems.
>
> gnulib-tool has options --with-tests and --tests-base, that allow a
> package to ship the Gnulib tests and thus help Gnulib (by having more
> people run the unit tests and report possible test failures).
>
> If a package maintainer insists on having warning-free builds, I kindly
> ask them to not bundle the Gnulib tests any more — because requiring
> warning-free tests from Gnulib is more of an impediment than of a help.
>
> For those package maintainers who are OK to accept warnings in the
> Gnulib tests directory, on the other hand, I repeat my offer to
> automatically add -Wno-error in the tests/Makefile.am of that directory.

I can live without -Wmissing-prototypes in gnulib tests, but I still
remember times where using that option exposed a real bug.

My point about the cost/benefit was regarding that 3-line addition for
a single, deliberate NULL-deref.
That one really does not deserve to quash -Wnull-dereference for all tests.