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

[Kevin Wolf]

Not sure where in this thread to reply, but since my name is mentioned
in this mail, it might at least be not the worst one.

Am 02.10.2019 um 13:57 hat Max Reitz geschrieben:
> On 02.10.19 06:46, Thomas Huth wrote:
> > 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.
> 
> Then we come to Windows and macOS.
> 
> What I’ve proposed to John on IRC was to simply skip the iotests in make
> check for non-Linux systems.

I think this really makes sense. Or at least have a blacklist of OSes
that we can't support iotests on, which would contain at least Windows
and OS X. Windows because I'm pretty sure that it doesn't work anyway,
and OS X because if something fails there, we have no way to reproduce.
The occasional build failures on OS X that must be fixed by blindly
guessing what could work without any way to test the fix are already bad
enough.

> > 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?
> 
> It usually worked fine for me because it’s rather rare that non-block
> patches broke the iotests.

I disagree. It happened all the time that someone else broke the iotests
in master and we needed to fix it up.

> Maybe my main problem is that I feel like now I have to deal with all of
> the fallout, even though adding the iotests to make check wasn’t my idea
> and neither me nor Kevin ever consented.  (I don’t know whether Kevin
> consented implicitly, but I don’t see his name in bdd95e47844f2d8b.)
> 
> You can’t just give me more responsibility without my consent and then
> call me egoistic for not liking it.

I think I may have implicitly consented by helping Alex with the changes
to 'check' that made its output fit better in 'make check'.

Basically, my stance is that I share your dislike for the effects on me
personally (also, I can't run 'make check' any more before a pull
request without testing half of the iotests twice because I still need a
full iotests run), but I also think that having iotests in 'make check'
is really the right thing for the project despite the inconvenience it
means for me.

> I know you’ll say that we just need to ensure we can add more tests,
> then.  But for one thing, the most important tests are the ones that
> take the longest, like 041.  And the other of course is that adding any
> more tests to make check just brings me more pain, so I won’t do it.

That's something that can be fixed by tweaking the auto group.

Can we run tests in 'auto' that require the system emulator? If so,
let's add 030 040 041 to the default set. They take long, but they are
among the most valuable tests we have. If this makes the tests take too
much time, let's remove some less important ones instead.

> [1] There is the recent case of Kevin’s pull request having been
> rejected because his test failed on anything but x64.  I’m torn here,
> because I would have seen that failure on my -m32 build.  So it isn’t
> like it would have evaded our view for long.

I messed up, so this pull request was rightly stopped. I consider this
one a feature, not a bug.

Kevin

signature.asc
Description: PGP signature