Re: [PATCH] Add selftests run filtering

Simon Marchi
2017-09-06T18:38:06Z

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