Re: [PATCH v2 5/5] tests/functional: Adapt reverse_debugging to run w/o Avocado

[Gustavo Romero <gustavo.romero@linaro.org>]

Hi Daniel!

On 9/8/25 06:16, Daniel P. Berrangé wrote:
> On Thu, Sep 04, 2025 at 03:46:40PM +0000, Gustavo Romero wrote:
>> This commit removes Avocado as a dependency for running the
>> reverse_debugging test.
>>
>> The main benefit, beyond eliminating an extra dependency, is that there
>> is no longer any need to handle GDB packets manually. This removes the
>> need for ad-hoc functions dealing with endianness and arch-specific
>> register numbers, making the test easier to read. The timeout variable
>> is also removed, since Meson now manages timeouts automatically.
>>
>> The reverse_debugging test is now executed through a runner, because it
>> requires GDB in addition to QMP. The runner is responsible for invoking
>> GDB with the appropriate arguments and for passing the test script to
>> GDB.
>>
>> Since the test now runs inside GDB, its output, particularly from
>> 'stepi' commands, which cannot be disabled, interleaves with the TAP
>> output from unittest. To avoid this conflict, the test no longer uses
>> Meson’s 'tap' protocol and instead relies on the simpler 'exitcode'
>> protocol.
>>
>> reverse_debugging is kept "skipped" for aarch64, ppc64, and x86_64, so
>> won't run unless QEMU_TEST_FLAKY_TESTS=1 is set in the test environment,
>> before running 'make check-functional' or 'meson test [...]'.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> ---
>>   tests/functional/aarch64/meson.build          |   7 +-
>>   .../functional/aarch64/test_reverse_debug.py  |  15 +-
>>   tests/functional/ppc64/meson.build            |   7 +-
>>   tests/functional/ppc64/test_reverse_debug.py  |  17 +-
>>   tests/functional/reverse_debugging.py         | 175 +++++++++---------
>>   tests/functional/x86_64/meson.build           |   7 +-
>>   tests/functional/x86_64/test_reverse_debug.py |  21 +--
>>   7 files changed, 131 insertions(+), 118 deletions(-)
>>
>> diff --git a/tests/functional/aarch64/meson.build b/tests/functional/aarch64/meson.build
>> index 04846c6eb1..4f3168dd55 100644
>> --- a/tests/functional/aarch64/meson.build
>> +++ b/tests/functional/aarch64/meson.build
>> @@ -32,7 +32,6 @@ tests_aarch64_system_thorough = [
>>     'raspi3',
>>     'raspi4',
>>     'replay',
>> -  'reverse_debug',
>>     'rme_virt',
>>     'rme_sbsaref',
>>     'sbsaref',
>> @@ -46,3 +45,9 @@ tests_aarch64_system_thorough = [
>>     'xen',
>>     'xlnx_versal',
>>   ]
>> +
>> +if is_variable('gdb_runner')
>> +tests_aarch64_system_thorough_with_runner = [
>> +    ['reverse_debug', gdb_runner, 'exitcode'],
>> +]
>> +endif
> 
> Why do you need to replace the of 'tap' with 'exitcode' for this script ?
> 
> I don't like the idea that a subset of our functional tests are not using
> the TAP protocol.

Unfortunately, the GDB Python API seems to be broken regarding being able
to redirect GDB's output to a file (as it's possible to do via GDB's prompt)
so we can't redirect the printed addresses that are printed after each "stepi"
command in the test script. As a consequence Meson's TAP gets confused with
it and fails the test. Since I could not find any directive in QEMU functional
tests that forbid using the classic exitcode, I used it, at least while
the GDB redirect is not fixed.

Why don't you like it?


Cheers,
Gustavo