public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Add selftests run filtering
@ 2017-09-05 11:51 Simon Marchi
  2017-09-06 15:25 ` Pedro Alves
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2017-09-05 11:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

With the growing number of selftests, I think it would be useful to be
able to run only a subset of the tests.  This patch associates a name to
each registered selftest.  It then allows doing something like:

  (gdb) maintenance selftest aarch64
  Running self-tests.
  Running selftest aarch64-analyze-prologue.
  Running selftest aarch64-process-record.
  Ran 2 unit tests, 0 failed

which will only run the tests that contain "aarch64" in their name.  To
help validate that the tests you want to run were actually ran, it also
prints a message with the test name before running each test.

Right now, the arch-dependent tests are registered as a single test of
the selftests.  To be able to filter those too, I made them completely
separate.  run_tests and run_tests_with_arch now return their results,
and maintenance_selftest takes care of printing the combined results.

There aren't many selftests in gdbserver yet, but it wasn't too hard to
make it accept a filter too.  It can be given with --selftest=FILTER.

gdb/ChangeLog:

	* common/selftest.h (struct selftests_results): New struct.
	(register_test): Add name parameter.
	(run_tests): Add filter parameter.
	* common/selftest.c (tests): Change type to std::map.
	(register_test): Add name parameter and use it.
	(run_tests): Add filter parameter and use it.  Add prints.
	Adjust to vector -> map change.  Return results.
	* aarch64-tdep.c (_initialize_aarch64_tdep): Add names when
	registering selftests.
	* arm-tdep.c (_initialize_arm_tdep): Likewise.
	* disasm-selftests.c (_initialize_disasm_selftests): Likewise.
	* dwarf2-frame.c (_initialize_dwarf2_frame): Likewise.
	* dwarf2loc.c (_initialize_dwarf2loc): Likewise.
	* findvar.c (_initialize_findvar): Likewise.
	* gdbarch-selftests.c (_initialize_gdbarch_selftests): Likewise.
	* maint.c: Include "selftest-arch.h".
	(maintenance_selftest): Call run_tests_with_arch.  Print
	results.
	* regcache.c (_initialize_regcache): Add names when registering
	selftests.
	* rust-exp.y (_initialize_rust_exp): Likewise.
	* selftest-arch.c: Include <map>.
	(gdbarch_tests): Change type to std::map.
	(register_test_foreach_arch): Add name parameter and use it.
	(run_tests_with_arch): Add filter parameter and use it.  Add
	prints.  Adjust to vector -> map change.  Return results.
	(_initialize_selftests_foreach_arch): Remove.
	* selftest-arch.h (register_test_foreach_arch): Add name
	parameter.
	(run_tests_with_arch): New declaration.
	* utils-selftests.c (_initialize_utils_selftests): Add names
	when registering selftests.
	* utils.c (_initialize_utils): Likewise.
	* unittests/array-view-selftests.c
	(_initialize_array_view_selftests): Likewise.
	* unittests/environ-selftests.c (_initialize_environ_selftests):
	Likewise.
	* unittests/function-view-selftests.c
	(_initialize_function_view_selftests): Likewise.
	* unittests/offset-type-selftests.c
	(_initialize_offset_type_selftests): Likewise.
	* unittests/optional-selftests.c
	(_initialize_optional_selftests): Likewise.
	* unittests/scoped_restore-selftests.c
	(_initialize_scoped_restore_selftests): Likewise.

gdb/gdbserver/ChangeLog:

	* server.c (captured_main): Accept argument for --selftest.
	Update run_tests call, print test results.
	* linux-x86-tdesc-selftest.c (initialize_low_tdesc): Add names
	when registering selftests.
---
 gdb/aarch64-tdep.c                       |  6 +++--
 gdb/arm-tdep.c                           |  2 +-
 gdb/common/selftest.c                    | 32 ++++++++++++++---------
 gdb/common/selftest.h                    | 19 +++++++++++---
 gdb/disasm-selftests.c                   |  6 +++--
 gdb/dwarf2-frame.c                       |  3 ++-
 gdb/dwarf2loc.c                          |  2 +-
 gdb/findvar.c                            |  4 ++-
 gdb/gdbarch-selftests.c                  |  3 ++-
 gdb/gdbserver/linux-x86-tdesc-selftest.c |  4 +--
 gdb/gdbserver/server.c                   | 15 ++++++++++-
 gdb/maint.c                              |  8 +++++-
 gdb/regcache.c                           |  3 ++-
 gdb/rust-exp.y                           |  2 +-
 gdb/selftest-arch.c                      | 44 +++++++++++++++++---------------
 gdb/selftest-arch.h                      | 10 +++++++-
 gdb/unittests/array-view-selftests.c     |  3 ++-
 gdb/unittests/environ-selftests.c        |  3 ++-
 gdb/unittests/function-view-selftests.c  |  3 ++-
 gdb/unittests/offset-type-selftests.c    |  2 +-
 gdb/unittests/optional-selftests.c       |  2 +-
 gdb/unittests/scoped_restore-selftests.c |  3 ++-
 gdb/utils-selftests.c                    |  2 +-
 gdb/utils.c                              |  2 +-
 24 files changed, 122 insertions(+), 61 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 5a627a3..ebd3e47 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3068,8 +3068,10 @@ When on, AArch64 specific debugging is enabled."),
 			    &setdebuglist, &showdebuglist);
 
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::aarch64_analyze_prologue_test);
-  selftests::register_test (selftests::aarch64_process_record_test);
+  selftests::register_test ("aarch64-analyze-prologue",
+			    selftests::aarch64_analyze_prologue_test);
+  selftests::register_test ("aarch64-process-record",
+			    selftests::aarch64_process_record_test);
 #endif
 }
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 0c1a0b3..751ee27 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9727,7 +9727,7 @@ vfp - VFP co-processor."),
 			   &setdebuglist, &showdebuglist);
 
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::arm_record_test);
+  selftests::register_test ("arm-record", selftests::arm_record_test);
 #endif
 
 }
diff --git a/gdb/common/selftest.c b/gdb/common/selftest.c
index 0fb8f2a..ac34977 100644
--- a/gdb/common/selftest.c
+++ b/gdb/common/selftest.c
@@ -20,39 +20,48 @@
 #include "common-exceptions.h"
 #include "common-debug.h"
 #include "selftest.h"
-#include <vector>
+#include <map>
 
 namespace selftests
 {
 
 /* All the tests that have been registered.  */
 
-static std::vector<self_test_function *> tests;
+static std::map<std::string, self_test_function *> tests;
 
 /* See selftest.h.  */
 
 void
-register_test (self_test_function *function)
+register_test (const std::string &name, self_test_function *function)
 {
-  tests.push_back (function);
+  /* Make sure we don't already have a test with this name.  */
+  gdb_assert (tests.find (name) == tests.end ());
+
+  tests[name] = function;
 }
 
 /* See selftest.h.  */
 
-void
-run_tests (void)
+selftests_results
+run_tests (const char *filter)
 {
-  int failed = 0;
+  selftests_results results;
 
-  for (int i = 0; i < tests.size (); ++i)
+  for (const auto &test : tests)
     {
+      if (filter != NULL && *filter != '\0'
+	  && test.first.find (filter) == std::string::npos)
+	continue;
+
       TRY
 	{
-	  tests[i] ();
+	  debug_printf (_("Running selftest %s.\n"), test.first.c_str ());
+	  results.ran++;
+	  test.second ();
 	}
       CATCH (ex, RETURN_MASK_ERROR)
 	{
-	  ++failed;
+	  results.failed++;
 	  debug_printf ("Self test failed: %s\n", ex.message);
 	}
       END_CATCH
@@ -60,7 +69,6 @@ run_tests (void)
       reset ();
     }
 
-  debug_printf ("Ran %lu unit tests, %d failed\n",
-		(long) tests.size (), failed);
+  return results;
 }
 } // namespace selftests
diff --git a/gdb/common/selftest.h b/gdb/common/selftest.h
index e211c34..ece5da2 100644
--- a/gdb/common/selftest.h
+++ b/gdb/common/selftest.h
@@ -26,15 +26,26 @@ typedef void self_test_function (void);
 
 namespace selftests
 {
+struct selftests_results
+{
+  /* Number of test cases executed.  */
+  int ran = 0;
+
+  /* Number of test cases that failed.  */
+  int failed = 0;
+};
 
 /* Register a new self-test.  */
 
-extern void register_test (self_test_function *function);
+extern void register_test (const std::string &name,
+			   self_test_function *function);
+
+/* Run all the architecture-agnostic self tests.
 
-/* Run all the self tests.  This print a message describing the number
-   of test and the number of failures.  */
+   If FILTER is not NULL and not empty, only tests with names containing FILTER
+   will be ran.  */
 
-extern void run_tests (void);
+extern selftests_results run_tests (const char *filter);
 
 /* Reset GDB or GDBserver's internal state.  */
 extern void reset ();
diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index 4d38ccf..b8ef381 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -214,7 +214,9 @@ void
 _initialize_disasm_selftests (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test_foreach_arch (selftests::print_one_insn_test);
-  selftests::register_test_foreach_arch (selftests::memory_error_test);
+  selftests::register_test_foreach_arch ("print_one_insn",
+					 selftests::print_one_insn_test);
+  selftests::register_test_foreach_arch ("memory_error",
+					 selftests::memory_error_test);
 #endif
 }
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index aaf3aee..268ecda 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -2406,6 +2406,7 @@ _initialize_dwarf2_frame (void)
   dwarf2_frame_objfile_data = register_objfile_data ();
 
 #if GDB_SELF_TEST
-  selftests::register_test_foreach_arch (selftests::execute_cfa_program_test);
+  selftests::register_test_foreach_arch ("execute_cfa_program",
+					 selftests::execute_cfa_program_test);
 #endif
 }
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 1a1b06a..6dd962d 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -4687,6 +4687,6 @@ _initialize_dwarf2loc (void)
 			     &setdebuglist, &showdebuglist);
 
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::copy_bitwise_tests);
+  selftests::register_test ("copy_bitwise", selftests::copy_bitwise_tests);
 #endif
 }
diff --git a/gdb/findvar.c b/gdb/findvar.c
index de6b6ed..0787e74 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -1095,6 +1095,8 @@ void
 _initialize_findvar (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::findvar_tests::copy_integer_to_size_test);
+  selftests::register_test (
+    "copy_integer_to_size",
+    selftests::findvar_tests::copy_integer_to_size_test);
 #endif
 }
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index cb15964..f0b8d5d 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -151,6 +151,7 @@ void
 _initialize_gdbarch_selftests (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test_foreach_arch (selftests::register_to_value_test);
+  selftests::register_test_foreach_arch ("register_to_value",
+					 selftests::register_to_value_test);
 #endif
 }
diff --git a/gdb/gdbserver/linux-x86-tdesc-selftest.c b/gdb/gdbserver/linux-x86-tdesc-selftest.c
index aa5a8e9..c5ab2ab 100644
--- a/gdb/gdbserver/linux-x86-tdesc-selftest.c
+++ b/gdb/gdbserver/linux-x86-tdesc-selftest.c
@@ -164,7 +164,7 @@ initialize_low_tdesc ()
   init_registers_i386_avx_avx512_linux ();
   init_registers_i386_avx_mpx_avx512_pku_linux ();
 
-  selftests::register_test (selftests::tdesc::i386_tdesc_test);
+  selftests::register_test ("i386-tdesc", selftests::tdesc::i386_tdesc_test);
 
 #ifdef __x86_64__
   init_registers_x32_linux ();
@@ -178,6 +178,6 @@ initialize_low_tdesc ()
   init_registers_amd64_avx_avx512_linux ();
   init_registers_amd64_avx_mpx_avx512_pku_linux ();
 
-  selftests::register_test (selftests::tdesc::amd64_tdesc_test);
+  selftests::register_test ("amd64-tdesc", selftests::tdesc::amd64_tdesc_test);
 #endif
 }
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 56c6393..c0a62c4 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3587,6 +3587,7 @@ captured_main (int argc, char *argv[])
   volatile int attach = 0;
   int was_running;
   bool selftest = false;
+  const char *selftest_filter = NULL;
 
   while (*next_arg != NULL && **next_arg == '-')
     {
@@ -3707,6 +3708,11 @@ captured_main (int argc, char *argv[])
 	run_once = 1;
       else if (strcmp (*next_arg, "--selftest") == 0)
 	selftest = true;
+      else if (startswith (*next_arg, "--selftest="))
+	{
+	  selftest = true;
+	  selftest_filter = *next_arg + strlen ("--selftest=");
+	}
       else
 	{
 	  fprintf (stderr, "Unknown argument: %s\n", *next_arg);
@@ -3783,7 +3789,14 @@ captured_main (int argc, char *argv[])
 
   if (selftest)
     {
-      selftests::run_tests ();
+      debug_printf (_("Running self-tests.\n"));
+
+      selftests::selftests_results results
+	= selftests::run_tests (selftest_filter);
+
+      debug_printf (_("Ran %d unit tests, %d failed\n"),
+		    results.ran, results.failed);
+
       throw_quit ("Quit");
     }
 
diff --git a/gdb/maint.c b/gdb/maint.c
index 28f7287..e1eeb52 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -39,6 +39,7 @@
 #include "top.h"
 #include "maint.h"
 #include "selftest.h"
+#include "selftest-arch.h"
 
 #include "cli/cli-decode.h"
 #include "cli/cli-utils.h"
@@ -959,7 +960,12 @@ show_per_command_cmd (char *args, int from_tty)
 static void
 maintenance_selftest (char *args, int from_tty)
 {
-  selftests::run_tests ();
+  selftests::selftests_results results_noarch = selftests::run_tests (args);
+  selftests::selftests_results results_arch = selftests::run_tests_with_arch (args);
+
+  printf_filtered (_("Ran %d unit tests, %d failed\n"),
+		   results_noarch.ran + results_arch.ran,
+		   results_noarch.failed + results_arch.failed);
 }
 
 \f
diff --git a/gdb/regcache.c b/gdb/regcache.c
index dcbcedd..9e20b67 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1775,7 +1775,8 @@ Print the internal register configuration including each register's\n\
 remote register number and buffer offset in the g/G packets.\n\
 Takes an optional file parameter."),
 	   &maintenanceprintlist);
+
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::current_regcache_test);
+  selftests::register_test ("current_regcache", selftests::current_regcache_test);
 #endif
 }
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index 4cb3aa2..34070ee 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -2781,6 +2781,6 @@ _initialize_rust_exp (void)
   gdb_assert (code == 0);
 
 #if GDB_SELF_TEST
-  selftests::register_test (rust_lex_tests);
+  selftests::register_test ("rust-lex", rust_lex_tests);
 #endif
 }
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index 9a19f76..e6459cc 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -22,15 +22,20 @@
 #include "selftest.h"
 #include "selftest-arch.h"
 #include "arch-utils.h"
+#include <map>
 
 namespace selftests {
 
-static std::vector<self_test_foreach_arch_function *> gdbarch_tests;
+static std::map<std::string, self_test_foreach_arch_function *> gdbarch_tests;
 
 void
-register_test_foreach_arch (self_test_foreach_arch_function *function)
+register_test_foreach_arch (const std::string &name,
+			    self_test_foreach_arch_function *function)
 {
-  gdbarch_tests.push_back (function);
+  /* Make sure we don't already have a test with this name.  */
+  gdb_assert (gdbarch_tests.find (name) == gdbarch_tests.end ());
+
+  gdbarch_tests[name] = function;
 }
 
 void
@@ -41,13 +46,17 @@ reset ()
   reinit_frame_cache ();
 }
 
-static void
-tests_with_arch ()
+selftests_results
+run_tests_with_arch (const char *filter)
 {
-  int failed = 0;
+  selftests_results results;
 
-  for (const auto &f : gdbarch_tests)
+  for (const auto &test : gdbarch_tests)
     {
+      if (filter != NULL && *filter != '\0'
+	  && test.first.find (filter) == std::string::npos)
+	continue;
+
       const char **arches = gdbarch_printable_names ();
 
       for (int i = 0; arches[i] != NULL; i++)
@@ -80,11 +89,15 @@ tests_with_arch ()
 
 	      struct gdbarch *gdbarch = gdbarch_find_by_info (info);
 	      SELF_CHECK (gdbarch != NULL);
-	      f (gdbarch);
+
+	      printf_unfiltered (_("Running selftest %s for arch %s.\n"),
+				 test.first.c_str (), arches[i]);
+	      results.ran++;
+	      test.second (gdbarch);
 	    }
 	  CATCH (ex, RETURN_MASK_ERROR)
 	    {
-	      ++failed;
+	      results.failed++;
 	      exception_fprintf (gdb_stderr, ex,
 				 _("Self test failed: arch %s: "), arches[i]);
 	    }
@@ -94,19 +107,8 @@ tests_with_arch ()
 	}
     }
 
-  SELF_CHECK (failed == 0);
+  return results;
 }
 
 } // namespace selftests
 #endif /* GDB_SELF_TEST */
-
-/* Suppress warning from -Wmissing-prototypes.  */
-extern initialize_file_ftype _initialize_selftests_foreach_arch;
-
-void
-_initialize_selftests_foreach_arch ()
-{
-#if GDB_SELF_TEST
-  selftests::register_test (selftests::tests_with_arch);
-#endif
-}
diff --git a/gdb/selftest-arch.h b/gdb/selftest-arch.h
index dc16c4d..e42eb86 100644
--- a/gdb/selftest-arch.h
+++ b/gdb/selftest-arch.h
@@ -24,7 +24,15 @@ typedef void self_test_foreach_arch_function (struct gdbarch *);
 namespace selftests
 {
 extern void
-  register_test_foreach_arch (self_test_foreach_arch_function *function);
+  register_test_foreach_arch (const std::string &name,
+			      self_test_foreach_arch_function *function);
+
+/* Run all the architecture-dependent self tests.
+
+   If FILTER is not NULL and not empty, only tests with names containing FILTER
+   will be ran.  */
+
+extern selftests_results run_tests_with_arch (const char *filter);
 }
 
 #endif /* SELFTEST_ARCH_H */
diff --git a/gdb/unittests/array-view-selftests.c b/gdb/unittests/array-view-selftests.c
index e5c0043..f618c40 100644
--- a/gdb/unittests/array-view-selftests.c
+++ b/gdb/unittests/array-view-selftests.c
@@ -491,5 +491,6 @@ run_tests ()
 void
 _initialize_array_view_selftests ()
 {
-  selftests::register_test (selftests::array_view_tests::run_tests);
+  selftests::register_test ("array_view",
+			    selftests::array_view_tests::run_tests);
 }
diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c
index 81a71ee..f770901 100644
--- a/gdb/unittests/environ-selftests.c
+++ b/gdb/unittests/environ-selftests.c
@@ -301,5 +301,6 @@ run_tests ()
 void
 _initialize_environ_selftests ()
 {
-  selftests::register_test (selftests::gdb_environ_tests::run_tests);
+  selftests::register_test ("gdb_environ",
+			    selftests::gdb_environ_tests::run_tests);
 }
diff --git a/gdb/unittests/function-view-selftests.c b/gdb/unittests/function-view-selftests.c
index d3018ba..a899299 100644
--- a/gdb/unittests/function-view-selftests.c
+++ b/gdb/unittests/function-view-selftests.c
@@ -174,5 +174,6 @@ run_tests ()
 void
 _initialize_function_view_selftests ()
 {
-  selftests::register_test (selftests::function_view::run_tests);
+  selftests::register_test ("function_view",
+			    selftests::function_view::run_tests);
 }
diff --git a/gdb/unittests/offset-type-selftests.c b/gdb/unittests/offset-type-selftests.c
index 3e66547..5176f20 100644
--- a/gdb/unittests/offset-type-selftests.c
+++ b/gdb/unittests/offset-type-selftests.c
@@ -174,5 +174,5 @@ run_tests ()
 void
 _initialize_offset_type_selftests ()
 {
-  selftests::register_test (selftests::offset_type::run_tests);
+  selftests::register_test ("offset_type", selftests::offset_type::run_tests);
 }
diff --git a/gdb/unittests/optional-selftests.c b/gdb/unittests/optional-selftests.c
index 0bcf964..8ea19bb 100644
--- a/gdb/unittests/optional-selftests.c
+++ b/gdb/unittests/optional-selftests.c
@@ -90,5 +90,5 @@ run_tests ()
 void
 _initialize_optional_selftests ()
 {
-  selftests::register_test (selftests::optional::run_tests);
+  selftests::register_test ("optional", selftests::optional::run_tests);
 }
diff --git a/gdb/unittests/scoped_restore-selftests.c b/gdb/unittests/scoped_restore-selftests.c
index ea7492b..bc9aa2b 100644
--- a/gdb/unittests/scoped_restore-selftests.c
+++ b/gdb/unittests/scoped_restore-selftests.c
@@ -106,5 +106,6 @@ run_tests ()
 void
 _initialize_scoped_restore_selftests ()
 {
-  selftests::register_test (selftests::scoped_restore_tests::run_tests);
+  selftests::register_test ("scoped_restore",
+			    selftests::scoped_restore_tests::run_tests);
 }
diff --git a/gdb/utils-selftests.c b/gdb/utils-selftests.c
index 08feac6..5a30a93 100644
--- a/gdb/utils-selftests.c
+++ b/gdb/utils-selftests.c
@@ -55,6 +55,6 @@ void
 _initialize_utils_selftests (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::common_utils_tests);
+  selftests::register_test ("common-utils", selftests::common_utils_tests);
 #endif
 }
diff --git a/gdb/utils.c b/gdb/utils.c
index af50cf0..c67dc90 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3307,6 +3307,6 @@ _initialize_utils (void)
   add_internal_problem_command (&demangler_warning_problem);
 
 #if GDB_SELF_TEST
-  selftests::register_test (gdb_realpath_tests);
+  selftests::register_test ("gdb_realpath", gdb_realpath_tests);
 #endif
 }
-- 
2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add selftests run filtering
  2017-09-05 11:51 [PATCH] Add selftests run filtering Simon Marchi
@ 2017-09-06 15:25 ` Pedro Alves
  2017-09-06 18:38   ` Simon Marchi
  2017-09-06 21:21   ` [PATCH v2] " Simon Marchi
  0 siblings, 2 replies; 10+ messages in thread
