tests: kdoc: improve comparison and output

Currently, the kdoc test fails when the number of lines of outout from
./scripts/kernel-doc increases. This is a decent heuristic but can fail in
some cases. For example, if a patch adds one warning but removes another,
the total line count may not change. This could result in false negatives
where some patches do not fail the test despite adding new warnings.

Upon detecting a difference in warning count, the test uses diff to compare
the old and new warnings. This output is not very useful to humans. The
diff comparison includes the line numbers which are impacted by code
movement, making relevant warnings difficult to spot when reviewing test
output.

An ideal kdoc test should determine the actual set of new warnings. To
achieve this, use the following algorithm:

1) Capture the kernel-doc output for the old commit
2) Capture the kernel-doc output for the new commit
3) Carefully join and sort output so that its stable w.r.t line number changes
4) Strip line numbers from the warning content for both old and new output
5) Diff the stripped output, using --*-group-format to just print line
   number ranges.
6) Use the difference line ranges to print from the unstripped output,
   generating a set of new and removed warnings.

By ensuring that the sorted input is stable even if line numbers change,
this correctly locates all of the new warnings, and additionally any
removed warnings, avoiding a false test pass.

In addition, the output is far more useful for humans as the output is 1)
just the new warnings, and doesn't contain diff syntax and 2) doesn't
contain warnings whose only change is the line number.

This is a bit more complicated, especially since it requires reading
through the input and sorting multiple times. However, the end result is
significantly more useful.

Signed-off-by: Jacob Keller <[email protected]>

Author: jacob-keller

tests/patch/kdoc/kdoc.sh

@@ -4,8 +4,41 @@
 # Copyright (C) 2019 Netronome Systems, Inc.
 # Copyright (c) 2020 Facebook
 
+# Cleanup warnings by merging lines which end with ':' to their referenced
+# line content, and sorting the output carefully to ensure that the entries
+# are stable w.r.t code being re-ordered.
+#
+# In particular, we sort first by the warning description and then by the
+# filename path without its line number element.
+#
+# This assumes all lines are "Warning:" or "Error:" and that file names do not
+# have whitespace.
+function merge_and_sort() {
+  awk '{ if ($0 ~ /:$/) { ORS="" } else { ORS="\n" }; print }' | \
+    sort -s -k 3 | sort -s -t ':' -k 2,2
+}
+
+# Strip out line number information from warnings and errors
+function strip_line_numbers() {
+  sed -e 's@^\(\([Ww]arning\|[Ee]rror\):\s*[/a-zA-Z0-9_.-]*.[ch]\):[0-9]*@\1@' "${@}"
+}
+
+# Diff output, printing a sed range expression for any removed lines.
+function old_lines_to_range() {
+  diff --line-format="" --old-group-format="%df,%dlp;" "${@}"
+}
+
+# Diff output, printing a sed range expression for any newly added lines.
+function new_lines_to_range() {
+  diff --line-format="" --new-group-format="%dF,%dLp;" "${@}"
+}
+
 tmpfile_o=$(mktemp)
 tmpfile_n=$(mktemp)
+tmpfile_so=$(mktemp)
+tmpfile_sn=$(mktemp)
+tmpfile_rm=$(mktemp)
+tmpfile_new=$(mktemp)
 rc=0
 
 files_mod=$(git show --diff-filter=M  --pretty="" --name-only "HEAD")
@@ -15,38 +48,58 @@ HEAD=$(git rev-parse HEAD)
 
 echo "Checking the tree before the patch"
 git checkout -q HEAD~
-./scripts/kernel-doc -Wall -none $files_mod 2> >(tee $tmpfile_o >&2)
+
+echo "Before patch:" >&2
+./scripts/kernel-doc -Wall -none $files_mod 2>&1 | merge_and_sort | tee $tmpfile_o >&2
+
+strip_line_numbers $tmpfile_o > $tmpfile_so
 
 incumbent=$(grep -v 'Error: Cannot open file ' $tmpfile_o | wc -l)
 
 echo "Checking the tree with the patch"
 
 git checkout -q $HEAD
-./scripts/kernel-doc -Wall -none $files_all 2> >(tee $tmpfile_n >&2)
+
+echo "After patch:" >&2
+./scripts/kernel-doc -Wall -none $files_all 2>&1 | merge_and_sort | tee $tmpfile_n >&2
+
+strip_line_numbers $tmpfile_n > $tmpfile_sn
 
 current=$(grep -v 'Error: Cannot open file ' $tmpfile_n | wc -l)
 
-echo "Errors and warnings before: $incumbent this patch: $current" >&$DESC_FD
+# Find removed warnings
+removed_range=$(old_lines_to_range ${tmpfile_so} ${tmpfile_sn})
+sed -n -e "${removed_range}" ${tmpfile_o} > ${tmpfile_rm}
+
+removed=$(grep -v 'Error: Cannot open file ' $tmpfile_rm | wc -l)
+
+# Find new warnings
+new_range=$(new_lines_to_range ${tmpfile_so} ${tmpfile_sn})
+sed -n -e "${new_range}" ${tmpfile_n} > ${tmpfile_new}
 
-if [ $current -gt $incumbent ]; then
-  echo "New warnings added" 1>&2
-  diff $tmpfile_o $tmpfile_n 1>&2
+new_warnings=$(grep -v 'Error: Cannot open file ' $tmpfile_new | wc -l)
 
-  echo "Per-file breakdown" 1>&2
-  tmpfile_fo=$(mktemp)
-  tmpfile_fn=$(mktemp)
+echo "Errors and warnings before: $incumbent This patch: $current" >&$DESC_FD
+echo "New: $new_warnings Removed: $removed" >&$DESC_FD
+
+if [ $removed -gt 0 ]; then
+  echo "Warnings removed:" 1>&2
+  sort -V $tmpfile_rm 1>&2
+
+  echo "Per-file breakdown:" 1>&2
+  grep -i "\(warn\|error\)" $tmpfile_rm | sed -n 's@^\([Ww]arning\|[Ee]rror\):\s*\([/a-zA-Z0-9_.-]*.[ch]\):.*@\2@p' | sort | uniq -c 1>&2
+fi
 
-  grep -i "\(warn\|error\)" $tmpfile_o | sed -n 's@\(^[/a-zA-Z0-9_.-]*.[ch]\):.*@\1@p' | sort | uniq -c \
-    > $tmpfile_fo
-  grep -i "\(warn\|error\)" $tmpfile_n | sed -n 's@\(^[/a-zA-Z0-9_.-]*.[ch]\):.*@\1@p' | sort | uniq -c \
-    > $tmpfile_fn
+if [ $new_warnings -gt 0 ]; then
+  echo "New warnings added:" 1>&2
+  sort -V $tmpfile_new 1>&2
 
-  diff $tmpfile_fo $tmpfile_fn 1>&2
-  rm $tmpfile_fo $tmpfile_fn
+  echo "Per-file breakdown:" 1>&2
+  grep -i "\(warn\|error\)" $tmpfile_new | sed -n 's@^\([Ww]arning\|[Ee]rror\):\s*\([/a-zA-Z0-9_.-]*.[ch]\):.*@\2@p' | sort | uniq -c 1>&2
 
   rc=1
 fi
 
-rm $tmpfile_o $tmpfile_n
+rm $tmpfile_o $tmpfile_n $tmpfile_so $tmpfile_sn
 
 exit $rc