Re: [PATCH] Add selftests run filtering

[Simon Marchi <simon.marchi@polymtl.ca>]

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