From: Pedro Alves @ 2017-09-06 15:25 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

Sounds useful.  

Patch looks fine to me.  Nits and comments below.

I wonder whether we'll want to be able to do select tests
with e.g., a regexp instead of exact matching.  If we do, then
the std::vector->std::map change would seem pointless.

Do you plan on adding some command to list the existing tests?

On 09/05/2017 12:50 PM, Simon Marchi wrote:
>  static void
>  maintenance_selftest (char *args, int from_tty)
>  {
> -  selftests::run_tests ();
> +  selftests::selftests_results results_noarch = selftests::run_tests (args);
> +  selftests::selftests_results results_arch = selftests::run_tests_with_arch (args);

Line too long?  You could also shorten "selftests_results"
to e.g., "test_results", since the type is already in the
"selftests" namespace.

> +  printf_filtered (_("Ran %d unit tests, %d failed\n"),
> +		   results_noarch.ran + results_arch.ran,
> +		   results_noarch.failed + results_arch.failed);

Wonder whether it'd make sense to implement operator+= for
selftests_results so that you'd write:

  selftests::selftests_results results = selftests::run_tests (args);
  results += selftests::run_tests_with_arch (args);
  // etc.

  printf_filtered (_("Ran %d unit tests, %d failed\n"),
		   results.ran, results.failed);

>  _initialize_findvar (void)
>  {
>  #if GDB_SELF_TEST
> -  selftests::register_test (selftests::findvar_tests::copy_integer_to_size_test);
> +  selftests::register_test (
> +    "copy_integer_to_size",
> +    selftests::findvar_tests::copy_integer_to_size_test);
>  #endif

In GNU style it's much more common to break the line
before "(", not after.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add selftests run filtering
  2017-09-06 15:25 ` Pedro Alves
@ 2017-09-06 18:38   ` Simon Marchi
  2017-09-06 18:41     ` Simon Marchi
  2017-09-06 21:21   ` [PATCH v2] " Simon Marchi
  1 sibling, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2017-09-06 18:38 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2017-09-06 17:25, Pedro Alves wrote:
> Sounds useful.
> 
> Patch looks fine to me.  Nits and comments below.
> 
> I wonder whether we'll want to be able to do select tests
> with e.g., a regexp instead of exact matching.  If we do, then
> the std::vector->std::map change would seem pointless.

Not sure I understand.  I am not using exact matching now either, so 
using a map is probably already pointless.  I am not sure what lead me 
there, maybe I started with the intent of using exact matching.  I'll 
change it to a vector of a new struct type.

> Do you plan on adding some command to list the existing tests?

Yeah, why not.  "maintenance info selftests"?

> On 09/05/2017 12:50 PM, Simon Marchi wrote:
>>  static void
>>  maintenance_selftest (char *args, int from_tty)
>>  {
>> -  selftests::run_tests ();
>> +  selftests::selftests_results results_noarch = selftests::run_tests 
>> (args);
>> +  selftests::selftests_results results_arch = 
>> selftests::run_tests_with_arch (args);
> 
> Line too long?  You could also shorten "selftests_results"
> to e.g., "test_results", since the type is already in the
> "selftests" namespace.

Done.

>> +  printf_filtered (_("Ran %d unit tests, %d failed\n"),
>> +		   results_noarch.ran + results_arch.ran,
>> +		   results_noarch.failed + results_arch.failed);
> 
> Wonder whether it'd make sense to implement operator+= for
> selftests_results so that you'd write:
> 
>   selftests::selftests_results results = selftests::run_tests (args);
>   results += selftests::run_tests_with_arch (args);
>   // etc.
> 
>   printf_filtered (_("Ran %d unit tests, %d failed\n"),
> 		   results.ran, results.failed);

I thought it was a bit overkill for our needs, but on the other side 
there's no good argument against it and it's cleaner.  I'll do that.

>>  _initialize_findvar (void)
>>  {
>>  #if GDB_SELF_TEST
>> -  selftests::register_test 
>> (selftests::findvar_tests::copy_integer_to_size_test);
>> +  selftests::register_test (
>> +    "copy_integer_to_size",
>> +    selftests::findvar_tests::copy_integer_to_size_test);
>>  #endif
> 
> In GNU style it's much more common to break the line
> before "(", not after.

Ah ok thanks, I always have a doubt in that situation.

Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add selftests run filtering
  2017-09-06 18:38   ` Simon Marchi
@ 2017-09-06 18:41     ` Simon Marchi
  2017-09-06 18:44       ` Simon Marchi
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2017-09-06 18:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2017-09-06 20:37, Simon Marchi wrote:
> On 2017-09-06 17:25, Pedro Alves wrote:
>> Sounds useful.
>> 
>> Patch looks fine to me.  Nits and comments below.
>> 
>> I wonder whether we'll want to be able to do select tests
>> with e.g., a regexp instead of exact matching.  If we do, then
>> the std::vector->std::map change would seem pointless.
> 
> Not sure I understand.  I am not using exact matching now either, so
> using a map is probably already pointless.  I am not sure what lead me
> there, maybe I started with the intent of using exact matching.  I'll
> change it to a vector of a new struct type.

Ah, now I remember.  It was so we could easily check that there isn't 
already a test with this name, when registering a new test.  It's won't 
break anything if we happened to have two tests with the same name, so I 
can just remove this check.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add selftests run filtering
  2017-09-06 18:41     ` Simon Marchi
@ 2017-09-06 18:44       ` Simon Marchi
  2017-09-06 18:49         ` Pedro Alves
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2017-09-06 18:44 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Simon Marchi, gdb-patches

On 2017-09-06 20:41, Simon Marchi wrote:
> On 2017-09-06 20:37, Simon Marchi wrote:
>> On 2017-09-06 17:25, Pedro Alves wrote:
>>> Sounds useful.
>>> 
>>> Patch looks fine to me.  Nits and comments below.
>>> 
>>> I wonder whether we'll want to be able to do select tests
>>> with e.g., a regexp instead of exact matching.  If we do, then
>>> the std::vector->std::map change would seem pointless.
>> 
>> Not sure I understand.  I am not using exact matching now either, so
>> using a map is probably already pointless.  I am not sure what lead me
>> there, maybe I started with the intent of using exact matching.  I'll
>> change it to a vector of a new struct type.
> 
> Ah, now I remember.  It was so we could easily check that there isn't
> already a test with this name, when registering a new test.  It's
> won't break anything if we happened to have two tests with the same
> name, so I can just remove this check.

Oh, and also so it would automatically iterate in alphabetical order, I 
thought it was prettier :).

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Add selftests run filtering
  2017-09-06 18:44       ` Simon Marchi
@ 2017-09-06 18:49         ` Pedro Alves
  0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2017-09-06 18:49 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Simon Marchi, gdb-patches


On 09/06/2017 07:43 PM, Simon Marchi wrote:
> On 2017-09-06 20:41, Simon Marchi wrote:
>> On 2017-09-06 20:37, Simon Marchi wrote:
>>> On 2017-09-06 17:25, Pedro Alves wrote:
>>>> Sounds useful.
>>>>
>>>> Patch looks fine to me.  Nits and comments below.
>>>>
>>>> I wonder whether we'll want to be able to do select tests
>>>> with e.g., a regexp instead of exact matching.  If we do, then
>>>> the std::vector->std::map change would seem pointless.
>>>
>>> Not sure I understand.  I am not using exact matching now either, so
>>> using a map is probably already pointless.  I am not sure what lead me
>>> there, maybe I started with the intent of using exact matching.  I'll
>>> change it to a vector of a new struct type.
>>
>> Ah, now I remember.  It was so we could easily check that there isn't
>> already a test with this name, when registering a new test.  It's
>> won't break anything if we happened to have two tests with the same
>> name, so I can just remove this check.
> 
> Oh, and also so it would automatically iterate in alphabetical order, I
> thought it was prettier :).

Ah, making sure order is stable makes sense.  Might be worth it
of a comment.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] Add selftests run filtering
  2017-09-06 15:25 ` Pedro Alves
  2017-09-06 18:38   ` Simon Marchi
@ 2017-09-06 21:21   ` Simon Marchi
  2017-09-07 15:09     ` Simon Marchi
  1 sibling, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2017-09-06 21:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: palves, Simon Marchi

New in v2:

Instead of storing normal and arch selftests separately as in v1,
duplicating how they're stored and ran, I decided to have a single
container, and different types of selftests that implement how they
should be executed.  The "normal" type simply executes the test
function, whereas the "gdbarch" type calls it for each arch.  A visible
difference compared to v1 is that we'll get a single entry/message for
an arch test, not one for each (arch variant x test).  I think it's more
reasonnable, we have 19 tests instead of 2000-ish.

I also added "maintenance info selftests".

Actual commit log:

With the growing number of selftests, I think it would be useful to be
able to run only a subset of the tests.  This patch associates a name to
each registered selftest.  It then allows doing something like:

  (gdb) maintenance selftest aarch64
  Running self-tests.
  Running selftest aarch64-analyze-prologue.
  Running selftest aarch64-process-record.
  Ran 2 unit tests, 0 failed

or with gdbserver:

  ./gdbserver --selftest=aarch64

In both cases, only the tests that contain "aarch64" in their name are
ran.  To help validate that the tests you want to run were actually ran,
it also prints a message with the test name before running each test.

Right now, all the arch-dependent tests are registered as a single test
of the selftests.  To be able to filter those too, I made them
"first-class citizen" selftests.  The selftest type is an interface,
with different implementations for "simple selftests" and "arch
selftests".  The run_tests function simply iterates on that an invokes
operator() on each test.

I changed the tests data structure from a vector to a map, because

  - it allows iterating in a stable (alphabetical) order
  - it allows to easily verify if a test with a given name has been
    registered, to avoid duplicates

There's also a new command "maintenance info selftests" that lists the
registered selftests.

gdb/ChangeLog:

	* common/selftest.h (selftest): New struct/interface.
	(register_test): Add name parameter, add new overload.
	(run_tests): Add filter parameter.
	(for_each_selftest_ftype): New typedef.
	(for_each_selftest): New declaration.
	* common/selftest.c (tests): Change type to
	map<string, unique_ptr<selftest>>.
	(simple_selftest): New struct.
	(register_test): New function.
	(register_test): Add name parameter and use it.
	(run_tests): Add filter parameter and use it.  Add prints.
	Adjust to vector -> map change.
	* aarch64-tdep.c (_initialize_aarch64_tdep): Add names when
	registering selftests.
	* arm-tdep.c (_initialize_arm_tdep): Likewise.
	* disasm-selftests.c (_initialize_disasm_selftests): Likewise.
	* dwarf2-frame.c (_initialize_dwarf2_frame): Likewise.
	* dwarf2loc.c (_initialize_dwarf2loc): Likewise.
	* findvar.c (_initialize_findvar): Likewise.
	* gdbarch-selftests.c (_initialize_gdbarch_selftests): Likewise.
	* maint.c (maintenance_selftest): Update call to run_tests.
	(maintenance_info_selftests): New function.
	(_initialize_maint_cmds): Register "maintenance info selftests"
	command.
	* regcache.c (_initialize_regcache): Add names when registering
	selftests.
	* rust-exp.y (_initialize_rust_exp): Likewise.
	* selftest-arch.c (gdbarch_selftest): New struct.
	(gdbarch_tests): Remove.
	(register_test_foreach_arch): Add name parameter.  Call
	register_test.
	(tests_with_arch): Remove, move most content to
	gdbarch_selftest::operator().
	(_initialize_selftests_foreach_arch): Remove.
	* selftest-arch.h (register_test_foreach_arch): Add name
	parameter.
	(run_tests_with_arch): New declaration.
	* utils-selftests.c (_initialize_utils_selftests): Add names
	when registering selftests.
	* utils.c (_initialize_utils): Likewise.
	* unittests/array-view-selftests.c
	(_initialize_array_view_selftests): Likewise.
	* unittests/environ-selftests.c (_initialize_environ_selftests):
	Likewise.
	* unittests/function-view-selftests.c
	(_initialize_function_view_selftests): Likewise.
	* unittests/offset-type-selftests.c
	(_initialize_offset_type_selftests): Likewise.
	* unittests/optional-selftests.c
	(_initialize_optional_selftests): Likewise.
	* unittests/scoped_restore-selftests.c
	(_initialize_scoped_restore_selftests): Likewise.

gdb/gdbserver/ChangeLog:

	* server.c (captured_main): Accept argument for --selftest.
	Update run_tests call.
	* linux-x86-tdesc-selftest.c (initialize_low_tdesc): Add names
	when registering selftests.
---
 gdb/aarch64-tdep.c                       |   6 +-
 gdb/arm-tdep.c                           |   2 +-
 gdb/common/selftest.c                    |  67 ++++++++++++---
 gdb/common/selftest.h                    |  26 +++++-
 gdb/disasm-selftests.c                   |   6 +-
 gdb/dwarf2-frame.c                       |   3 +-
 gdb/dwarf2loc.c                          |   2 +-
 gdb/findvar.c                            |   4 +-
 gdb/gdbarch-selftests.c                  |   3 +-
 gdb/gdbserver/linux-x86-tdesc-selftest.c |   4 +-
 gdb/gdbserver/server.c                   |   8 +-
 gdb/maint.c                              |  14 +++-
 gdb/regcache.c                           |   3 +-
 gdb/rust-exp.y                           |   2 +-
 gdb/selftest-arch.c                      | 136 +++++++++++++++----------------
 gdb/selftest-arch.h                      |   3 +-
 gdb/unittests/array-view-selftests.c     |   3 +-
 gdb/unittests/environ-selftests.c        |   3 +-
 gdb/unittests/function-view-selftests.c  |   3 +-
 gdb/unittests/offset-type-selftests.c    |   2 +-
 gdb/unittests/optional-selftests.c       |   2 +-
 gdb/unittests/scoped_restore-selftests.c |   3 +-
 gdb/utils-selftests.c                    |   2 +-
 gdb/utils.c                              |   2 +-
 24 files changed, 201 insertions(+), 108 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 5a627a3..ebd3e47 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3068,8 +3068,10 @@ When on, AArch64 specific debugging is enabled."),
 			    &setdebuglist, &showdebuglist);
 
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::aarch64_analyze_prologue_test);
-  selftests::register_test (selftests::aarch64_process_record_test);
+  selftests::register_test ("aarch64-analyze-prologue",
+			    selftests::aarch64_analyze_prologue_test);
+  selftests::register_test ("aarch64-process-record",
+			    selftests::aarch64_process_record_test);
 #endif
 }
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 0c1a0b3..751ee27 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9727,7 +9727,7 @@ vfp - VFP co-processor."),
 			   &setdebuglist, &showdebuglist);
 
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::arm_record_test);
+  selftests::register_test ("arm-record", selftests::arm_record_test);
 #endif
 
 }
