Re: [PATCH 6/8] tools: make sure that test groups are described in the documentation
From: Amir Goldstein <amir73il@gmail.com>
Date: 2021-09-03 03:38:53
Also in:
fstests
quoted hunk ↗ jump to hunk
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
quoted hunk ↗ jump to hunk
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. 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? Thanks, Amir.