fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Eryu Guan <guaneryu@gmail.com>,
	linux-xfs <linux-xfs@vger.kernel.org>,
	fstests <fstests@vger.kernel.org>, Eryu Guan <guan@eryu.me>
Subject: Re: [PATCH 6/8] tools: make sure that test groups are described in the documentation
Date: Fri, 3 Sep 2021 18:29:46 -0700	[thread overview]
Message-ID: <20210904012946.GD9911@magnolia> (raw)
In-Reply-To: <CAOQ4uxit3G=0o3nXVFvW740v6Xi-pSn5uHsgKdOvH4ybc+3jKw@mail.gmail.com>

On Fri, Sep 03, 2021 at 06:38:38AM +0300, Amir Goldstein wrote:
> > diff --git a/include/buildgrouplist b/include/buildgrouplist
> > index d898efa3..489de965 100644
> > --- a/include/buildgrouplist
> > +++ b/include/buildgrouplist
> > @@ -6,3 +6,4 @@
> >  group.list:
> >         @echo " [GROUP] $$PWD/$@"
> >         $(Q)$(TOPDIR)/tools/mkgroupfile $@
> > +       $(Q)$(TOPDIR)/tools/check-groups $(TOPDIR)/doc/group-names.txt $@
> 
> I would like to argue against checking groups post mkgroupfile
> and for checking groups during mkgroupfile

Done.

> > diff --git a/tools/check-groups b/tools/check-groups
> > new file mode 100755
> > index 00000000..0d193615
> > --- /dev/null
> > +++ b/tools/check-groups
> > @@ -0,0 +1,35 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2021 Oracle.  All Rights Reserved.
> > +#
> > +# Make sure that all groups listed in a group.list file are mentioned in the
> > +# group description file.
> > +
> > +if [ -z "$1" ] || [ "$1" = "--help" ]; then
> > +       echo "Usage: $0 path_to_group_names [group.list files...]"
> > +       exit 1
> > +fi
> > +
> > +groups_doc_file="$1"
> > +shift
> > +
> > +get_group_list() {
> > +       for file in "$@"; do
> > +               while read testname groups; do
> > +                       test -z "${testname}" && continue
> > +                       test "${testname:0:1}" = "#" && continue
> > +
> > +                       echo "${groups}" | tr ' ' '\n'
> > +               done < "${file}"
> > +       done | sort | uniq
> > +}
> > +
> > +ret=0
> > +while read group; do
> > +       if ! grep -q "^${group}[[:space:]]" "${groups_doc_file}"; then
> > +               echo "${group}: group not mentioned in documentation." 1>&2
> 
> This message would have been more informative with the offending
> test file.

Hm.  This becomes much easier if I make the _begin_fstest helper do the
checking of the group names.

> Now after you crunched all the test files into group.list files and
> all the group.list files into a unique group set, this is too late.
> But this same check during generate_groupfile() would have
> been trivial and would allow reporting the offending test.
> 
> While we are on the subject of generate_groupfile(), can you please
> explain the rationale behind the method of extracting the test file
> groups by executing the test with GENERATE_GROUPS=yes?
> As opposed to just getting the list of groups on the stop from the file
> using grep?

Well... now that you point that out, it's so that we can put in custom
logic like checking group names. ;)

--D

> 
> Thanks,
> Amir.

  reply	other threads:[~2021-09-04  1:29 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02 23:52 [PATCHSET v2 0/8] fstests: document all test groups Darrick J. Wong
2021-09-02 23:52 ` [PATCH 1/8] ceph: re-tag copy_file_range as being in the copy_range group Darrick J. Wong
2021-09-02 23:52 ` [PATCH 2/8] xfs: move reflink tests into the clone group Darrick J. Wong
2021-09-02 23:52 ` [PATCH 3/8] xfs: fix incorrect fuzz test group name Darrick J. Wong
2021-09-02 23:52 ` [PATCH 4/8] btrfs: fix incorrect subvolume " Darrick J. Wong
2021-09-02 23:52 ` [PATCH 5/8] generic/631: change this test to use the 'whiteout' group Darrick J. Wong
2021-09-02 23:52 ` [PATCH 6/8] tools: make sure that test groups are described in the documentation Darrick J. Wong
2021-09-03  3:38   ` Amir Goldstein
2021-09-04  1:29     ` Darrick J. Wong [this message]
2021-09-04  3:06   ` [PATCH v2.1 " Darrick J. Wong
2021-09-04  8:52     ` Amir Goldstein
2021-09-13 19:03       ` Darrick J. Wong
2021-09-02 23:53 ` [PATCH 7/8] tools: add missing license tags to my scripts Darrick J. Wong
2021-09-02 23:53 ` [PATCH 8/8] new: only allow documented test group names Darrick J. Wong
2021-09-04  8:43   ` Amir Goldstein
2021-09-13 19:11     ` Darrick J. Wong
2021-09-17  0:39 [PATCHSET v4 0/8] fstests: document all test groups Darrick J. Wong
2021-09-17  0:39 ` [PATCH 6/8] tools: make sure that test groups are described in the documentation Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210904012946.GD9911@magnolia \
    --to=djwong@kernel.org \
    --cc=amir73il@gmail.com \
    --cc=fstests@vger.kernel.org \
    --cc=guan@eryu.me \
    --cc=guaneryu@gmail.com \
    --cc=linux-xfs@vger.kernel.org \
    --subject='Re: [PATCH 6/8] tools: make sure that test groups are described in the documentation' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).