Re: [PATCH 14/25] [RFC] GDBserver self test

[Yao Qi <qiyaoltc@gmail.com>]

Pedro Alves <palves@redhat.com> writes:

> The name of the corresponding gdb command is "maint selftest",
> i.e., no hyphen.  If wonder if we should be consistent.
>

I chose "--self-test" because gcc uses "-fself-test".  I am fine to use
"--selftest" to align with GDB command "maint selftest".

>> 
>> gdb/gdbserver:
>> 
>> 2017-05-26  Yao Qi  <yao.qi@linaro.org>
>> 
>> 	* configure.ac: AC_DEFINE GDB_SELF_TEST if $development.
>> 	* configure, config.in: Re-generated.
>> 	* server.c: Include sefltest.h and selftest.c.
>> 	(captured_main): Handle option --self-test.
>> gdb:
>> 
>> 2017-05-26  Yao Qi  <yao.qi@linaro.org>
>> 
>> 	* selftest.c: Adjust it for GDBserver.
>> 
>> gdb/testsuite:
>> 
>> 2017-05-26  Yao Qi  <yao.qi@linaro.org>
>> 
>> 	* gdb.server/unittest.exp: New.
>
> I like the idea of running unit tests in gdbserver
> (and we briefly discussed it when we introduced selftest-arch.c
> IIRC).

Yes, we discussed this _idea_ before, but we didn't know how to do the
unit tests in GDBserver.  I remembered that some one said GDBserver unit
tests can be triggered by a special packet sent by GDB.  When I wrote
this patch, it was still an open question to me.  I chose a new command
line option to trigger the tests, post this patch, and see if people
like this way.

>
> But I don't know whether you're proposing the patch as
> is, or whether it's really just for comments on the idea.

The latter.  The whole patch series is about collecting comments on the
flexible target description, and GDBserver unit tests are used to make
sure my patches don't break target descriptions on GDBserver.  It is of
hacks, and I'll fix them in v3, definitely.

> I say this because there are several hacks in the patch
> that ideally we'd avoid.
>
>> +#include "../selftest.h"
>> +#include "../selftest.c"
>
> E.g., we should really move these to common/ and void
> the #ifdefery within them.
>

Yes, I'll move them to common/.

>> +global server_spawn_id
>> +
>> +set gdbserver [find_gdbserver]
>> +set gdbserver_command "$gdbserver --self-test"
>> +
>> +set server_spawn_id [remote_spawn target $gdbserver_command]
>> +
>> +gdb_expect {
>> +    -i $server_spawn_id
>> +    -re "Ran $decimal unit tests, 0 failed" {
>> +	pass "unit tests"
>> +    }
>> +    -re "Ran $decimal unit tests, $decimal failed" {
>> +	fail "unit tests"
>> +    }
>
> Shouldn't this have a "default" case that fails instead
> of only handling one specific failure mode?

OK, I'll fix it too.

-- 
Yao (齐尧)