[RFAv2,2/2] Add a selftest that checks documentation invariants.
Commit Message
Several approaches were discussed (mail or irc) to verify the invariants of
the GDB help documentation : checking with apropos ., modifying add_cmd
to do the check and output a warning, implement maintenance check-doc.
A selftest was finally chosen as:
* this can be run on demand, including by users if they want
to check user defined commands.
* it does not interact with the normal behaviour of apropos, define,
python, ...
(such as output warnings when a user defines a command help that
does not respect the doc).
* when the selftest runs, it checks the user defined and python
defined commands currently defined.
gdb/ChangeLog
* unittests/help-doc-selftests.c: New file.
* Makefile.in: Add the new file.
---
gdb/Makefile.in | 1 +
gdb/unittests/help-doc-selftests.c | 106 +++++++++++++++++++++++++++++
2 files changed, 107 insertions(+)
create mode 100644 gdb/unittests/help-doc-selftests.c
Comments
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
Philippe> gdb/ChangeLog
Philippe> * unittests/help-doc-selftests.c: New file.
Philippe> * Makefile.in: Add the new file.
Thanks for doing this.
Philippe> +static void
Philippe> +broken_doc_invariant (const char *prefix, const char *name, const char *msg)
Philippe> +{
Philippe> + fprintf_filtered (gdb_stdout,
Philippe> + "help doc broken invariant: command '%s%s' help doc %s\n",
Philippe> + prefix, name, msg);
Normally I'd probably complain about being i18n-unfriendly here, but TBH
I don't think that matters much for unit tests.
Philippe> + /* Walk through the commands. */
Philippe> + for (c=commandlist;c;c=c->next)
This needs some spaces.
Philippe> + while (*p && *p != '\n')
Philippe> + p++;
I think this could just be "p = strchr (p, '\n')".
This is ok with those things fixed.
Tom
On Tue, 2019-08-06 at 12:33 -0600, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
>
> Philippe> gdb/ChangeLog
> Philippe> * unittests/help-doc-selftests.c: New file.
> Philippe> * Makefile.in: Add the new file.
>
> Thanks for doing this.
>
> Philippe> +static void
> Philippe> +broken_doc_invariant (const char *prefix, const char *name, const char *msg)
> Philippe> +{
> Philippe> + fprintf_filtered (gdb_stdout,
> Philippe> + "help doc broken invariant: command '%s%s' help doc %s\n",
> Philippe> + prefix, name, msg);
>
> Normally I'd probably complain about being i18n-unfriendly here, but TBH
> I don't think that matters much for unit tests.
>
> Philippe> + /* Walk through the commands. */
> Philippe> + for (c=commandlist;c;c=c->next)
fixed.
>
> This needs some spaces.
>
> Philippe> + while (*p && *p != '\n')
> Philippe> + p++;
>
> I think this could just be "p = strchr (p, '\n')".
I have kept the code above, as strchr returns NULL if no \n
is found, but it is normal to have a doc with just the
first line (not terminated by a LF).
In this case, we must still check that we have a . before
the terminating null byte.
I have put the following comment to clarify:
/* Position p on the first LF, or on terminating null byte. */
while (*p && *p != '\n')
p++;
>
> This is ok with those things fixed.
I have pushed after having fixed the missing spaces and added
the comment.
Thanks for the reviews
Philippe
Philippe> I have kept the code above, as strchr returns NULL if no \n
Philippe> is found, but it is normal to have a doc with just the
Philippe> first line (not terminated by a LF).
Philippe> In this case, we must still check that we have a . before
Philippe> the terminating null byte.
Thanks, that makes sense.
Tom
@@ -414,6 +414,7 @@ SUBDIR_UNITTESTS_SRCS = \
unittests/environ-selftests.c \
unittests/format_pieces-selftests.c \
unittests/function-view-selftests.c \
+ unittests/help-doc-selftests.c \
unittests/lookup_name_info-selftests.c \
unittests/memory-map-selftests.c \
unittests/memrange-selftests.c \
new file mode 100644
@@ -0,0 +1,106 @@
+/* Self tests for help doc for GDB, the GNU debugger.
+
+ Copyright (C) 2019 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "cli/cli-cmds.h"
+#include "cli/cli-decode.h"
+#include "gdbsupport/selftest.h"
+
+namespace selftests {
+namespace help_doc_tests {
+
+static unsigned int nr_failed_invariants;
+
+/* Report a broken invariant and increments nr_failed_invariants. */
+
+static void
+broken_doc_invariant (const char *prefix, const char *name, const char *msg)
+{
+ fprintf_filtered (gdb_stdout,
+ "help doc broken invariant: command '%s%s' help doc %s\n",
+ prefix, name, msg);
+ nr_failed_invariants++;
+}
+
+/* Recursively walk the commandlist structures, and check doc invariants:
+ - The first line of the doc must end with a '.'.
+ - the doc must not end with a new line.
+ If an invariant is not respected, produce a message and increment
+ nr_failed_invariants.
+ Note that we do not call SELF_CHECK in this function, as we want
+ all commands to be checked before making the test fail. */
+
+static void
+check_doc (struct cmd_list_element *commandlist, const char *prefix)
+{
+ struct cmd_list_element *c;
+
+ /* Walk through the commands. */
+ for (c=commandlist;c;c=c->next)
+ {
+ /* Checks the doc has a first line terminated with a '.'. */
+ const char *p = c->doc;
+
+ while (*p && *p != '\n')
+ p++;
+ if (p == c->doc)
+ broken_doc_invariant
+ (prefix, c->name,
+ "is missing the first line terminated with a '.' character");
+ else if (*(p-1) != '.')
+ broken_doc_invariant
+ (prefix, c->name,
+ "first line is not terminated with a '.' character");
+
+ /* Checks the doc is not terminated with a new line. */
+ if (c->doc[strlen (c->doc) - 1] == '\n')
+ broken_doc_invariant
+ (prefix, c->name,
+ "has a superfluous trailing end of line");
+
+ /* Check if this command has subcommands and is not an
+ abbreviation. We skip checking subcommands of abbreviations
+ in order to avoid duplicates in the output. */
+ if (c->prefixlist != NULL && !c->abbrev_flag)
+ {
+ /* Recursively call ourselves on the subcommand list,
+ passing the right prefix in. */
+ check_doc (*c->prefixlist, c->prefixname);
+ }
+ }
+}
+
+static void
+help_doc_invariants_tests ()
+{
+ nr_failed_invariants = 0;
+ check_doc (cmdlist, "");
+ SELF_CHECK (nr_failed_invariants == 0);
+}
+
+} /* namespace help_doc_tests */
+} /* namespace selftests */
+
+void
+_initialize_help_doc_selftests ()
+{
+ selftests::register_test
+ ("help_doc_invariants",
+ selftests::help_doc_tests::help_doc_invariants_tests);
+}