[binutils-gdb] gdb/testsuite: handle invalid .exp names passed in TESTS

[binutils-gdb] gdb/testsuite: handle invalid .exp names passed in TESTS

[Andrew Burgess]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c3d64d467d49edd8ac226679553686034d004c13

commit c3d64d467d49edd8ac226679553686034d004c13
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sat Sep 10 11:10:25 2022 +0100

    gdb/testsuite: handle invalid .exp names passed in TESTS
    
    I ran some tests like:
    
      $ make check-gdb TESTS="gdb.base/break.exp"
    
    then, then I went to rerun the tests later, I managed to corrupt the
    command line, like this:
    
      $ make check-gdb TESTS="gdb.base/breakff.exp"
    
    the make command did exit with an error, but DejaGnu appeared to
    report that every test passed!  The tail end of the output looks like
    this:
    
      Illegal Argument "no-matching-tests-found"
      try "runtest --help" for option list
                    === gdb Summary ===
    
      # of expected passes          115
      /tmp/build/gdb/gdb version  13.0.50.20220831-git -nw -nx -iex "set height 0" -iex "set width 0" -data-directory /tmp/build/gdb/testsuite/../data-directory
    
      make[3]: *** [Makefile:212: check-single] Error 1
      make[3]: Leaving directory '/tmp/build/gdb/testsuite'
      make[2]: *** [Makefile:161: check] Error 2
      make[2]: Leaving directory '/tmp/build/gdb/testsuite'
      make[1]: *** [Makefile:1916: check] Error 2
      make[1]: Leaving directory '/tmp/build/gdb'
      make: *** [Makefile:13565: check-gdb] Error 2
    
    For a while, I didn't spot that DejaGnu had failed at all, I saw the
    115 passes, and thought everything had run correctly - though I was
    puzzled that make was reporting an error.
    
    What happens is that in gdb/testsuite/Makefile, in the check-single
    rule, we first run DejaGnu, then run the dg-add-core-file-count.sh
    script, and finally, we use sed to extract the results from the
    gdb.sum file.
    
    In my case, with the invalid test name, DejaGnu fails, but the
    following steps are still run, the final result, the 115 passes, is
    then extracted from the pre-existing gdb.sum file.
    
    If I use 'make -jN' then the 'check-parallel' rule, rather than the
    'check-single' rule is used.  In this case the behaviour is slightly
    different, the tail end of the output now looks like this:
    
      No matching tests found.
    
      make[4]: Leaving directory '/tmp/build/gdb/testsuite'
      find: ‘outputs’: No such file or directory
      Usage: ../../../src/gdb/testsuite/../../contrib/dg-extract-results.py [-t tool] [-l variant-list] [-L] log-or-sum-file ...
    
          tool           The tool (e.g. g++, libffi) for which to create a
                         new test summary file.  If not specified then output
                         is created for all tools.
          variant-list   One or more test variant names.  If the list is
                         not specified then one is constructed from all
                         variants in the files for <tool>.
          sum-file       A test summary file with the format of those
                         created by runtest from DejaGnu.
          If -L is used, merge *.log files instead of *.sum.  In this
          mode the exact order of lines may not be preserved, just different
          Running *.exp chunks should be in correct order.
      find: ‘outputs’: No such file or directory
      Usage: ../../../src/gdb/testsuite/../../contrib/dg-extract-results.py [-t tool] [-l variant-list] [-L] log-or-sum-file ...
    
          tool           The tool (e.g. g++, libffi) for which to create a
                         new test summary file.  If not specified then output
                         is created for all tools.
          variant-list   One or more test variant names.  If the list is
                         not specified then one is constructed from all
                         variants in the files for <tool>.
          sum-file       A test summary file with the format of those
                         created by runtest from DejaGnu.
          If -L is used, merge *.log files instead of *.sum.  In this
          mode the exact order of lines may not be preserved, just different
          Running *.exp chunks should be in correct order.
      make[3]: Leaving directory '/tmp/build/gdb/testsuite'
      make[2]: Leaving directory '/tmp/build/gdb/testsuite'
      make[1]: Leaving directory '/tmp/build/gdb'
    
    Rather than DejaGnu failing, we now get a nice 'No matching tests
    found' message, followed by some other noise.  This other noise is
    first `find` failing, followed by the dg-extract-results.py script
    failing.
    
    What happens here is that, in the check-parallel rule, the outputs
    directory is deleted before DejaGnu is invoked.  Then we try to run
    all the tests, and finally we use find and dg-extract-results.py to
    combine all the separate .sun and .log files together.  However, if
    there are no tests run then the outputs/ directory is never created,
    so the find command and consequently the dg-extract-results.py script,
    fail.
    
    This commit aims to fix the following issues:
    
     (1) For check-single and check-parallel rules, don't run any of the
     post-processing steps if DejaGnu failed to run.  This will avoid all
     the noise after the initial failure of DejaGnu,
    
     (2) For check-single ensure that we don't accidentally report
     previous results, this is related to the above, but is worth calling
     out as a separate point, and
    
     (3) For check-single, print the 'No matching tests found' message
     just like we do for a parallel test run.  This makes the parallel and
     non-parallel testing behaviour more similar, and I think is clearer
     than the current 'Illegal Argument' error message.
    
    Points (1) and (2) will be handled by moving the post processing steps
    inside an if block within the recipe.  For check-single I propose
    deleting the gdb.sum and gdb.log files before running DejaGnu, this is
    similar (I think) to how we delete the outputs/ directory in the
    check-parallel rule.
    
    For point (3) I plan to split the check-single rule in two, the
    existing check-single will be renamed do-check-single, then a new
    check-single rule will be added.  The new check-single rule can either
    depend on the new do-check-single, or will ensure the 'No matching
    tests found' message is printed when appropriate.