diff --git a/gdb/common/selftest.c b/gdb/common/selftest.c
index 0fb8f2a..793c15f 100644
--- a/gdb/common/selftest.c
+++ b/gdb/common/selftest.c
@@ -20,35 +20,71 @@
 #include "common-exceptions.h"
 #include "common-debug.h"
 #include "selftest.h"
-#include <vector>
+#include <map>
 
 namespace selftests
 {
+/* All the tests that have been registered.  Using an std::map allows keeping
+   the order of tests stable and easily looking whether a test name exists.  */
 
-/* All the tests that have been registered.  */
+static std::map<std::string, std::unique_ptr<selftest>> tests;
 
-static std::vector<self_test_function *> tests;
+/* A selftest that calls the test function without arguments.  */
+
+struct simple_selftest : public selftest
+{
+  simple_selftest (self_test_function *function_)
+  : function (function_)
+  {}
+
+  void operator() () const override
+  {
+    function ();
+  }
+
+  self_test_function *function;
+};
 
 /* See selftest.h.  */
 
 void
-register_test (self_test_function *function)
+register_test (const std::string &name, selftest *test)
 {
-  tests.push_back (function);
+  /* Check that no test with this name already exist.  */
+  gdb_assert (tests.find (name) == tests.end ());
+
+  tests[name] = std::unique_ptr<selftest> (test);
 }
 
 /* See selftest.h.  */
 
 void
-run_tests (void)
+register_test (const std::string &name, self_test_function *function)
 {
-  int failed = 0;
+  register_test (name, new simple_selftest (function));
+}
+
+/* See selftest.h.  */
 
-  for (int i = 0; i < tests.size (); ++i)
+void
+run_tests (const char *filter)
+{
+  int ran = 0, failed = 0;
+
+  for (const auto &pair : tests)
     {
+      const std::string &name = pair.first;
+      const std::unique_ptr<selftest> &test = pair.second;
+
+      if (filter != NULL && *filter != '\0'
+	  && name.find (filter) == std::string::npos)
+	continue;
+
       TRY
 	{
-	  tests[i] ();
+	  debug_printf (_("Running selftest %s.\n"), name.c_str ());
+	  ++ran;
+	  (*test) ();
 	}
       CATCH (ex, RETURN_MASK_ERROR)
 	{
@@ -60,7 +96,16 @@ run_tests (void)
       reset ();
     }
 
-  debug_printf ("Ran %lu unit tests, %d failed\n",
-		(long) tests.size (), failed);
+  debug_printf (_("Ran %d unit tests, %d failed\n"),
+		ran, failed);
 }
+
+/* See selftest.h.  */
+
+void for_each_selftest (for_each_selftest_ftype func)
+{
+  for (const auto &pair : tests)
+    func (pair.first);
+}
+
 } // namespace selftests
diff --git a/gdb/common/selftest.h b/gdb/common/selftest.h
index e211c34..35a344f 100644
--- a/gdb/common/selftest.h
+++ b/gdb/common/selftest.h
@@ -27,18 +27,38 @@ typedef void self_test_function (void);
 namespace selftests
 {
 
+/* Interface for the various kinds of selftests.  */
+
+struct selftest
+{
+  virtual void operator() () const = 0;
+};
+
 /* Register a new self-test.  */
 
-extern void register_test (self_test_function *function);
+extern void register_test (const std::string &name, selftest *test);
+
+/* Register a new self-test.  */
+
+extern void register_test (const std::string &name,
+			   self_test_function *function);
 
 /* Run all the self tests.  This print a message describing the number
-   of test and the number of failures.  */
+   of test and the number of failures.
+
+   If FILTER is not NULL and not empty, only tests with names containing FILTER
+   will be ran.  */
 
-extern void run_tests (void);
+extern void run_tests (const char *filter);
 
 /* Reset GDB or GDBserver's internal state.  */
 extern void reset ();
 
+typedef void for_each_selftest_ftype (const std::string &name);
+
+/* Call FUNC for each registered selftest.  */
+
+extern void for_each_selftest (for_each_selftest_ftype func);
 }
 
 /* Check that VALUE is true, and, if not, throw an exception.  */
diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index 4d38ccf..b8ef381 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -214,7 +214,9 @@ void
 _initialize_disasm_selftests (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test_foreach_arch (selftests::print_one_insn_test);
-  selftests::register_test_foreach_arch (selftests::memory_error_test);
+  selftests::register_test_foreach_arch ("print_one_insn",
+					 selftests::print_one_insn_test);
+  selftests::register_test_foreach_arch ("memory_error",
+					 selftests::memory_error_test);
 #endif
 }
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index aaf3aee..268ecda 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -2406,6 +2406,7 @@ _initialize_dwarf2_frame (void)
   dwarf2_frame_objfile_data = register_objfile_data ();
 
 #if GDB_SELF_TEST
-  selftests::register_test_foreach_arch (selftests::execute_cfa_program_test);
+  selftests::register_test_foreach_arch ("execute_cfa_program",
+					 selftests::execute_cfa_program_test);
 #endif
 }
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 1a1b06a..6dd962d 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -4687,6 +4687,6 @@ _initialize_dwarf2loc (void)
 			     &setdebuglist, &showdebuglist);
 
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::copy_bitwise_tests);
+  selftests::register_test ("copy_bitwise", selftests::copy_bitwise_tests);
 #endif
 }
