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

Thomas Huth posted 1 patch 4 years, 6 months ago
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test docker-clang@ubuntu failed
Test asan passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191002142146.6124-1-thuth@redhat.com
Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>
tests/Makefile.include   | 2 +-
tests/qemu-iotests/group | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
[PATCH] iotests: Do not run the iotests during "make check" anymore
Posted by Thomas Huth 4 years, 6 months ago
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
Posted by Max Reitz 4 years, 6 months ago
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

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore
Posted by Daniel P. Berrangé 4 years, 6 months ago
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
Posted by Thomas Huth 4 years, 6 months ago
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
Posted by Max Reitz 4 years, 6 months ago
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

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore
Posted by John Snow 4 years, 6 months ago

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
Posted by Max Reitz 4 years, 6 months ago
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

Re: [PATCH] iotests: Do not run the iotests during "make check" anymore
Posted by John Snow 4 years, 6 months ago

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
Posted by Alex Bennée 4 years, 6 months ago
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
Posted by Thomas Huth 4 years, 6 months ago
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
Posted by Max Reitz 4 years, 6 months ago
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