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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 8319 bytes --]

On 04.10.19 12:19, Kevin Wolf wrote:
> Am 02.10.2019 um 19:47 hat Max Reitz geschrieben:
>> On 02.10.19 18:44, Kevin Wolf wrote:
>>> Am 02.10.2019 um 13:57 hat Max Reitz geschrieben:
>>>> 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.
>>
>> OK.
>>
>> (In my experience, it’s still mostly block patches, only that they tend
>> to go through non-block trees.)
> 
> Fair enough, it's usually code that touches block code. I assumed "block
> patches" to mean patches that go through one of the block trees and for
> which iotests are run before sending a pull request.
> 
> In the end, I don't care what code these patches touched. I do an
> innocent git pull, and when I finally see that it's master that breaks
> iotests and not my patches on top of it, I'm annoyed.

Hm.  Part of my point was that this still happens all the time.

Which is why I’d prefer more tests to run in CI than a handful of not
very useful ones in make check.

(Of course, running it in a CI won’t prevent iotest failures on your
machine, but in practice neither does the current model.)

>>>> 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),

-x auto for the qcow2 pass, by the way.

>>> 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.
>>
>> Then I expect you to NACK Thomas’s patch, and I take it to mean that I
>> can rely on you to handle basically all make check iotests failures that
>> are not caused by my own pull requests.
>>
>> Works for me.
> 
> Ah, we can also play this game the other way round.
> 
> So if you merge that revert, when iotests break in master, I take it I
> can rely on you to fix it up before it impacts my working branch?

This is not a game, I’m talking purely from my standpoint:
(I talked wrongly, but more on that below)

Whenever make check fails, it’s urgent.  Without iotests running in make
check, we had some time to sort the situation out.  That’s no longer the
case.

That means we need to take care of everything immediately.  And I purely
want help with that.  I just did not have the impression that such help
was there in the past months.
(This impression may or may not have a correlation to reality.)

I thought that was just because nobody really cared about the iotests in
make check.  Now you say you do, so that’s why I said you should NACK
the revert and then I know that I can count on your help.


Now where I was wrong: I didn’t say “your help” but “you handle it”.
That was wrong.  I’m sorry.  That would mean I reap all the benefits and
you have to pay the price.


What I’m not so sure of is why you didn’t just say that’s unfair and
instead reply with an impossible suggestion.  That makes it a bit
difficult for me to find out exactly what you plan to do.

I take your point as “Exactly, this suggestion would be impossible.
With the tests in make check, the worst that happens is CI breakage and
rejected pull requests, and dealing with those is very much possible.”[1]

As I’ve said, my POV is different, because I find CI breakage, make
check breakage, and rejected pull requests to be worse because those are
failures on other people’s machines.  That puts me personally under much
more stress than failures on my own machine.  But it seems that you feel
differently.

[1] Or maybe you wanted to say that you find a rare intermittent failure
that slips into make check less worse than 100 % failure of some test
for everyone who runs the iotests.  I don’t know.
I know that the 100 % failures are annoying but generally easily fixed;
whereas the intermittent ones are the ones that stick and hard to fix.
And those intermittent ones are unlikely to cause pull requests to be
rejected.

>>>> 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.
>>
>> Sure, that was a good thing.  I just wonder whether that instance is
>> enough to justify the pain.
> 
> Yes, making iotests stable on CI probably involves some pain, especially
> initially. However, if we don't fix them upstream, they'll end up on our
> desk again downstream because people run tests neither on your nor on my
> laptop.
> 
> I think we might actually save time by fixing them once and for all,
> even if they are problems in the test cases and not in QEMU, and making
> new test cases portable from the start, instead of getting individual
> reports one at a time and having to look at the same test cases again
> and again.

You should really know I’m all for that and that I’ve done my share of
work there.

But from my perspective we’ve said and tried that for six years and we
aren’t there still.  So to me “We should just fix it” of course sounds
correct, but I also don’t believe it’ll happen any time soon.  Mostly
because we generally don’t know what to fix before it breaks, but also
because that’s exactly what we’ve tried to do for at least six years.


So, hm.  I don’t quite know where we’re going.

I still think that I personally would prefer the iotests to not run as
part of make check, but just as CI.

You do have a point that dropping the iotests from make check would
benefit me at your expense.  (Well, it’d be the same result for both of
us, it just appears that we have different perceptions of pain.)

OTOH, keeping the iotests in make check means we have to act on failure
reports immediately.  For example, someone™ needs to investigate the 130
failure Peter has reported.  I’ve just replied “I don’t know”, CC’d
Thomas, and waited whether anyone else would do anything.  Nobody did,
and that can’t work.  (I also wrote that I wouldn’t act on it, precisely
because I didn’t see the point.  I assumed that if anyone disagreed with
that statement, they would have said something.)

So acting on such reports means pain, too.  I consider it greater than
the previous kind of pain, because I prefer “This sucks, I’ll have to
fix it this week or so” to “Oh crap, I’ll have to investigate now,
reproduce it, write an email as soon as possible, and fix it”.

But at least we’ll all have this pain as long as we share it.  And then
it might not be so bad.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]