diff --git a/gdb/findvar.c b/gdb/findvar.c
index de6b6ed..b3fb337 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -1095,6 +1095,8 @@ void
 _initialize_findvar (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::findvar_tests::copy_integer_to_size_test);
+  selftests::register_test
+    ("copy_integer_to_size",
+     selftests::findvar_tests::copy_integer_to_size_test);
 #endif
 }
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index cb15964..f0b8d5d 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -151,6 +151,7 @@ void
 _initialize_gdbarch_selftests (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test_foreach_arch (selftests::register_to_value_test);
+  selftests::register_test_foreach_arch ("register_to_value",
+					 selftests::register_to_value_test);
 #endif
 }
diff --git a/gdb/gdbserver/linux-x86-tdesc-selftest.c b/gdb/gdbserver/linux-x86-tdesc-selftest.c
index aa5a8e9..c5ab2ab 100644
--- a/gdb/gdbserver/linux-x86-tdesc-selftest.c
+++ b/gdb/gdbserver/linux-x86-tdesc-selftest.c
@@ -164,7 +164,7 @@ initialize_low_tdesc ()
   init_registers_i386_avx_avx512_linux ();
   init_registers_i386_avx_mpx_avx512_pku_linux ();
 
-  selftests::register_test (selftests::tdesc::i386_tdesc_test);
+  selftests::register_test ("i386-tdesc", selftests::tdesc::i386_tdesc_test);
 
 #ifdef __x86_64__
   init_registers_x32_linux ();
@@ -178,6 +178,6 @@ initialize_low_tdesc ()
   init_registers_amd64_avx_avx512_linux ();
   init_registers_amd64_avx_mpx_avx512_pku_linux ();
 
