This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Add selftests run filtering


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]