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

[Thomas Huth]

On 07/10/2019 14.52, Max Reitz wrote:
> On 07.10.19 14:16, Thomas Huth wrote:
>> On 04/10/2019 14.44, Max Reitz wrote:
>>> 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.
>>
>> Ok, so let's try to add some more useful test to the "auto" group. Kevin
>> mentioned 030, 040 and 041, and I think it should be ok to enable them
>> (IIRC the only issue was that they run a little bit longer, but if they
>> are very useful, we should include them anyway).
> 
> I agree on those.  (Maybe not 040, but definitely 030 and 041.)
> 
> Maybe one of the issues was the “path too long” thing for Unix sockets?

Ah, right. I've applied John's "remove 'linux' from default" patch and
added the three iotests to the "auto" group, and indeed, they fail now
on cirrus-ci due to the "path too long" socket problem. "We" (royal we,
I guess) should likely fix that first...

>> Do you have any other tests in mind which could be very useful?
> 
> I’d like a test for iothreads, it doesn’t look like there currently is
> one in auto.  (The problem of course is that they have a higher chance
> of being flaky, but I think they also have a higher probability of
> breaking because of non-block stuff.)
> 
> 127 and 256 look promising to me.
> 
> 
> Also, I don’t see any migration tests in auto (156 doesn’t really count).
> 
> The ones that looks interesting here are 091, 181 (but I seem to
> remember that 181 is flaky under load, I should investigate that), 183,
> and 203 (which also tests iothreads).
> 
> 
> Those are the two area that I spontaneously think of when considering
> what is more likely to be broken by non-block patches.  Unfortunately,
> those are also the two areas with the tests that tend to be the
> flakiest, I guess...

Ok, thanks for the list. I'll have a look at them whether it's feasible
to get them enabled...

>>> 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”.
>>
>> I think that "I have to investigate now ..." mindset is not the right
>> way to handle these kind of issues. If a test is shaky and can not be
>> fixed easily, we should simply disable it from the "auto" group again.
>> So if you like, I can send a patch to remove 130 from the "auto" group
>> (personally, I'd rather wait to see if it fails anytime soon again, or
>> if this was maybe rather a one-time issue due to a non-clean test system
>> ...)
> 
> Waiting for another failure sounds OK to me.  (OTOH, 130 doesn’t seem
> like something that needs to be in auto, in case we want to take
> something out to save time for more important tests.)
> 
> But that reminds me that iotest failures probably should be
> automatically signaled to me and Kevin instead of Peter having to write
> an email himself.  Would that be possible?

Not sure whether an automatic e-mail is feasible, but you could register
a github account to test with Travis (and maybe Cirrus-CI) before
sending pull requests to Peter, I guess that will catch most of the
issues on other machines already.

 Thomas

signature.asc
Description: OpenPGP digital signature