Re: [PATCH v5 1/5] iotests: remove 'linux' from default supported platforms

[Thomas Huth <thuth@redhat.com>]

On 01/10/2019 20.44, Max Reitz wrote:
[...]
> As I have said, the conceptual problem is that the iotests now run as
> part of make check.  As such, allowing auto tests to run on non-Linux
> platforms may introduce build failures that I cannot do anything about.

Well, simply run "make vm-build-openbsd", "make vm-build-freebsd", ...
if something fails there, it likely should not be in the auto group.

> And those are very much new failures.
> 
>> I think that I have demonstrated sufficiently that it's not correct to
>> prohibit python tests from running on other platforms wholesale, so I'd
>> prefer we don't do that anymore.
> 
> You have not.
> 
> The actual argument to convince me is “This does not affect any tests in
> the auto group, so it will not introduce build failures at this time”.

I've applied the patch here and it works fine with FreeBSD and macOS:

 https://cirrus-ci.com/build/5169384718336000
 https://travis-ci.com/huth/qemu/builds/129968676

It also works fine with "make vm-build-openbsd BUILD_TARGET=check-block"
... so I think you don't have to worry here.

>> Further, iotests on FreeBSD already weren't 100% green, so I'm not
>> causing a regression in that sense, either.
> 
> My problem is twofold:
> 
> (1) You claim “Sure, there are failures, but let’s just deal with them”
> and then you do not deal with them.  Seems wrong to me.
> 
> I’m fine with the argument “Sorry, royal ‘we’.  But it just doesn’t help
> anyone to hide the errors.  If someone’s on BSD and wants to run the
> iotests, let them.”
> 
> That sounds good to me.
> 
> (2) Maybe someone adds a Python test in the future that is in auto and
> that does not specify Linux as the only supported platform.  Then I send
> a pull request and it breaks on macOS.  Now what?  Remove it from auto?
>  Blindly put "macOS" in unsupported platforms?

I think both solutions are good. If a test does not work on all systems,
it either should not be in the "auto" group, or it needs to be modified
to skip testing when running in an unsupported environment.

> In any case, it’ll be a problem for no good reason.

Really? Is "more test coverage" not a good reason?

> More on that in the next chunk.
> 
>> I'm going to meekly push and ask that we stage this as-is, and when
>> something bad happens you can remind me that I wanted this and make me
>> do it.
> 
> Make you do what?  Deal with the fact that a pull request is rejected
> because a test fails on macOS?
> 
> This is precisely the kind of problem I already had with adding the
> iotests to make check, and I’m already very much not happy about it.
> (In that case it was $DISPLAY not being set on Peter’s test system.)
> 
> I’ll let you make the deduction of “The problem isn’t allowing the
> iotests to run on non-Linux platforms, but the fact that they run in
> make check” yourself, so that I no longer feel like I’m the only one who
> considers that having been a mistake.

Max, I can understand that you are a little bit annoyed that this "make
check with iotests" caused some extra hurdles for you. But honestly,
removing that again would be quite egoistic by you. Try to put yourself
into the position of a "normal", non-blocklayer-maintainer QEMU
developer. For me, iotests were a *constant* source of frustration.
Often, when I ran them on top of my latest and greatest patches, to
check whether I caused a regression, the iotests simply failed. Then I
had to start debugging - did my patches cause the break, or is "master"
broken, too? In almost all cases, there was an issue in the master
branch already, either because they were failing on s390x, or because I
was using ext4 instead of xfs, or because I was using an older distro
than you, etc... . So while the iotests likely worked fine for the
limited set of systems that you, Kevin and the other hard-core block
layer developers are using, it's constantly broken for everybody else
who is not using the very same setup as you. The only way of *not*
getting upset about the iotests was to simply completely *ignore* them.
Is it that what you want?

Or maybe let me phrase it differently: Do you consider the iotests as
something that is more or less "private" to the hard-core block layer
developers, and it's ok if others completely ignore them and break them
by accident (and you also don't expect the normal developers to fix the
iotests afterwards)? Then sure, please go ahead and remove the iotests
from "make check" again. Maybe I just understood the idea of having
common tests in the repository wrong (or maybe the iotests should be
moved to a separate repository instead, so that the normal QEMU
developers do not get in touch with them anymore?) ... Otherwise, I
think it was the right step to add them "make check" so that everybody
now *has* to run at least a basic set of the iotests now before they can
their patches merged.

 Thomas


PS: Sorry, if my mail sounded a little bit harsh... but I really had
quite some frustration with the iotests in the past already.