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 <[email protected]> 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.
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 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.
