[RFC] Big patch to contribute.html

[Joseph S. Myers]

On Mon, 7 Jun 2004, Giovanni Bajo wrote:

> this patch adds two new sections in contribute.html:

contribute.html describes procedural requirements.

codingconventions.html describes conventions as to what are or ideally
would be invariants of the source tree (which won't immediately and
obviously break the build if you fail to follow them, hence the need for
the documentations).

sourcebuild.texi gives the technical descriptions of the structure and
idioms of the testsuites (though it may not always be clear whether
something belongs in the internals manual or in codingconventions.html).

install.texi gives instructions on using the testsuite.

The general (non-GCC-specific) user interface to dg.exp ought to be
documented in the DejaGnu manual, but it seems they aren't.

I.e., almost all of this is good, but not all of it belongs in
contribute.html.

> * A section about testcase preparation. This explains the guidelines people
> should follow while preparing testcases to submit with the patches. It gives a
> brief overview of DejaGnu, and how to use the main dg-* commands, with
> examples. It also explains where to add the testcases, how to test a specific
> testcase and where to find testsuite logs to debug failures. I remember that
> when I started contributing to GCC, I had troubles preparing testcases, I was
> totally unfamiliar with DejaGnu, and I really missed some documentation like
> this.

This seems long enough to unbalance contribute.html.  I suggest generic
dg.exp information goes in the DejaGnu manual, GCC-specific information in
sourcebuild.texi, and that codingconventions.html have a link to these
(and possibly to your tutorial as a separate page if it still seems
desirable to have it as a coherent tutorial).

> +<p>Every patch submitted for approval should always come with a testcase
> +to be added to the testsuite (there are a few exceptions that are listed
> +below). The testcase should be able to reproduce the bug on all
> +affected platforms, and it is used to make sure that the bug stays fixed.</p>

Not all testcases are for bugs - they are for new features as well.

There also may be a terminological confusion; does "testcase" refer to a
file, or to a single test assertion in such a file (that a particular
error does/does not occur, etc.)?

Note that the following conventions probably belong in
codingconventions.html.

> + <li>The testcase should be as small as possible. Most bugs can be
> + minimized to a hanful of lines of code, trying hard enough. Usually,
> + volunteers take care of the minimization if the bug is reported in the
> + <a href="http://gcc.gnu.org/bugzilla/">bug database</a>.</li>

For feature testcases, however, they should err on the side of being long;  
covering as many cases as possible that might be relevant to the issue,
even if with the present code they follow identical paths.

> + <li>The testcase should be atomic. If your patch fixes a bug which can be
> + reproduced with different code sequences (or a family of slightly similar
> + bugs), it is better to prepare several different testcases instead of a
> + longer one.</li>

Do you really think gcc.dg/typespec-1.c would have been better as 921
separate files?  (Most cases there never were bugs, but it does a
logically complete test of all the cases for a particular issue, and there
are 921 of them, automatically generated.  921 separate files might avoid
triggering expect bugs that truncate output, but I can't think it would be
an improvement otherwise.)

> + <li>The testcase should not rely on external files (such as headers, even
> + standard headers), if possible. It is better to manually bring the
> + declarations of the needed functions into scope. For instance,
> + instead of:

The C headers that GCC includes, such as <limits.h> and <stddef.h>, are
fine for testcases to include.

> + <li>There are a few excuses for not preparing a testcase. First, it is
> + not strictly necessary for a bug which occurred during GCC bootstrap,
> + as GCC itself is considered a big testcase. Second, there are situations

But small ones are still useful (GCC may cease to have the problem code).

> + where it is very hard to minimize a bug (a weird garbage collector
> + crash, a very complex C++ testcase causing a failure somewhere deep
> + into RTL optimizers, etc.). Third, a testcase could obviously be alrady
> + present and just XFAIL-ed (see below), in which case it is sufficient to
> + remove the <code>xfail</code> mark.</li>

Fourth, the testcase may not be writable in the present framework (there
seems to be no simple way to test for duplicated error messages).  Fifth,
it might be from a proprietary testsuite (so if you've seen the case,
already small, writing one that isn't a copy may be hard; but I don't know
if there is any guidance from Eben Moglen on this case).

> + not suited for the torture testsuite. The following is a list of all
> + the active directories in the testsuite:

Directories and their contents are documented in sourcebuild.texi (or
should be).  (The miscellaneous READMEs are somewhat obsolescent but not
yet merged into the manual, and some people object to merging them in on
GFDL grounds.)

> +<p>The main commands that can be used are the following:</p>

Clearly for DejaGnu manual or (where GCC-specific) sourcebuild.texi,
though this as a tutorial *in addition* may be useful.

> +<p>The following testcase shows an example of this:</p>
> +
> +<pre><code>/* { dg-do link } */
> +/* { dg-options "-O2" } */
> +
> +void link_error(void);

A bad example, since this sort of thing belongs in the torture framework
(which includes gcc.dg/torture), run at multiple options and define
link_error if not __OPTIMIZE__ rather than just run at one option.

-- 
Joseph S. Myers
jsm@polyomino.org.uk