-  selftests::register_test (selftests::tdesc::amd64_tdesc_test);
+  selftests::register_test ("amd64-tdesc", selftests::tdesc::amd64_tdesc_test);
 #endif
 }
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 56c6393..4669fb7 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3587,6 +3587,7 @@ captured_main (int argc, char *argv[])
   volatile int attach = 0;
   int was_running;
   bool selftest = false;
+  const char *selftest_filter = NULL;
 
   while (*next_arg != NULL && **next_arg == '-')
     {
@@ -3707,6 +3708,11 @@ captured_main (int argc, char *argv[])
 	run_once = 1;
       else if (strcmp (*next_arg, "--selftest") == 0)
 	selftest = true;
+      else if (startswith (*next_arg, "--selftest="))
+	{
+	  selftest = true;
+	  selftest_filter = *next_arg + strlen ("--selftest=");
+	}
       else
 	{
 	  fprintf (stderr, "Unknown argument: %s\n", *next_arg);
@@ -3783,7 +3789,7 @@ captured_main (int argc, char *argv[])
 
   if (selftest)
     {
-      selftests::run_tests ();
+      selftests::run_tests (selftest_filter);
       throw_quit ("Quit");
     }
 
diff --git a/gdb/maint.c b/gdb/maint.c
index 28f7287..49dbee5 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -959,7 +959,16 @@ show_per_command_cmd (char *args, int from_tty)
 static void
 maintenance_selftest (char *args, int from_tty)
 {
-  selftests::run_tests ();
+  selftests::run_tests (args);
+}
+
+static void
+maintenance_info_selftests (char *arg, int from_tty)
+{
+  printf_filtered ("Registered selftests:\n");
+  selftests::for_each_selftest ([] (const std::string &name) {
+    printf_filtered (" - %s\n", name.c_str ());
+  });
 }
 
 \f
@@ -1147,6 +1156,9 @@ This will run any unit tests that were built in to gdb.\n\
 gdb will abort if any test fails."),
 	   &maintenancelist);
 
+  add_cmd ("selftests", class_maintenance, maintenance_info_selftests,
+	 _("List the registered selftests."), &maintenanceinfolist);
+
   add_setshow_zinteger_cmd ("watchdog", class_maintenance, &watchdog, _("\
 Set watchdog timer."), _("\
 Show watchdog timer."), _("\
diff --git a/gdb/regcache.c b/gdb/regcache.c
index dcbcedd..9e20b67 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1775,7 +1775,8 @@ Print the internal register configuration including each register's\n\
 remote register number and buffer offset in the g/G packets.\n\
 Takes an optional file parameter."),
 	   &maintenanceprintlist);
+
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::current_regcache_test);
+  selftests::register_test ("current_regcache", selftests::current_regcache_test);
 #endif
 }
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index 4cb3aa2..34070ee 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -2781,6 +2781,6 @@ _initialize_rust_exp (void)
   gdb_assert (code == 0);
 
 #if GDB_SELF_TEST
-  selftests::register_test (rust_lex_tests);
+  selftests::register_test ("rust-lex", rust_lex_tests);
 #endif
 }
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index 9a19f76..c258da3 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -25,12 +25,75 @@
 
 namespace selftests {
 
-static std::vector<self_test_foreach_arch_function *> gdbarch_tests;
+/* A kind of selftest that calls the test function once for each gdbarch known
+   to GDB.  */
+
+struct gdbarch_selftest : public selftest
+{
+  gdbarch_selftest (self_test_foreach_arch_function *function_)
+  : function (function_)
+  {}
+
+  void operator() () const override
+  {
+    const char **arches = gdbarch_printable_names ();
+    bool pass = true;
+
+    for (int i = 0; arches[i] != NULL; i++)
+      {
+	if (strcmp ("fr300", arches[i]) == 0)
+	  {
+	    /* PR 20946 */
+	    continue;
+	  }
+	else if (strcmp ("powerpc:EC603e", arches[i]) == 0
+		 || strcmp ("powerpc:e500mc", arches[i]) == 0
+		 || strcmp ("powerpc:e500mc64", arches[i]) == 0
+		 || strcmp ("powerpc:titan", arches[i]) == 0
+		 || strcmp ("powerpc:vle", arches[i]) == 0
+		 || strcmp ("powerpc:e5500", arches[i]) == 0
+		 || strcmp ("powerpc:e6500", arches[i]) == 0)
+	  {
+	    /* PR 19797 */
+	    continue;
+	  }
+
+	QUIT;
+
+	TRY
+	  {
+	    struct gdbarch_info info;
+
+	    gdbarch_info_init (&info);
+	    info.bfd_arch_info = bfd_scan_arch (arches[i]);
+
+	    struct gdbarch *gdbarch = gdbarch_find_by_info (info);
+	    SELF_CHECK (gdbarch != NULL);
+
+	    function (gdbarch);
+	  }
+	CATCH (ex, RETURN_MASK_ERROR)
+	  {
+	    pass = false;
+	    exception_fprintf (gdb_stderr, ex,
+			       _("Self test failed: arch %s: "), arches[i]);
+	  }
+	END_CATCH
+
+	reset ();
+      }
+
+    SELF_CHECK (pass);
+  }
+
+  self_test_foreach_arch_function *function;
+};
 
 void
-register_test_foreach_arch (self_test_foreach_arch_function *function)
+register_test_foreach_arch (const std::string &name,
+			    self_test_foreach_arch_function *function)
 {
-  gdbarch_tests.push_back (function);
+  register_test (name, new gdbarch_selftest (function));
 }
 
 void
@@ -41,72 +104,5 @@ reset ()
   reinit_frame_cache ();
 }
 
-static void
-tests_with_arch ()
-{
-  int failed = 0;
-
-  for (const auto &f : gdbarch_tests)
-    {
-      const char **arches = gdbarch_printable_names ();
-
-      for (int i = 0; arches[i] != NULL; i++)
-	{
-	  if (strcmp ("fr300", arches[i]) == 0)
-	    {
-	      /* PR 20946 */
-	      continue;
-	    }
-	  else if (strcmp ("powerpc:EC603e", arches[i]) == 0
-		   || strcmp ("powerpc:e500mc", arches[i]) == 0
-		   || strcmp ("powerpc:e500mc64", arches[i]) == 0
-		   || strcmp ("powerpc:titan", arches[i]) == 0
-		   || strcmp ("powerpc:vle", arches[i]) == 0
-		   || strcmp ("powerpc:e5500", arches[i]) == 0
-		   || strcmp ("powerpc:e6500", arches[i]) == 0)
-	    {
-	      /* PR 19797 */
-	      continue;
-	    }
-
-	  QUIT;
-
-	  TRY
-	    {
-	      struct gdbarch_info info;
-
-	      gdbarch_info_init (&info);
-	      info.bfd_arch_info = bfd_scan_arch (arches[i]);
-
-	      struct gdbarch *gdbarch = gdbarch_find_by_info (info);
-	      SELF_CHECK (gdbarch != NULL);
-	      f (gdbarch);
-	    }
-	  CATCH (ex, RETURN_MASK_ERROR)
-	    {
-	      ++failed;
-	      exception_fprintf (gdb_stderr, ex,
-				 _("Self test failed: arch %s: "), arches[i]);
-	    }
-	  END_CATCH
-
-	  reset ();
-	}
-    }
-
-  SELF_CHECK (failed == 0);
-}
-
 } // namespace selftests
 #endif /* GDB_SELF_TEST */
-
-/* Suppress warning from -Wmissing-prototypes.  */
-extern initialize_file_ftype _initialize_selftests_foreach_arch;
-
-void
-_initialize_selftests_foreach_arch ()
-{
-#if GDB_SELF_TEST
-  selftests::register_test (selftests::tests_with_arch);
-#endif
-}
diff --git a/gdb/selftest-arch.h b/gdb/selftest-arch.h
index dc16c4d..f1fa38e 100644
--- a/gdb/selftest-arch.h
+++ b/gdb/selftest-arch.h
@@ -24,7 +24,8 @@ typedef void self_test_foreach_arch_function (struct gdbarch *);
 namespace selftests
 {
 extern void
-  register_test_foreach_arch (self_test_foreach_arch_function *function);
+  register_test_foreach_arch (const std::string &name,
+			      self_test_foreach_arch_function *function);
 }
 
 #endif /* SELFTEST_ARCH_H */
diff --git a/gdb/unittests/array-view-selftests.c b/gdb/unittests/array-view-selftests.c
index e5c0043..f618c40 100644
--- a/gdb/unittests/array-view-selftests.c
+++ b/gdb/unittests/array-view-selftests.c
@@ -491,5 +491,6 @@ run_tests ()
 void
 _initialize_array_view_selftests ()
 {
-  selftests::register_test (selftests::array_view_tests::run_tests);
+  selftests::register_test ("array_view",
+			    selftests::array_view_tests::run_tests);
 }
diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c
index 81a71ee..f770901 100644
--- a/gdb/unittests/environ-selftests.c
+++ b/gdb/unittests/environ-selftests.c
@@ -301,5 +301,6 @@ run_tests ()
 void
 _initialize_environ_selftests ()
 {
-  selftests::register_test (selftests::gdb_environ_tests::run_tests);
+  selftests::register_test ("gdb_environ",
+			    selftests::gdb_environ_tests::run_tests);
 }
diff --git a/gdb/unittests/function-view-selftests.c b/gdb/unittests/function-view-selftests.c
index d3018ba..a899299 100644
--- a/gdb/unittests/function-view-selftests.c
+++ b/gdb/unittests/function-view-selftests.c
@@ -174,5 +174,6 @@ run_tests ()
 void
 _initialize_function_view_selftests ()
 {
-  selftests::register_test (selftests::function_view::run_tests);
+  selftests::register_test ("function_view",
+			    selftests::function_view::run_tests);
 }
diff --git a/gdb/unittests/offset-type-selftests.c b/gdb/unittests/offset-type-selftests.c
index 3e66547..5176f20 100644
--- a/gdb/unittests/offset-type-selftests.c
+++ b/gdb/unittests/offset-type-selftests.c
@@ -174,5 +174,5 @@ run_tests ()
 void
 _initialize_offset_type_selftests ()
 {
-  selftests::register_test (selftests::offset_type::run_tests);
+  selftests::register_test ("offset_type", selftests::offset_type::run_tests);
 }
diff --git a/gdb/unittests/optional-selftests.c b/gdb/unittests/optional-selftests.c
index 0bcf964..8ea19bb 100644
--- a/gdb/unittests/optional-selftests.c
+++ b/gdb/unittests/optional-selftests.c
@@ -90,5 +90,5 @@ run_tests ()
 void
 _initialize_optional_selftests ()
 {
-  selftests::register_test (selftests::optional::run_tests);
+  selftests::register_test ("optional", selftests::optional::run_tests);
 }
diff --git a/gdb/unittests/scoped_restore-selftests.c b/gdb/unittests/scoped_restore-selftests.c
index ea7492b..bc9aa2b 100644
--- a/gdb/unittests/scoped_restore-selftests.c
+++ b/gdb/unittests/scoped_restore-selftests.c
@@ -106,5 +106,6 @@ run_tests ()
 void
 _initialize_scoped_restore_selftests ()
 {
-  selftests::register_test (selftests::scoped_restore_tests::run_tests);
+  selftests::register_test ("scoped_restore",
+			    selftests::scoped_restore_tests::run_tests);
 }