Diff:
---
 gdb/testsuite/Makefile.in | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/gdb/testsuite/Makefile.in b/gdb/testsuite/Makefile.in
index 87ba522c9e0..49b8a60a35c 100644
--- a/gdb/testsuite/Makefile.in
+++ b/gdb/testsuite/Makefile.in
@@ -203,18 +203,36 @@ expanded_tests := $(patsubst $(srcdir)/%,%,$(wildcard $(addprefix $(srcdir)/,$(T
 expanded_tests_or_none := $(or $(expanded_tests),no-matching-tests-found)
 endif
 
+# With check-single, if TESTS was expanded to "no-matching-tests-found" then
+# this will be passed to DejaGnu, resuling in an error.  With check-parallel
+# in the same situation, we avoid invoking DejaGnu, and instead just call
+# the check/no-matching-tests-found rule (which prints a helpful message).
+#
+# To get the same behaviour for check-single we decide here, based on how
+# TESTS expanded, whether check-single should redirect to do-check-single or
+# to check/no-matching-tests-found.
+ifeq ($(expanded_tests_or_none),no-matching-tests-found)
+CHECK_SINGLE_DEP=check/no-matching-tests-found
+else
+CHECK_SINGLE_DEP=do-check-single
+endif
+
 # Shorthand for running all the tests in a single directory.
 check-gdb.%:
 	$(MAKE) check TESTS="gdb.$*/*.exp"
 
-check-single:
-	-rm -f *core*
+do-check-single:
+	-rm -f *core* gdb.sum gdb.log
 	$(DO_RUNTEST) $(RUNTESTFLAGS) $(expanded_tests_or_none) $(TIMESTAMP); \
 	result=$$?; \
-	$(SHELL) $(srcdir)/lib/dg-add-core-file-count.sh; \
-	sed -n '/=== gdb Summary ===/,$$ p' gdb.sum; \
+	if test -e gdb.sum; then \
+	  $(SHELL) $(srcdir)/lib/dg-add-core-file-count.sh; \
+	  sed -n '/=== gdb Summary ===/,$$ p' gdb.sum; \
+	fi; \
 	exit $$result
 
+check-single: $(CHECK_SINGLE_DEP)
+
 check-single-racy:
 	-rm -rf cache racy_outputs temp
 	mkdir -p racy_outputs; \
@@ -240,12 +258,14 @@ check-parallel:
 	-rm -rf cache outputs temp
 	$(MAKE) -k do-check-parallel; \
 	result=$$?; \
-	$(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
-	  `find outputs -name gdb.sum -print` > gdb.sum; \
-	$(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
-	  `find outputs -name gdb.log -print` > gdb.log; \
-	$(SHELL) $(srcdir)/lib/dg-add-core-file-count.sh; \
-	sed -n '/=== gdb Summary ===/,$$ p' gdb.sum; \
+	if test -d outputs; then \
+	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
+	    `find outputs -name gdb.sum -print` > gdb.sum; \
+	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
+	    `find outputs -name gdb.log -print` > gdb.log; \
+	  $(SHELL) $(srcdir)/lib/dg-add-core-file-count.sh; \
+	  sed -n '/=== gdb Summary ===/,$$ p' gdb.sum; \
+	fi; \
 	exit $$result
 
 check-parallel-racy: