Re: [Qemu-devel] [PATCH] Acceptance tests: host arch to target arch name mapping

[Cleber Rosa]

On 10/17/18 5:16 PM, Eduardo Habkost wrote:
> On Wed, Oct 17, 2018 at 05:33:30PM -0300, Wainer dos Santos Moschetta wrote:
>>
>> On 10/17/2018 05:05 PM, Eduardo Habkost wrote:
>>> On Wed, Oct 17, 2018 at 04:43:15PM -0300, Murilo Opsfelder Araujo wrote:
>>>> On Wed, Oct 17, 2018 at 07:40:51PM +0100, Peter Maydell wrote:
>>>>> On 17 October 2018 at 18:38, Cleber Rosa <address@hidden> wrote:
>>>>>>
>>>>>> On 10/17/18 12:29 PM, Eduardo Habkost wrote:
>>>>>>> On Wed, Oct 17, 2018 at 01:34:41PM +0100, Peter Maydell wrote:
>>>>>>>> So, why does the test code need to care? It's not clear
>>>>>>>> from the patch... My expectation would be that you'd
>>>>>>>> just test all the testable target architectures,
>>>>>>>> regardless of what the host architecture is.
>>>>>>> I tend to agree.  Maybe the right solution is to get rid of the
>>>>>>> os.uname().  I think the default should be testing all QEMU
>>>>>>> binaries that were built, and the host architecture shouldn't
>>>>>>> matter.
>>>>> Yes, looking at os.uname() also seems like an odd thing
>>>>> for the tests to be doing here. The test framework
>>>>> should be as far as possible host-architecture agnostic.
>>>>> (For some of the KVM cases there probably is a need to
>>>>> care, but those are exceptions, not the rule.)
>>>>>
>>>>>> I'm in favor of exercising all built targets, but that seems to me to be
>>>>>> on another layer, above the test themselves. This change is about the
>>>>>> behavior of a test when not told about the target arch (and thus binary)
>>>>>> it should use.
>>>>> At that level, I think the right answer is "tell the user
>>>>> they need to specify the qemu executable they are trying to test".
>>>>> In particular, there is no guarantee that the user has actually
>>>>> built the executable for the target that corresponds to the
>>>>> host, so it doesn't work to try to default to that anyway.
>>>>>
>>>>> thanks
>>>>> -- PMM
>>>>>
>>>> I agree with Peter.  We can make qemu_bin parameter mandatory.  If it is 
>>>> not
>>>> given, error out.  Trying to guess it based on host architecture turns out 
>>>> to be
>>>> troublesome.
>>>>
>>>> If we decide to follow this approach of not guessing QEMU binary, we should
>>>> update docs/devel/testing.rst to make it crystal clear qemu_bin parameter 
>>>> is
>>>> mandatory.
>>> That's not a perfect solution to me, but it sounds better than
>>> using uname() and silently making a decision for the user.
>>>
>>
>> As someone that have been writing acceptance tests recently, I find very
>> convenient it picking the qemu binary from current build automatically.
> 
> Picking the QEMU binary(ies) from the current build is a good
> idea.
> 
> Picking one one arbitrary QEMU binary from the build (e.g. using
> uname) and ignoring all the others silently may cause confusion.
> 
> At least iotests doesn't do that silently: it prints a message
> indicating what exactly is being tested, so people know only
> one specific QEMU binary is being tested.
> 

This is also done in the acceptance tests, the difference is the amount
of logging that by default goes to the UI (and/or the format).  But it's
always in the log:

$ avocado --show=test run tests/acceptance/version.py | grep qemu_bin
PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64)
=> 'x86_64-softmmu/qemu-system-x86_64'

>>
>> On the other hand, as an CI system or tester (I presume), I would want to
>> run tests on all target built. Or at least have control over it.
> 
> I'm not thinking of CI systems, but of the confusion that may
> arise from a developer doing this on his machine:
> 
> 1) Developer runs "./configure --target-list=x86_64-softmmu,ppc64-softmmu"
>    on his x86_64 workstation.
> 2) Developer implements a change that breaks qemu-system-ppc64 crash.
> 3) Developer runs "make".
> 4) Developer runs "avocado run tests/acceptance" or
>    "./tests/acceptance/mytest.py".

IMO, this will be done by someone writing mytest.py, and not by someone
hacking QEMU.  Someone hacking QEMU will run "make check-acceptance"...

> 5) Avocado runs tests for qemu-system-x86_64 only.

Which as I said before, is higher level and can be changed to run on all
built targets.  So with that, *this* doesn't happen.

> 6) Developer incorrectly believes everything is fine.
> 
> 

And neither does that.  The error will be flagged.

Again, I just think it's a layer issue and using one simple IMO default
setting at the test level.

- Cleber.

>>
>> I don't know if the architectures in question are so broadly used on
>> workstations that would justify this patch. Maybe we can stick with current
>> uname() approach (and if not able to find the binary, well, you should point
>> do it), and rather introduce a mechanism for running tests against several
>> targets (satisfying CI needs)?
>>
>> I hope it helps,
>> Wainer.
>