diff --git a/gdb/utils-selftests.c b/gdb/utils-selftests.c
index 08feac6..5a30a93 100644
--- a/gdb/utils-selftests.c
+++ b/gdb/utils-selftests.c
@@ -55,6 +55,6 @@ void
 _initialize_utils_selftests (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::common_utils_tests);
+  selftests::register_test ("common-utils", selftests::common_utils_tests);
 #endif
 }
diff --git a/gdb/utils.c b/gdb/utils.c
index af50cf0..c67dc90 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3307,6 +3307,6 @@ _initialize_utils (void)
   add_internal_problem_command (&demangler_warning_problem);
 
 #if GDB_SELF_TEST
-  selftests::register_test (gdb_realpath_tests);
+  selftests::register_test ("gdb_realpath", gdb_realpath_tests);
 #endif
 }
-- 
2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] Add selftests run filtering
  2017-09-06 21:21   ` [PATCH v2] " Simon Marchi
@ 2017-09-07 15:09     ` Simon Marchi
  2017-09-07 15:11       ` [PATCH v3] " Simon Marchi
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2017-09-07 15:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: palves, eliz, Simon Marchi

New in v3:

I realized I had forgotten help, news and doc, this patch adds those.

Actual commit log:

With the growing number of selftests, I think it would be useful to be
able to run only a subset of the tests.  This patch associates a name to
each registered selftest.  It then allows doing something like:

  (gdb) maintenance selftest aarch64
  Running self-tests.
  Running selftest aarch64-analyze-prologue.
  Running selftest aarch64-process-record.
  Ran 2 unit tests, 0 failed

or with gdbserver:

  ./gdbserver --selftest=aarch64

In both cases, only the tests that contain "aarch64" in their name are
ran.  To help validate that the tests you want to run were actually ran,
it also prints a message with the test name before running each test.

Right now, all the arch-dependent tests are registered as a single test
of the selftests.  To be able to filter those too, I made them
"first-class citizen" selftests.  The selftest type is an interface,
with different implementations for "simple selftests" and "arch
selftests".  The run_tests function simply iterates on that an invokes
operator() on each test.

I changed the tests data structure from a vector to a map, because

  - it allows iterating in a stable (alphabetical) order
  - it allows to easily verify if a test with a given name has been
    registered, to avoid duplicates

There's also a new command "maintenance info selftests" that lists the
registered selftests.

gdb/ChangeLog:

	* common/selftest.h (selftest): New struct/interface.
	(register_test): Add name parameter, add new overload.
	(run_tests): Add filter parameter.
	(for_each_selftest_ftype): New typedef.
	(for_each_selftest): New declaration.
	* common/selftest.c (tests): Change type to
	map<string, unique_ptr<selftest>>.
	(simple_selftest): New struct.
	(register_test): New function.
	(register_test): Add name parameter and use it.
	(run_tests): Add filter parameter and use it.  Add prints.
	Adjust to vector -> map change.
	* aarch64-tdep.c (_initialize_aarch64_tdep): Add names when
	registering selftests.
	* arm-tdep.c (_initialize_arm_tdep): Likewise.
	* disasm-selftests.c (_initialize_disasm_selftests): Likewise.
	* dwarf2-frame.c (_initialize_dwarf2_frame): Likewise.
	* dwarf2loc.c (_initialize_dwarf2loc): Likewise.
	* findvar.c (_initialize_findvar): Likewise.
	* gdbarch-selftests.c (_initialize_gdbarch_selftests): Likewise.
	* maint.c (maintenance_selftest): Update call to run_tests.
	(maintenance_info_selftests): New function.
	(_initialize_maint_cmds): Register "maintenance info selftests"
	command.  Update "maintenance selftest" doc.
	* regcache.c (_initialize_regcache): Add names when registering
	selftests.
	* rust-exp.y (_initialize_rust_exp): Likewise.
	* selftest-arch.c (gdbarch_selftest): New struct.
	(gdbarch_tests): Remove.
	(register_test_foreach_arch): Add name parameter.  Call
	register_test.
	(tests_with_arch): Remove, move most content to
	gdbarch_selftest::operator().
	(_initialize_selftests_foreach_arch): Remove.
	* selftest-arch.h (register_test_foreach_arch): Add name
	parameter.
	(run_tests_with_arch): New declaration.
	* utils-selftests.c (_initialize_utils_selftests): Add names
	when registering selftests.
	* utils.c (_initialize_utils): Likewise.
	* unittests/array-view-selftests.c
	(_initialize_array_view_selftests): Likewise.
	* unittests/environ-selftests.c (_initialize_environ_selftests):
	Likewise.
	* unittests/function-view-selftests.c
	(_initialize_function_view_selftests): Likewise.
	* unittests/offset-type-selftests.c
	(_initialize_offset_type_selftests): Likewise.
	* unittests/optional-selftests.c
	(_initialize_optional_selftests): Likewise.
	* unittests/scoped_restore-selftests.c
	(_initialize_scoped_restore_selftests): Likewise.
	* NEWS: Document "maintenance selftest" and "maint info
	selftests".

gdb/gdbserver/ChangeLog:

	* server.c (captured_main): Accept argument for --selftest.
	Update run_tests call.
	* linux-x86-tdesc-selftest.c (initialize_low_tdesc): Add names
	when registering selftests.

gdb/doc/ChangeLog:

	* gdb.texinfo (Maintenance Commands): Document filter parameter
	of "maint selftest".  Document "maint info selftests" command.
---
 gdb/NEWS                                 |   6 ++
 gdb/aarch64-tdep.c                       |   6 +-
 gdb/arm-tdep.c                           |   2 +-
 gdb/common/selftest.c                    |  67 ++++++++++++---
 gdb/common/selftest.h                    |  26 +++++-
 gdb/disasm-selftests.c                   |   6 +-
 gdb/doc/gdb.texinfo                      |   8 ++
 gdb/dwarf2-frame.c                       |   3 +-
 gdb/dwarf2loc.c                          |   2 +-
 gdb/findvar.c                            |   4 +-
 gdb/gdbarch-selftests.c                  |   3 +-
 gdb/gdbserver/linux-x86-tdesc-selftest.c |   4 +-
 gdb/gdbserver/server.c                   |   8 +-
 gdb/maint.c                              |  18 +++-
 gdb/regcache.c                           |   3 +-
 gdb/rust-exp.y                           |   2 +-
 gdb/selftest-arch.c                      | 136 +++++++++++++++----------------
 gdb/selftest-arch.h                      |   3 +-
 gdb/unittests/array-view-selftests.c     |   3 +-
 gdb/unittests/environ-selftests.c        |   3 +-
 gdb/unittests/function-view-selftests.c  |   3 +-
 gdb/unittests/offset-type-selftests.c    |   2 +-
 gdb/unittests/optional-selftests.c       |   2 +-
 gdb/unittests/scoped_restore-selftests.c |   3 +-
 gdb/utils-selftests.c                    |   2 +-
 gdb/utils.c                              |   2 +-
 26 files changed, 217 insertions(+), 110 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 0280a2e..052bafd 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -53,6 +53,9 @@ QStartupWithShell
 * The "maintenance print c-tdesc" command now takes an optional
   argument which is the file name of XML target description.
 
+* The "maintenance selftest" command now takes an optional argument to
+  filter the tests to be run.
+
 * New commands
 
 set|show compile-gcc
@@ -63,6 +66,9 @@ set debug separate-debug-file
 show debug separate-debug-file
   Control the display of debug output about separate debug file search.
 
+maint info selftests
+  List the registered selftests.
+
 * TUI Single-Key mode now supports two new shortcut keys: `i' for stepi and
   `o' for nexti.
 
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 5a627a3..ebd3e47 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -3068,8 +3068,10 @@ When on, AArch64 specific debugging is enabled."),
 			    &setdebuglist, &showdebuglist);
 
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::aarch64_analyze_prologue_test);
-  selftests::register_test (selftests::aarch64_process_record_test);
+  selftests::register_test ("aarch64-analyze-prologue",
+			    selftests::aarch64_analyze_prologue_test);
+  selftests::register_test ("aarch64-process-record",
+			    selftests::aarch64_process_record_test);
 #endif
 }
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 0c1a0b3..751ee27 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -9727,7 +9727,7 @@ vfp - VFP co-processor."),
 			   &setdebuglist, &showdebuglist);
 
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::arm_record_test);
+  selftests::register_test ("arm-record", selftests::arm_record_test);
 #endif
 
 }
diff --git a/gdb/common/selftest.c b/gdb/common/selftest.c
index 0fb8f2a..793c15f 100644
--- a/gdb/common/selftest.c
+++ b/gdb/common/selftest.c
@@ -20,35 +20,71 @@
 #include "common-exceptions.h"
 #include "common-debug.h"
 #include "selftest.h"
-#include <vector>
+#include <map>
 
 namespace selftests
 {
+/* All the tests that have been registered.  Using an std::map allows keeping
+   the order of tests stable and easily looking whether a test name exists.  */
 
-/* All the tests that have been registered.  */
+static std::map<std::string, std::unique_ptr<selftest>> tests;
 
-static std::vector<self_test_function *> tests;
+/* A selftest that calls the test function without arguments.  */
+
+struct simple_selftest : public selftest
+{
+  simple_selftest (self_test_function *function_)
+  : function (function_)
+  {}
+
+  void operator() () const override
+  {
+    function ();
+  }
+
+  self_test_function *function;
+};
 
 /* See selftest.h.  */
 
 void
-register_test (self_test_function *function)
+register_test (const std::string &name, selftest *test)
 {
-  tests.push_back (function);
+  /* Check that no test with this name already exist.  */
+  gdb_assert (tests.find (name) == tests.end ());
+
+  tests[name] = std::unique_ptr<selftest> (test);
 }
 
 /* See selftest.h.  */
 
 void
-run_tests (void)
+register_test (const std::string &name, self_test_function *function)
 {
-  int failed = 0;
+  register_test (name, new simple_selftest (function));
+}
+
+/* See selftest.h.  */
 
-  for (int i = 0; i < tests.size (); ++i)
+void
+run_tests (const char *filter)
+{
+  int ran = 0, failed = 0;
+
+  for (const auto &pair : tests)
     {
+      const std::string &name = pair.first;
+      const std::unique_ptr<selftest> &test = pair.second;
+
+      if (filter != NULL && *filter != '\0'
+	  && name.find (filter) == std::string::npos)
+	continue;
+
       TRY
 	{
-	  tests[i] ();
+	  debug_printf (_("Running selftest %s.\n"), name.c_str ());
+	  ++ran;
+	  (*test) ();
 	}
       CATCH (ex, RETURN_MASK_ERROR)
 	{
@@ -60,7 +96,16 @@ run_tests (void)
       reset ();
     }
 
-  debug_printf ("Ran %lu unit tests, %d failed\n",
-		(long) tests.size (), failed);
+  debug_printf (_("Ran %d unit tests, %d failed\n"),
+		ran, failed);
 }
+
+/* See selftest.h.  */
+
+void for_each_selftest (for_each_selftest_ftype func)
+{
+  for (const auto &pair : tests)
+    func (pair.first);
+}
+
 } // namespace selftests
diff --git a/gdb/common/selftest.h b/gdb/common/selftest.h
index e211c34..35a344f 100644
--- a/gdb/common/selftest.h
+++ b/gdb/common/selftest.h
@@ -27,18 +27,38 @@ typedef void self_test_function (void);
 namespace selftests
 {
 
+/* Interface for the various kinds of selftests.  */
+
+struct selftest
+{
+  virtual void operator() () const = 0;
+};
+
 /* Register a new self-test.  */
 
-extern void register_test (self_test_function *function);
+extern void register_test (const std::string &name, selftest *test);
+
+/* Register a new self-test.  */
+
+extern void register_test (const std::string &name,
+			   self_test_function *function);
 
 /* Run all the self tests.  This print a message describing the number
-   of test and the number of failures.  */
+   of test and the number of failures.
+
+   If FILTER is not NULL and not empty, only tests with names containing FILTER
+   will be ran.  */
 
-extern void run_tests (void);
+extern void run_tests (const char *filter);
 
 /* Reset GDB or GDBserver's internal state.  */
 extern void reset ();
 
+typedef void for_each_selftest_ftype (const std::string &name);
+
+/* Call FUNC for each registered selftest.  */
+
+extern void for_each_selftest (for_each_selftest_ftype func);
 }
 
 /* Check that VALUE is true, and, if not, throw an exception.  */
diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index 4d38ccf..b8ef381 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -214,7 +214,9 @@ void
 _initialize_disasm_selftests (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test_foreach_arch (selftests::print_one_insn_test);
-  selftests::register_test_foreach_arch (selftests::memory_error_test);
+  selftests::register_test_foreach_arch ("print_one_insn",
+					 selftests::print_one_insn_test);
+  selftests::register_test_foreach_arch ("memory_error",
+					 selftests::memory_error_test);
 #endif
 }
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8282dae..af5fe6f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35067,8 +35067,16 @@ data structures, including its flags and contained types.
 
 @kindex maint selftest
 @cindex self tests
+@item maint selftest @r{[}@var{filter}@r{]}
 Run any self tests that were compiled in to @value{GDBN}.  This will
 print a message showing how many tests were run, and how many failed.
+If a @var{filter} is passed, only the tests with @var{filter} in their
+name will by ran.
+
+@kindex "maint info selftests"
+@cindex self tests
+@item maint info selftests
+List the selftests compiled in to @value{GDBN}.
 
 @kindex maint set dwarf always-disassemble
 @kindex maint show dwarf always-disassemble
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index aaf3aee..268ecda 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -2406,6 +2406,7 @@ _initialize_dwarf2_frame (void)
   dwarf2_frame_objfile_data = register_objfile_data ();
 
 #if GDB_SELF_TEST
-  selftests::register_test_foreach_arch (selftests::execute_cfa_program_test);
+  selftests::register_test_foreach_arch ("execute_cfa_program",
+					 selftests::execute_cfa_program_test);
 #endif
 }
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 1a1b06a..6dd962d 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -4687,6 +4687,6 @@ _initialize_dwarf2loc (void)
 			     &setdebuglist, &showdebuglist);
 
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::copy_bitwise_tests);
+  selftests::register_test ("copy_bitwise", selftests::copy_bitwise_tests);
 #endif
 }
diff --git a/gdb/findvar.c b/gdb/findvar.c
index de6b6ed..b3fb337 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -1095,6 +1095,8 @@ void
 _initialize_findvar (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::findvar_tests::copy_integer_to_size_test);
+  selftests::register_test
+    ("copy_integer_to_size",
+     selftests::findvar_tests::copy_integer_to_size_test);
 #endif
 }
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index cb15964..f0b8d5d 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -151,6 +151,7 @@ void
 _initialize_gdbarch_selftests (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test_foreach_arch (selftests::register_to_value_test);
+  selftests::register_test_foreach_arch ("register_to_value",
+					 selftests::register_to_value_test);
 #endif
 }
diff --git a/gdb/gdbserver/linux-x86-tdesc-selftest.c b/gdb/gdbserver/linux-x86-tdesc-selftest.c
index aa5a8e9..c5ab2ab 100644
--- a/gdb/gdbserver/linux-x86-tdesc-selftest.c
+++ b/gdb/gdbserver/linux-x86-tdesc-selftest.c
@@ -164,7 +164,7 @@ initialize_low_tdesc ()
   init_registers_i386_avx_avx512_linux ();
   init_registers_i386_avx_mpx_avx512_pku_linux ();
 
-  selftests::register_test (selftests::tdesc::i386_tdesc_test);
+  selftests::register_test ("i386-tdesc", selftests::tdesc::i386_tdesc_test);
 
 #ifdef __x86_64__
   init_registers_x32_linux ();
@@ -178,6 +178,6 @@ initialize_low_tdesc ()
   init_registers_amd64_avx_avx512_linux ();
   init_registers_amd64_avx_mpx_avx512_pku_linux ();
 
-  selftests::register_test (selftests::tdesc::amd64_tdesc_test);
+  selftests::register_test ("amd64-tdesc", selftests::tdesc::amd64_tdesc_test);
 #endif
 }
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 56c6393..4669fb7 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3587,6 +3587,7 @@ captured_main (int argc, char *argv[])
   volatile int attach = 0;
   int was_running;
   bool selftest = false;
+  const char *selftest_filter = NULL;
 
   while (*next_arg != NULL && **next_arg == '-')
     {
@@ -3707,6 +3708,11 @@ captured_main (int argc, char *argv[])
 	run_once = 1;
       else if (strcmp (*next_arg, "--selftest") == 0)
 	selftest = true;
+      else if (startswith (*next_arg, "--selftest="))
+	{
+	  selftest = true;
+	  selftest_filter = *next_arg + strlen ("--selftest=");
+	}
       else
 	{
 	  fprintf (stderr, "Unknown argument: %s\n", *next_arg);
@@ -3783,7 +3789,7 @@ captured_main (int argc, char *argv[])
 
   if (selftest)
     {
-      selftests::run_tests ();
+      selftests::run_tests (selftest_filter);
       throw_quit ("Quit");
     }
 
diff --git a/gdb/maint.c b/gdb/maint.c
index 28f7287..b152e26 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -959,7 +959,16 @@ show_per_command_cmd (char *args, int from_tty)
 static void
 maintenance_selftest (char *args, int from_tty)
 {
-  selftests::run_tests ();
+  selftests::run_tests (args);
+}
+
+static void
+maintenance_info_selftests (char *arg, int from_tty)
+{
+  printf_filtered ("Registered selftests:\n");
+  selftests::for_each_selftest ([] (const std::string &name) {
+    printf_filtered (" - %s\n", name.c_str ());
+  });
 }
 
 \f
@@ -1142,11 +1151,14 @@ If you decide you want to use it: maintenance undeprecate 'commandname'"),
 
   add_cmd ("selftest", class_maintenance, maintenance_selftest, _("\
 Run gdb's unit tests.\n\
-Usage: maintenance selftest\n\
+Usage: maintenance selftest [filter]\n\
 This will run any unit tests that were built in to gdb.\n\
-gdb will abort if any test fails."),
+If a filter is given, only the tests with that value in their name will ran."),
 	   &maintenancelist);
 
+  add_cmd ("selftests", class_maintenance, maintenance_info_selftests,
+	 _("List the registered selftests."), &maintenanceinfolist);
+
   add_setshow_zinteger_cmd ("watchdog", class_maintenance, &watchdog, _("\
 Set watchdog timer."), _("\
 Show watchdog timer."), _("\
diff --git a/gdb/regcache.c b/gdb/regcache.c
index dcbcedd..9e20b67 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1775,7 +1775,8 @@ Print the internal register configuration including each register's\n\
 remote register number and buffer offset in the g/G packets.\n\
 Takes an optional file parameter."),
 	   &maintenanceprintlist);
+
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::current_regcache_test);
+  selftests::register_test ("current_regcache", selftests::current_regcache_test);
 #endif
 }
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index 4cb3aa2..34070ee 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -2781,6 +2781,6 @@ _initialize_rust_exp (void)
   gdb_assert (code == 0);
 
 #if GDB_SELF_TEST
-  selftests::register_test (rust_lex_tests);
+  selftests::register_test ("rust-lex", rust_lex_tests);
 #endif
 }
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index 9a19f76..c258da3 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -25,12 +25,75 @@
 
 namespace selftests {
 
-static std::vector<self_test_foreach_arch_function *> gdbarch_tests;
+/* A kind of selftest that calls the test function once for each gdbarch known
+   to GDB.  */
+
+struct gdbarch_selftest : public selftest
+{
+  gdbarch_selftest (self_test_foreach_arch_function *function_)
+  : function (function_)
+  {}
+
+  void operator() () const override
+  {
+    const char **arches = gdbarch_printable_names ();
+    bool pass = true;
+
+    for (int i = 0; arches[i] != NULL; i++)
+      {
+	if (strcmp ("fr300", arches[i]) == 0)
+	  {
+	    /* PR 20946 */
+	    continue;
+	  }
+	else if (strcmp ("powerpc:EC603e", arches[i]) == 0
+		 || strcmp ("powerpc:e500mc", arches[i]) == 0
+		 || strcmp ("powerpc:e500mc64", arches[i]) == 0
+		 || strcmp ("powerpc:titan", arches[i]) == 0
+		 || strcmp ("powerpc:vle", arches[i]) == 0
+		 || strcmp ("powerpc:e5500", arches[i]) == 0
+		 || strcmp ("powerpc:e6500", arches[i]) == 0)
+	  {
+	    /* PR 19797 */
+	    continue;
+	  }
+
+	QUIT;
+
+	TRY
+	  {
+	    struct gdbarch_info info;
+
+	    gdbarch_info_init (&info);
+	    info.bfd_arch_info = bfd_scan_arch (arches[i]);
+
+	    struct gdbarch *gdbarch = gdbarch_find_by_info (info);
+	    SELF_CHECK (gdbarch != NULL);
+
+	    function (gdbarch);
+	  }
+	CATCH (ex, RETURN_MASK_ERROR)
+	  {
+	    pass = false;
+	    exception_fprintf (gdb_stderr, ex,
+			       _("Self test failed: arch %s: "), arches[i]);
+	  }
+	END_CATCH
+
+	reset ();
+      }
+
+    SELF_CHECK (pass);
+  }
+
+  self_test_foreach_arch_function *function;
+};
 
 void
-register_test_foreach_arch (self_test_foreach_arch_function *function)
+register_test_foreach_arch (const std::string &name,
+			    self_test_foreach_arch_function *function)
 {
-  gdbarch_tests.push_back (function);
+  register_test (name, new gdbarch_selftest (function));
 }
 
 void
@@ -41,72 +104,5 @@ reset ()
   reinit_frame_cache ();
 }
 
-static void
-tests_with_arch ()
-{
-  int failed = 0;
-
-  for (const auto &f : gdbarch_tests)
-    {
-      const char **arches = gdbarch_printable_names ();
-
-      for (int i = 0; arches[i] != NULL; i++)
-	{
-	  if (strcmp ("fr300", arches[i]) == 0)
-	    {
-	      /* PR 20946 */
-	      continue;
-	    }
-	  else if (strcmp ("powerpc:EC603e", arches[i]) == 0
-		   || strcmp ("powerpc:e500mc", arches[i]) == 0
-		   || strcmp ("powerpc:e500mc64", arches[i]) == 0
-		   || strcmp ("powerpc:titan", arches[i]) == 0
-		   || strcmp ("powerpc:vle", arches[i]) == 0
-		   || strcmp ("powerpc:e5500", arches[i]) == 0
-		   || strcmp ("powerpc:e6500", arches[i]) == 0)
-	    {
-	      /* PR 19797 */
-	      continue;
-	    }
-
-	  QUIT;
-
-	  TRY
-	    {
-	      struct gdbarch_info info;
-
-	      gdbarch_info_init (&info);
-	      info.bfd_arch_info = bfd_scan_arch (arches[i]);
-
-	      struct gdbarch *gdbarch = gdbarch_find_by_info (info);
-	      SELF_CHECK (gdbarch != NULL);
-	      f (gdbarch);
-	    }
-	  CATCH (ex, RETURN_MASK_ERROR)
-	    {
-	      ++failed;
-	      exception_fprintf (gdb_stderr, ex,
-				 _("Self test failed: arch %s: "), arches[i]);
-	    }
-	  END_CATCH
-
-	  reset ();
-	}
-    }
-
-  SELF_CHECK (failed == 0);
-}
-
 } // namespace selftests
 #endif /* GDB_SELF_TEST */
-
-/* Suppress warning from -Wmissing-prototypes.  */
-extern initialize_file_ftype _initialize_selftests_foreach_arch;
-
-void
-_initialize_selftests_foreach_arch ()
-{
-#if GDB_SELF_TEST
-  selftests::register_test (selftests::tests_with_arch);
-#endif
-}
diff --git a/gdb/selftest-arch.h b/gdb/selftest-arch.h
index dc16c4d..f1fa38e 100644
--- a/gdb/selftest-arch.h
+++ b/gdb/selftest-arch.h
@@ -24,7 +24,8 @@ typedef void self_test_foreach_arch_function (struct gdbarch *);
 namespace selftests
 {
 extern void
-  register_test_foreach_arch (self_test_foreach_arch_function *function);
+  register_test_foreach_arch (const std::string &name,
+			      self_test_foreach_arch_function *function);
 }
 
 #endif /* SELFTEST_ARCH_H */
diff --git a/gdb/unittests/array-view-selftests.c b/gdb/unittests/array-view-selftests.c
index e5c0043..f618c40 100644
--- a/gdb/unittests/array-view-selftests.c
+++ b/gdb/unittests/array-view-selftests.c
@@ -491,5 +491,6 @@ run_tests ()
 void
 _initialize_array_view_selftests ()
 {
-  selftests::register_test (selftests::array_view_tests::run_tests);
+  selftests::register_test ("array_view",
+			    selftests::array_view_tests::run_tests);
 }
diff --git a/gdb/unittests/environ-selftests.c b/gdb/unittests/environ-selftests.c
index 81a71ee..f770901 100644
--- a/gdb/unittests/environ-selftests.c
+++ b/gdb/unittests/environ-selftests.c
@@ -301,5 +301,6 @@ run_tests ()
 void
 _initialize_environ_selftests ()
 {
-  selftests::register_test (selftests::gdb_environ_tests::run_tests);
+  selftests::register_test ("gdb_environ",
+			    selftests::gdb_environ_tests::run_tests);
 }
diff --git a/gdb/unittests/function-view-selftests.c b/gdb/unittests/function-view-selftests.c
index d3018ba..a899299 100644
--- a/gdb/unittests/function-view-selftests.c
+++ b/gdb/unittests/function-view-selftests.c
@@ -174,5 +174,6 @@ run_tests ()
 void
 _initialize_function_view_selftests ()
 {
-  selftests::register_test (selftests::function_view::run_tests);
+  selftests::register_test ("function_view",
+			    selftests::function_view::run_tests);
 }
diff --git a/gdb/unittests/offset-type-selftests.c b/gdb/unittests/offset-type-selftests.c
index 3e66547..5176f20 100644
--- a/gdb/unittests/offset-type-selftests.c
+++ b/gdb/unittests/offset-type-selftests.c
@@ -174,5 +174,5 @@ run_tests ()
 void
 _initialize_offset_type_selftests ()
 {
-  selftests::register_test (selftests::offset_type::run_tests);
+  selftests::register_test ("offset_type", selftests::offset_type::run_tests);
 }
diff --git a/gdb/unittests/optional-selftests.c b/gdb/unittests/optional-selftests.c
index 0bcf964..8ea19bb 100644
--- a/gdb/unittests/optional-selftests.c
+++ b/gdb/unittests/optional-selftests.c
@@ -90,5 +90,5 @@ run_tests ()
 void
 _initialize_optional_selftests ()
 {
-  selftests::register_test (selftests::optional::run_tests);
+  selftests::register_test ("optional", selftests::optional::run_tests);
 }
diff --git a/gdb/unittests/scoped_restore-selftests.c b/gdb/unittests/scoped_restore-selftests.c
index ea7492b..bc9aa2b 100644
--- a/gdb/unittests/scoped_restore-selftests.c
+++ b/gdb/unittests/scoped_restore-selftests.c
@@ -106,5 +106,6 @@ run_tests ()
 void
 _initialize_scoped_restore_selftests ()
 {
-  selftests::register_test (selftests::scoped_restore_tests::run_tests);
+  selftests::register_test ("scoped_restore",
+			    selftests::scoped_restore_tests::run_tests);
 }
diff --git a/gdb/utils-selftests.c b/gdb/utils-selftests.c
index 08feac6..5a30a93 100644
--- a/gdb/utils-selftests.c
+++ b/gdb/utils-selftests.c
@@ -55,6 +55,6 @@ void
 _initialize_utils_selftests (void)
 {
 #if GDB_SELF_TEST
-  selftests::register_test (selftests::common_utils_tests);
+  selftests::register_test ("common-utils", selftests::common_utils_tests);
 #endif
 }
diff --git a/gdb/utils.c b/gdb/utils.c
index f2da2df..8d86b55 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3289,6 +3289,6 @@ _initialize_utils (void)
   add_internal_problem_command (&demangler_warning_problem);
 
 #if GDB_SELF_TEST
-  selftests::register_test (gdb_realpath_tests);
+  selftests::register_test ("gdb_realpath", gdb_realpath_tests);
 #endif
 }
-- 
2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add selftests run filtering
  2017-09-07 15:09     ` Simon Marchi
