[PATCH] iotests: Do not run the iotests during "make check" anymore

[PATCH] iotests: Do not run the iotests during "make check" anymore

[Thomas Huth]

Running the iotests during "make check" is causing more headaches than
benefits for the block layer maintainers, so let's disable the iotests
during "make check" again.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include   | 2 +-
 tests/qemu-iotests/group | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3543451ed3..5d19f39ee7 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1172,7 +1172,7 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
 check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qapi-schema/doc-good.texi
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-block: $(patsubst %,check-%, $(check-block-y))
-check: check-block check-qapi-schema check-unit check-softfloat check-qtest check-decodetree
+check: check-qapi-schema check-unit check-softfloat check-qtest check-decodetree
 check-clean:
 	rm -rf $(check-unit-y) tests/*.o $(QEMU_IOTESTS_HELPERS-y)
 	rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), $(check-qtest-$(target)-y)) $(check-qtest-generic-y))
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 5d3da937e4..246cf9aa65 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -10,7 +10,7 @@
 #
 # - img : Tests in this group can be used to excercise the qemu-img tool.
 #
-# - auto : Tests in this group are used during "make check" and should be
+# - auto : Tests in this group are usable in all environments and should be
 #   runnable in any case. That means they should run with every QEMU binary
 #   (also non-x86), with every QEMU configuration (i.e. must not fail if
 #   an optional feature is not compiled in - but reporting a "skip" is ok),
-- 
2.18.1

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[Max Reitz]

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

On 02.10.19 16:21, Thomas Huth wrote:
> Running the iotests during "make check" is causing more headaches than
> benefits for the block layer maintainers, so let's disable the iotests
> during "make check" again.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/Makefile.include   | 2 +-
>  tests/qemu-iotests/group | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

I’ll leave this patch on the list for a while so that others can speak
up if they disagree.

The problem is that this causes headaches of rather high importance
(make check), and I have to deal with them.  I’ll gladly accept help in
dealing with them, but without such help, I don’t feel like it’s worth it.

As far as I’m aware, the plan is to still the iotests in e.g. Travis,
and maybe even more thoroughly then (because we won’t be as constrained
by the test duration), just not in make check.

Max


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

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[Daniel P. Berrangé]

On Wed, Oct 02, 2019 at 04:21:46PM +0200, Thomas Huth wrote:
> Running the iotests during "make check" is causing more headaches than
> benefits for the block layer maintainers, so let's disable the iotests
> during "make check" again.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/Makefile.include   | 2 +-
>  tests/qemu-iotests/group | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

I don't have any objection to removing from 'make check', but I feel
like this commit should be modifying the travis.yml config so that
it explicitly runs the block tests, otherwise we're loosing automated
CI and the block tests will increase their rate of bitrot again.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[Thomas Huth]

On 02/10/2019 17.03, Daniel P. Berrangé wrote:
> On Wed, Oct 02, 2019 at 04:21:46PM +0200, Thomas Huth wrote:
>> Running the iotests during "make check" is causing more headaches than
>> benefits for the block layer maintainers, so let's disable the iotests
>> during "make check" again.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  tests/Makefile.include   | 2 +-
>>  tests/qemu-iotests/group | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> I don't have any objection to removing from 'make check', but I feel
> like this commit should be modifying the travis.yml config so that
> it explicitly runs the block tests, otherwise we're loosing automated
> CI and the block tests will increase their rate of bitrot again.

I was planning to send a separate patch for that (once my Travis builds
are through...), but if it is preferred, I can also send a v2 of this
patch here where I include that change.

Max, any preferences?

 Thomas

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[Max Reitz]

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

On 02.10.19 17:10, Thomas Huth wrote:
> On 02/10/2019 17.03, Daniel P. Berrangé wrote:
>> On Wed, Oct 02, 2019 at 04:21:46PM +0200, Thomas Huth wrote:
>>> Running the iotests during "make check" is causing more headaches than
>>> benefits for the block layer maintainers, so let's disable the iotests
>>> during "make check" again.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  tests/Makefile.include   | 2 +-
>>>  tests/qemu-iotests/group | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> I don't have any objection to removing from 'make check', but I feel
>> like this commit should be modifying the travis.yml config so that
>> it explicitly runs the block tests, otherwise we're loosing automated
>> CI and the block tests will increase their rate of bitrot again.
> 
> I was planning to send a separate patch for that (once my Travis builds
> are through...), but if it is preferred, I can also send a v2 of this
> patch here where I include that change.
> 
> Max, any preferences?

I don’t mind either way.  I don’t think we’re in danger of you
forgetting to send the Travis patch.

As for running the tests on macOS: Good question.  I’d just let them run
now and maybe see later whether that decision hurts.  macOS has its own
filesystem, so it may be worth testing there.

Max


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

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[John Snow]

On 10/2/19 11:50 AM, Max Reitz wrote:
> On 02.10.19 17:10, Thomas Huth wrote:
>> On 02/10/2019 17.03, Daniel P. Berrangé wrote:
>>> On Wed, Oct 02, 2019 at 04:21:46PM +0200, Thomas Huth wrote:
>>>> Running the iotests during "make check" is causing more headaches than
>>>> benefits for the block layer maintainers, so let's disable the iotests
>>>> during "make check" again.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>  tests/Makefile.include   | 2 +-
>>>>  tests/qemu-iotests/group | 2 +-
>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> I don't have any objection to removing from 'make check', but I feel
>>> like this commit should be modifying the travis.yml config so that
>>> it explicitly runs the block tests, otherwise we're loosing automated
>>> CI and the block tests will increase their rate of bitrot again.
>>
>> I was planning to send a separate patch for that (once my Travis builds
>> are through...), but if it is preferred, I can also send a v2 of this
>> patch here where I include that change.
>>
>> Max, any preferences?
> 
> I don’t mind either way.  I don’t think we’re in danger of you
> forgetting to send the Travis patch.
> 
> As for running the tests on macOS: Good question.  I’d just let them run
> now and maybe see later whether that decision hurts.  macOS has its own
> filesystem, so it may be worth testing there.
> 
> Max
> 

There are absolutely known bugs and problems using APFS that we have not
fixed.

--js

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[Max Reitz]

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

On 03.10.19 01:51, John Snow wrote:
> 
> 
> On 10/2/19 11:50 AM, Max Reitz wrote:
>> On 02.10.19 17:10, Thomas Huth wrote:
>>> On 02/10/2019 17.03, Daniel P. Berrangé wrote:
>>>> On Wed, Oct 02, 2019 at 04:21:46PM +0200, Thomas Huth wrote:
>>>>> Running the iotests during "make check" is causing more headaches than
>>>>> benefits for the block layer maintainers, so let's disable the iotests
>>>>> during "make check" again.
>>>>>
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>  tests/Makefile.include   | 2 +-
>>>>>  tests/qemu-iotests/group | 2 +-
>>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> I don't have any objection to removing from 'make check', but I feel
>>>> like this commit should be modifying the travis.yml config so that
>>>> it explicitly runs the block tests, otherwise we're loosing automated
>>>> CI and the block tests will increase their rate of bitrot again.
>>>
>>> I was planning to send a separate patch for that (once my Travis builds
>>> are through...), but if it is preferred, I can also send a v2 of this
>>> patch here where I include that change.
>>>
>>> Max, any preferences?
>>
>> I don’t mind either way.  I don’t think we’re in danger of you
>> forgetting to send the Travis patch.
>>
>> As for running the tests on macOS: Good question.  I’d just let them run
>> now and maybe see later whether that decision hurts.  macOS has its own
>> filesystem, so it may be worth testing there.
>>
>> Max
>>
> 
> There are absolutely known bugs and problems using APFS that we have not
> fixed.

Sorry, somehow missed your reply. :-/

Yes, that was the idea why I said that maybe running the tests there
would be worth it, because it’s a different FS that produces, well,
interesting results.

But on second thought, who’s going to address those problems?  So, yeah,
that wouldn’t really help.

Max


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

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[John Snow]

On 10/7/19 9:03 AM, Max Reitz wrote:
> On 03.10.19 01:51, John Snow wrote:
>>
>>
>> On 10/2/19 11:50 AM, Max Reitz wrote:
>>> On 02.10.19 17:10, Thomas Huth wrote:
>>>> On 02/10/2019 17.03, Daniel P. Berrangé wrote:
>>>>> On Wed, Oct 02, 2019 at 04:21:46PM +0200, Thomas Huth wrote:
>>>>>> Running the iotests during "make check" is causing more headaches than
>>>>>> benefits for the block layer maintainers, so let's disable the iotests
>>>>>> during "make check" again.
>>>>>>
>>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>>> ---
>>>>>>  tests/Makefile.include   | 2 +-
>>>>>>  tests/qemu-iotests/group | 2 +-
>>>>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> I don't have any objection to removing from 'make check', but I feel
>>>>> like this commit should be modifying the travis.yml config so that
>>>>> it explicitly runs the block tests, otherwise we're loosing automated
>>>>> CI and the block tests will increase their rate of bitrot again.
>>>>
>>>> I was planning to send a separate patch for that (once my Travis builds
>>>> are through...), but if it is preferred, I can also send a v2 of this
>>>> patch here where I include that change.
>>>>
>>>> Max, any preferences?
>>>
>>> I don’t mind either way.  I don’t think we’re in danger of you
>>> forgetting to send the Travis patch.
>>>
>>> As for running the tests on macOS: Good question.  I’d just let them run
>>> now and maybe see later whether that decision hurts.  macOS has its own
>>> filesystem, so it may be worth testing there.
>>>
>>> Max
>>>
>>
>> There are absolutely known bugs and problems using APFS that we have not
>> fixed.
> 
> Sorry, somehow missed your reply. :-/
> 
> Yes, that was the idea why I said that maybe running the tests there
> would be worth it, because it’s a different FS that produces, well,
> interesting results.
> 
> But on second thought, who’s going to address those problems?  So, yeah,
> that wouldn’t really help.
> 
> Max
> 

Yeah, the problem with APFS is I don't have access to using it to debug
any of those problems, so they stay broken and nobody has stepped up to
submit patches. :(

--js

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[Alex Bennée]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, Oct 02, 2019 at 04:21:46PM +0200, Thomas Huth wrote:
>> Running the iotests during "make check" is causing more headaches than
>> benefits for the block layer maintainers, so let's disable the iotests
>> during "make check" again.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  tests/Makefile.include   | 2 +-
>>  tests/qemu-iotests/group | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> I don't have any objection to removing from 'make check', but I feel
> like this commit should be modifying the travis.yml config so that
> it explicitly runs the block tests, otherwise we're loosing automated
> CI and the block tests will increase their rate of bitrot again.

I think we run a subset on gitlab as well. Do the iotests need any
particular build of QEMU? Lets try and avoid adding unneeded targets.

I must admit I've been out of the loop here. What headaches are they
causing? Too many false positives?


>
>
> Regards,
> Daniel


--
Alex Bennée

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[Thomas Huth]

On 02/10/2019 19.32, Alex Bennée wrote:
> 
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
>> On Wed, Oct 02, 2019 at 04:21:46PM +0200, Thomas Huth wrote:
>>> Running the iotests during "make check" is causing more headaches than
>>> benefits for the block layer maintainers, so let's disable the iotests
>>> during "make check" again.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  tests/Makefile.include   | 2 +-
>>>  tests/qemu-iotests/group | 2 +-
>>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> I don't have any objection to removing from 'make check', but I feel
>> like this commit should be modifying the travis.yml config so that
>> it explicitly runs the block tests, otherwise we're loosing automated
>> CI and the block tests will increase their rate of bitrot again.
> 
> I think we run a subset on gitlab as well. Do the iotests need any
> particular build of QEMU? Lets try and avoid adding unneeded targets.

The tests that are currently specified in .gitlab-ci.yml explicitly are
only tests that are not part of the "auto" group, i.e. not part of "make
check-block". So if we just run "make check-block" in Travis, we do not
have any overlap.

> I must admit I've been out of the loop here. What headaches are they
> causing? Too many false positives?

See this thread here:

https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg00288.html

 Thomas

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore

[Max Reitz]

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

On 02.10.19 16:21, Thomas Huth wrote:
> Running the iotests during "make check" is causing more headaches than
> benefits for the block layer maintainers, so let's disable the iotests
> during "make check" again.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/Makefile.include   | 2 +-
>  tests/qemu-iotests/group | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

So the background behind this patch is that I continued to
complain/whine until I had a short open discussion with Thomas in which
he agreed to send this patch.  My points were:

(1) It doesn’t seem like people care too much about this.

(2) It isn’t very useful, because we run so few tests, and they don’t
seem to be the critical ones.

(3) In the past months, I feel like I was the single person of contact
when it comes to iotests breaking, and given the above I don’t feel like
having to act immediately on a broken make check is a good use of my
time (there’s always too much to do, so I do have to prioritize (like
everyone else)).


I’d hoped that this patch would provoke people that disagree with (1) or
(2), and potentially help me out to alleviate (3).  Or maybe provoke
nobody, in which case (1) would have been confirmed.


That didn’t quite happen, but Kevin and Peter decided to reply to my
original discussion with Thomas.

From what they’ve said I gather that (1) and (2) are wrong, and I assume
that Kevin will as the/a block maintainer have the same responsibility
as me when it comes to (3).

As such, while I can’t NAK this patch in their name, I can say that I no
longer see a need for this patch, because the reasons for why I’ve
requested it have been shown to be wrong.

I assume that Thomas actually doesn’t want to see this patch merged, and
that Kevin won’t merge it either, so I think that effectively makes this
a “passive-NAK”.

Max


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