@ 2017-09-07 15:11       ` Simon Marchi
  2017-09-16 12:08         ` Simon Marchi
  0 siblings, 1 reply; 10+ messages in thread
From: Simon Marchi @ 2017-09-07 15:11 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, palves, eliz

On 2017-09-07 17:09, Simon Marchi wrote:
> New in v3:
> 
> I realized I had forgotten help, news and doc, this patch adds those.
> 
> Actual commit log:
> 
> With the growing number of selftests, I think it would be useful to be
> able to run only a subset of the tests.  This patch associates a name 
> to
> each registered selftest.  It then allows doing something like:
> 
>   (gdb) maintenance selftest aarch64
>   Running self-tests.
>   Running selftest aarch64-analyze-prologue.
>   Running selftest aarch64-process-record.
>   Ran 2 unit tests, 0 failed
> 
> or with gdbserver:
> 
>   ./gdbserver --selftest=aarch64
> 
> In both cases, only the tests that contain "aarch64" in their name are
> ran.  To help validate that the tests you want to run were actually 
> ran,
> it also prints a message with the test name before running each test.
> 
> Right now, all the arch-dependent tests are registered as a single test
> of the selftests.  To be able to filter those too, I made them
> "first-class citizen" selftests.  The selftest type is an interface,
> with different implementations for "simple selftests" and "arch
> selftests".  The run_tests function simply iterates on that an invokes
> operator() on each test.
> 
> I changed the tests data structure from a vector to a map, because
> 
>   - it allows iterating in a stable (alphabetical) order
>   - it allows to easily verify if a test with a given name has been
>     registered, to avoid duplicates
> 
> There's also a new command "maintenance info selftests" that lists the
> registered selftests.

Arg, the subject should have said v3.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add selftests run filtering
  2017-09-07 15:11       ` [PATCH v3] " Simon Marchi
@ 2017-09-16 12:08         ` Simon Marchi
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Marchi @ 2017-09-16 12:08 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, palves, eliz

On 2017-09-07 17:11, Simon Marchi wrote:
> On 2017-09-07 17:09, Simon Marchi wrote:
>> New in v3:
>> 
>> I realized I had forgotten help, news and doc, this patch adds those.
>> 
>> Actual commit log:
>> 
>> With the growing number of selftests, I think it would be useful to be
>> able to run only a subset of the tests.  This patch associates a name 
>> to
>> each registered selftest.  It then allows doing something like:
>> 
>>   (gdb) maintenance selftest aarch64
>>   Running self-tests.
>>   Running selftest aarch64-analyze-prologue.
>>   Running selftest aarch64-process-record.
>>   Ran 2 unit tests, 0 failed
>> 
>> or with gdbserver:
>> 
>>   ./gdbserver --selftest=aarch64
>> 
>> In both cases, only the tests that contain "aarch64" in their name are
>> ran.  To help validate that the tests you want to run were actually 
>> ran,
>> it also prints a message with the test name before running each test.
>> 
>> Right now, all the arch-dependent tests are registered as a single 
>> test
>> of the selftests.  To be able to filter those too, I made them
>> "first-class citizen" selftests.  The selftest type is an interface,
>> with different implementations for "simple selftests" and "arch
>> selftests".  The run_tests function simply iterates on that an invokes
>> operator() on each test.
>> 
>> I changed the tests data structure from a vector to a map, because
>> 
>>   - it allows iterating in a stable (alphabetical) order
>>   - it allows to easily verify if a test with a given name has been
>>     registered, to avoid duplicates
>> 
>> There's also a new command "maintenance info selftests" that lists the
>> registered selftests.
> 
> Arg, the subject should have said v3.

I pushed this patch, but I am still open to making changes if you have 
further comments.

Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-09-16 12:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05 11:51 [PATCH] Add selftests run filtering Simon Marchi
2017-09-06 15:25 ` Pedro Alves
2017-09-06 18:38   ` Simon Marchi
2017-09-06 18:41     ` Simon Marchi
2017-09-06 18:44       ` Simon Marchi
2017-09-06 18:49         ` Pedro Alves
2017-09-06 21:21   ` [PATCH v2] " Simon Marchi
2017-09-07 15:09     ` Simon Marchi
2017-09-07 15:11       ` [PATCH v3] " Simon Marchi
2017-09-16 12:08         ` Simon Marchi

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 read-only IMAP folder(s) and NNTP newsgroup(s).