[RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore

Philippe Mathieu-Daudé posted 1 patch 3 years, 4 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201127174110.1932671-1-philmd@redhat.com
.gitlab-ci.yml | 9 +++++++++
1 file changed, 9 insertions(+)
[RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Philippe Mathieu-Daudé 3 years, 4 months ago
We lately realized that the Avocado framework was not designed
to be regularly run on CI environments. Therefore, as of 5.2
we deprecate the gitlab-ci jobs using Avocado. To not disrupt
current users, it is possible to keep the current behavior by
setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
(see [*]).
From now on, using these jobs (or adding new tests to them)
is strongly discouraged.

Tests based on Avocado will be ported to new job schemes during
the next releases, with better documentation and templates.

[*] https://docs.gitlab.com/ee/ci/variables/README.html#create-a-custom-variable-in-the-ui

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 .gitlab-ci.yml | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index d0173e82b16..2674407cd13 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -66,6 +66,15 @@ include:
     - cd build
     - python3 -c 'import json; r = json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for t in r["tests"] if t["status"] not in ("PASS", "SKIP", "CANCEL")]' | xargs cat
     - du -chs ${CI_PROJECT_DIR}/avocado-cache
+  rules:
+  # As of QEMU 5.2, Avocado is not yet ready to run in CI environments, therefore
+  # the jobs based on this template are not run automatically (except if the user
+  # explicitly sets the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE environment
+  # variable). Adding new jobs on top of this template is strongly discouraged.
+  - if: $QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE == null
+    when: manual
+    allow_failure: true
+  - when: always
 
 build-system-ubuntu:
   <<: *native_build_job_definition
-- 
2.26.2

Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Thomas Huth 3 years, 4 months ago
On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
> We lately realized that the Avocado framework was not designed
> to be regularly run on CI environments. Therefore, as of 5.2
> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> current users, it is possible to keep the current behavior by
> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> (see [*]).
> From now on, using these jobs (or adding new tests to them)
> is strongly discouraged.
> 
> Tests based on Avocado will be ported to new job schemes during
> the next releases, with better documentation and templates.

Why should we disable the jobs by default as long as there is no replacement
available yet?

 Thomas


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Philippe Mathieu-Daudé 3 years, 4 months ago
On 11/27/20 6:47 PM, Thomas Huth wrote:
> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
>> We lately realized that the Avocado framework was not designed
>> to be regularly run on CI environments. Therefore, as of 5.2
>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
>> current users, it is possible to keep the current behavior by
>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
>> (see [*]).
>> From now on, using these jobs (or adding new tests to them)
>> is strongly discouraged.
>>
>> Tests based on Avocado will be ported to new job schemes during
>> the next releases, with better documentation and templates.
> 
> Why should we disable the jobs by default as long as there is no replacement
> available yet?

Why keep it enabled if it is failing randomly, if images hardcoded
in tests are being removed from public servers, etc...?


They are not disabled, they are still runnable manually or setting
QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...

We realized by default Avocado runs all tests on the CI jobs,
triggering failures and complains. Developer stopped to contribute/
review integration tests because of that. We want developers be
able to contribute tests to the repository fearlessly.

If we don't change anything, we'll keep having CI failures due
to Avocado design issues (artifacts removed from remote servers,
etc...).

I haven't seen any attempt on this list to improve the current
fragile situation, but better suggestions are certainly welcome.

Thanks,

Phil.


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Thomas Huth 3 years, 4 months ago
On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:
> On 11/27/20 6:47 PM, Thomas Huth wrote:
>> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
>>> We lately realized that the Avocado framework was not designed
>>> to be regularly run on CI environments. Therefore, as of 5.2
>>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
>>> current users, it is possible to keep the current behavior by
>>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
>>> (see [*]).
>>> From now on, using these jobs (or adding new tests to them)
>>> is strongly discouraged.
>>>
>>> Tests based on Avocado will be ported to new job schemes during
>>> the next releases, with better documentation and templates.
>>
>> Why should we disable the jobs by default as long as there is no replacement
>> available yet?
> 
> Why keep it enabled if it is failing randomly

We can still disable single jobs if they are not stable, but that's no
reason to disable all of them by default, is it?

> if images hardcoded
> in tests are being removed from public servers, etc...?

That's independent from Avocado, you'll always have that problem if you want
to test with external images, unless you mirror them into a repository on
the same server (ie. gitlab), which, however, might not always be possible...

> They are not disabled, they are still runnable manually or setting
> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...

And who do you think is going to set that variable? Hardly anybody, I guess.
So you could also simply remove the stuff from the yml file completely instead.

> We realized by default Avocado runs all tests on the CI jobs,
> triggering failures and complains. Developer stopped to contribute/
> review integration tests because of that.

Did anybody really stop contributing "acceptance" test since they were
afraid of the gitlab-CI running them? That's new to me, do you have a pointer?

> We want developers be
> able to contribute tests to the repository fearlessly.

You can always mark your test with @skipIf(os.getenv('GITLAB_CI')) if you
don't want to see it running in the gitlab-CI, so that's not a reason to be
afraid.

> If we don't change anything, we'll keep having CI failures due
> to Avocado design issues (artifacts removed from remote servers,
> etc...).

I fail to see the relation between Avocado and vanishing artifacts from 3rd
party servers... what do you plan to do instead if something gets (re-)moved
from a server that is not under our control?

> I haven't seen any attempt on this list to improve the current
> fragile situation, but better suggestions are certainly welcome.

At least I am hoping that the "check-acceptance" tests will break a little
bit less often once Peter uses the gitlab-CI for his gating tests, too. That
will at least prevent that one of the tests gets completely broken by a new
merged pull request. Of course there's still the risk that tests only fail
occasionally due to new bugs, but that can also happen for all other test
suites (unit, qtest, iotests, ...), too.

 Thomas


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Philippe Mathieu-Daudé 3 years, 4 months ago
On 11/27/20 7:29 PM, Thomas Huth wrote:
> On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:
>> On 11/27/20 6:47 PM, Thomas Huth wrote:
>>> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
>>>> We lately realized that the Avocado framework was not designed
>>>> to be regularly run on CI environments. Therefore, as of 5.2
>>>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
>>>> current users, it is possible to keep the current behavior by
>>>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
>>>> (see [*]).
>>>> From now on, using these jobs (or adding new tests to them)
>>>> is strongly discouraged.
>>>>
>>>> Tests based on Avocado will be ported to new job schemes during
>>>> the next releases, with better documentation and templates.
>>>
>>> Why should we disable the jobs by default as long as there is no replacement
>>> available yet?
>>
>> Why keep it enabled if it is failing randomly
> 
> We can still disable single jobs if they are not stable, but that's no
> reason to disable all of them by default, is it?
> 
>> if images hardcoded
>> in tests are being removed from public servers, etc...?
> 
> That's independent from Avocado, you'll always have that problem if you want
> to test with external images, unless you mirror them into a repository on
> the same server (ie. gitlab), which, however, might not always be possible...
> 
>> They are not disabled, they are still runnable manually or setting
>> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...
> 
> And who do you think is going to set that variable? Hardly anybody, I guess.

Does that mean nobody cares about these tests?

> So you could also simply remove the stuff from the yml file completely instead.

Yes, I'd prefer that too, but Alex objected.

>> We realized by default Avocado runs all tests on the CI jobs,
>> triggering failures and complains. Developer stopped to contribute/
>> review integration tests because of that.
> 
> Did anybody really stop contributing "acceptance" test since they were
> afraid of the gitlab-CI running them? That's new to me, do you have a pointer?

No, but alternatively, how many tests were contributed / reviewed
last year?

>> We want developers be
>> able to contribute tests to the repository fearlessly.
> 
> You can always mark your test with @skipIf(os.getenv('GITLAB_CI')) if you
> don't want to see it running in the gitlab-CI, so that's not a reason to be
> afraid.

This was the idea here (opposite, tag jobs with 'gating-ci' to run
them on GitLab):
https://www.mail-archive.com/qemu-devel@nongnu.org/msg756464.html

> 
>> If we don't change anything, we'll keep having CI failures due
>> to Avocado design issues (artifacts removed from remote servers,
>> etc...).
> 
> I fail to see the relation between Avocado and vanishing artifacts from 3rd
> party servers... what do you plan to do instead if something gets (re-)moved
> from a server that is not under our control?

Avocado tests and CI are orthogonal, but it will be painful to
fix Avocado tests with the current Avocado CI jobs.

> 
>> I haven't seen any attempt on this list to improve the current
>> fragile situation, but better suggestions are certainly welcome.
> 
> At least I am hoping that the "check-acceptance" tests will break a little
> bit less often once Peter uses the gitlab-CI for his gating tests, too. That
> will at least prevent that one of the tests gets completely broken by a new
> merged pull request. Of course there's still the risk that tests only fail
> occasionally due to new bugs, but that can also happen for all other test
> suites (unit, qtest, iotests, ...), too.

Or Peter (as other users) will get grumpy at these tests because they
are unreliable, hard to understand what fail and debug.

Thus the removal suggestion, so we can "fix" the missing Avocado parts
before it is used heavily.

Phil.


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Cornelia Huck 3 years, 4 months ago
On Fri, 27 Nov 2020 19:46:29 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 11/27/20 7:29 PM, Thomas Huth wrote:
> > On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:  
> >> On 11/27/20 6:47 PM, Thomas Huth wrote:  
> >>> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:  
> >>>> We lately realized that the Avocado framework was not designed
> >>>> to be regularly run on CI environments. Therefore, as of 5.2
> >>>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> >>>> current users, it is possible to keep the current behavior by
> >>>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> >>>> (see [*]).
> >>>> From now on, using these jobs (or adding new tests to them)
> >>>> is strongly discouraged.
> >>>>
> >>>> Tests based on Avocado will be ported to new job schemes during
> >>>> the next releases, with better documentation and templates.  
> >>>
> >>> Why should we disable the jobs by default as long as there is no replacement
> >>> available yet?  
> >>
> >> Why keep it enabled if it is failing randomly  
> > 
> > We can still disable single jobs if they are not stable, but that's no
> > reason to disable all of them by default, is it?
> >   
> >> if images hardcoded
> >> in tests are being removed from public servers, etc...?  
> > 
> > That's independent from Avocado, you'll always have that problem if you want
> > to test with external images, unless you mirror them into a repository on
> > the same server (ie. gitlab), which, however, might not always be possible...
> >   
> >> They are not disabled, they are still runnable manually or setting
> >> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...  
> > 
> > And who do you think is going to set that variable? Hardly anybody, I guess.  
> 
> Does that mean nobody cares about these tests?

If I first have to set some random config option before tests are run,
that's an extra hurdle. I want a simple workflow where I just push to
gitlab and don't have to care about extra configuration.

> 
> > So you could also simply remove the stuff from the yml file completely instead.  
> 
> Yes, I'd prefer that too, but Alex objected.
> 
> >> We realized by default Avocado runs all tests on the CI jobs,
> >> triggering failures and complains. Developer stopped to contribute/
> >> review integration tests because of that.  
> > 
> > Did anybody really stop contributing "acceptance" test since they were
> > afraid of the gitlab-CI running them? That's new to me, do you have a pointer?  
> 
> No, but alternatively, how many tests were contributed / reviewed
> last year?

I added one, just last week... plan to do more, but there's always less
time than things I want/need to do. Maybe this just needs more
advertising?

> 
> >> We want developers be
> >> able to contribute tests to the repository fearlessly.  
> > 
> > You can always mark your test with @skipIf(os.getenv('GITLAB_CI')) if you
> > don't want to see it running in the gitlab-CI, so that's not a reason to be
> > afraid.  
> 
> This was the idea here (opposite, tag jobs with 'gating-ci' to run
> them on GitLab):
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg756464.html

I guess you want all of that:
- tag tests that you know to not work, so they're not run
- tag tests that you know to be stable, so they can be gating
- all non-tagged tests are expected to work usually, but there might be
  an occasional failure
?

> 
> >   
> >> If we don't change anything, we'll keep having CI failures due
> >> to Avocado design issues (artifacts removed from remote servers,
> >> etc...).  
> > 
> > I fail to see the relation between Avocado and vanishing artifacts from 3rd
> > party servers... what do you plan to do instead if something gets (re-)moved
> > from a server that is not under our control?  
> 
> Avocado tests and CI are orthogonal, but it will be painful to
> fix Avocado tests with the current Avocado CI jobs.

What's the problem? Cryptic error messages when artifacts cannot be
fetched?

> 
> >   
> >> I haven't seen any attempt on this list to improve the current
> >> fragile situation, but better suggestions are certainly welcome.  
> > 
> > At least I am hoping that the "check-acceptance" tests will break a little
> > bit less often once Peter uses the gitlab-CI for his gating tests, too. That
> > will at least prevent that one of the tests gets completely broken by a new
> > merged pull request. Of course there's still the risk that tests only fail
> > occasionally due to new bugs, but that can also happen for all other test
> > suites (unit, qtest, iotests, ...), too.  
> 
> Or Peter (as other users) will get grumpy at these tests because they
> are unreliable, hard to understand what fail and debug.
> 
> Thus the removal suggestion, so we can "fix" the missing Avocado parts
> before it is used heavily.

I think disabling _all_ of them is way too harsh.


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Philippe Mathieu-Daudé 3 years, 4 months ago
On 11/30/20 9:10 AM, Cornelia Huck wrote:
> On Fri, 27 Nov 2020 19:46:29 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> On 11/27/20 7:29 PM, Thomas Huth wrote:
>>> On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:  
>>>> On 11/27/20 6:47 PM, Thomas Huth wrote:  
>>>>> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:  
>>>>>> We lately realized that the Avocado framework was not designed
>>>>>> to be regularly run on CI environments. Therefore, as of 5.2
>>>>>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
>>>>>> current users, it is possible to keep the current behavior by
>>>>>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
>>>>>> (see [*]).
>>>>>> From now on, using these jobs (or adding new tests to them)
>>>>>> is strongly discouraged.
>>>>>>
>>>>>> Tests based on Avocado will be ported to new job schemes during
>>>>>> the next releases, with better documentation and templates.  
>>>>>
>>>>> Why should we disable the jobs by default as long as there is no replacement
>>>>> available yet?  
>>>>
>>>> Why keep it enabled if it is failing randomly  
>>>
>>> We can still disable single jobs if they are not stable, but that's no
>>> reason to disable all of them by default, is it?
>>>   
>>>> if images hardcoded
>>>> in tests are being removed from public servers, etc...?  
>>>
>>> That's independent from Avocado, you'll always have that problem if you want
>>> to test with external images, unless you mirror them into a repository on
>>> the same server (ie. gitlab), which, however, might not always be possible...
>>>   
>>>> They are not disabled, they are still runnable manually or setting
>>>> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...  
>>>
>>> And who do you think is going to set that variable? Hardly anybody, I guess.  
>>
>> Does that mean nobody cares about these tests?
> 
> If I first have to set some random config option before tests are run,
> that's an extra hurdle. I want a simple workflow where I just push to
> gitlab and don't have to care about extra configuration.

Good, that means you are not particularly interested by
this specific test, so you don't need to set
QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE.

> 
>>
>>> So you could also simply remove the stuff from the yml file completely instead.  
>>
>> Yes, I'd prefer that too, but Alex objected.
>>
>>>> We realized by default Avocado runs all tests on the CI jobs,
>>>> triggering failures and complains. Developer stopped to contribute/
>>>> review integration tests because of that.  
>>>
>>> Did anybody really stop contributing "acceptance" test since they were
>>> afraid of the gitlab-CI running them? That's new to me, do you have a pointer?  
>>
>> No, but alternatively, how many tests were contributed / reviewed
>> last year?
> 
> I added one, just last week... plan to do more, but there's always less
> time than things I want/need to do. Maybe this just needs more
> advertising?

I noticed your test. Do you plan it to be run regularly on CI too
(a.k.a. a gating test)?

It is small, simple, run fast, it got quickly reviewed.

Nobody cared to review the COLO test 3 months, because it is
more complex:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg727381.html

> 
>>
>>>> We want developers be
>>>> able to contribute tests to the repository fearlessly.  
>>>
>>> You can always mark your test with @skipIf(os.getenv('GITLAB_CI')) if you
>>> don't want to see it running in the gitlab-CI, so that's not a reason to be
>>> afraid.  
>>
>> This was the idea here (opposite, tag jobs with 'gating-ci' to run
>> them on GitLab):
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg756464.html
> 
> I guess you want all of that:
> - tag tests that you know to not work, so they're not run
> - tag tests that you know to be stable, so they can be gating
> - all non-tagged tests are expected to work usually, but there might be
>   an occasional failure
> ?

Yes, but IIUC Alex didn't agree to use a gating tag for stable
tests, he wants everything to be run on GitLab.

> 
>>
>>>   
>>>> If we don't change anything, we'll keep having CI failures due
>>>> to Avocado design issues (artifacts removed from remote servers,
>>>> etc...).  
>>>
>>> I fail to see the relation between Avocado and vanishing artifacts from 3rd
>>> party servers... what do you plan to do instead if something gets (re-)moved
>>> from a server that is not under our control?  
>>
>> Avocado tests and CI are orthogonal, but it will be painful to
>> fix Avocado tests with the current Avocado CI jobs.
> 
> What's the problem? Cryptic error messages when artifacts cannot be
> fetched?

More importantly to display a reproducible command line on failure.

But there are some design issue (Avocado was not design to run on
CI environment at all) that will be hard to solve if we have to keep
what we current have.

> 
>>
>>>   
>>>> I haven't seen any attempt on this list to improve the current
>>>> fragile situation, but better suggestions are certainly welcome.  
>>>
>>> At least I am hoping that the "check-acceptance" tests will break a little
>>> bit less often once Peter uses the gitlab-CI for his gating tests, too. That
>>> will at least prevent that one of the tests gets completely broken by a new
>>> merged pull request. Of course there's still the risk that tests only fail
>>> occasionally due to new bugs, but that can also happen for all other test
>>> suites (unit, qtest, iotests, ...), too.  
>>
>> Or Peter (as other users) will get grumpy at these tests because they
>> are unreliable, hard to understand what fail and debug.
>>
>> Thus the removal suggestion, so we can "fix" the missing Avocado parts
>> before it is used heavily.
> 
> I think disabling _all_ of them is way too harsh.

I'm looking for ways to:

- allow developers to contribute integration tests easily,
  not being blocked by CI.
- allow maintainers to expand their default tests set (by
  marking non-gating tests as gating for their workflow).
- allow developers to disable tests they are not interested
  in to reduce how many CI minutes they burn on each pipeline
- figure out who to contact in case a job failed (runner
  owner? test owner? breaking patch author?)

I thought marking our current jobs as optional would be simpler
that introducing an Avocado2 framework and duplicating the tests,
so I wanted to try the fast path first.

Regards,

Phil.


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Cornelia Huck 3 years, 4 months ago
On Mon, 30 Nov 2020 10:36:43 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 11/30/20 9:10 AM, Cornelia Huck wrote:
> > On Fri, 27 Nov 2020 19:46:29 +0100
> > Philippe Mathieu-Daudé <philmd@redhat.com> wrote:  
> >> On 11/27/20 7:29 PM, Thomas Huth wrote:  
> >>> On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:    
> >>>> On 11/27/20 6:47 PM, Thomas Huth wrote:    
> >>>>> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:    
> >>>>>> We lately realized that the Avocado framework was not designed
> >>>>>> to be regularly run on CI environments. Therefore, as of 5.2
> >>>>>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> >>>>>> current users, it is possible to keep the current behavior by
> >>>>>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> >>>>>> (see [*]).
> >>>>>> From now on, using these jobs (or adding new tests to them)
> >>>>>> is strongly discouraged.
> >>>>>>
> >>>>>> Tests based on Avocado will be ported to new job schemes during
> >>>>>> the next releases, with better documentation and templates.    
> >>>>>
> >>>>> Why should we disable the jobs by default as long as there is no replacement
> >>>>> available yet?    
> >>>>
> >>>> Why keep it enabled if it is failing randomly    
> >>>
> >>> We can still disable single jobs if they are not stable, but that's no
> >>> reason to disable all of them by default, is it?
> >>>     
> >>>> if images hardcoded
> >>>> in tests are being removed from public servers, etc...?    
> >>>
> >>> That's independent from Avocado, you'll always have that problem if you want
> >>> to test with external images, unless you mirror them into a repository on
> >>> the same server (ie. gitlab), which, however, might not always be possible...
> >>>     
> >>>> They are not disabled, they are still runnable manually or setting
> >>>> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...    
> >>>
> >>> And who do you think is going to set that variable? Hardly anybody, I guess.    
> >>
> >> Does that mean nobody cares about these tests?  
> > 
> > If I first have to set some random config option before tests are run,
> > that's an extra hurdle. I want a simple workflow where I just push to
> > gitlab and don't have to care about extra configuration.  
> 
> Good, that means you are not particularly interested by
> this specific test, so you don't need to set
> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE.

Confused. Why does that mean I'm not interested in that test? If I push
to gitlab, I want every reasonable test to be run, without needing to
change specific configs.

> 
> >   
> >>  
> >>> So you could also simply remove the stuff from the yml file completely instead.    
> >>
> >> Yes, I'd prefer that too, but Alex objected.
> >>  
> >>>> We realized by default Avocado runs all tests on the CI jobs,
> >>>> triggering failures and complains. Developer stopped to contribute/
> >>>> review integration tests because of that.    
> >>>
> >>> Did anybody really stop contributing "acceptance" test since they were
> >>> afraid of the gitlab-CI running them? That's new to me, do you have a pointer?    
> >>
> >> No, but alternatively, how many tests were contributed / reviewed
> >> last year?  
> > 
> > I added one, just last week... plan to do more, but there's always less
> > time than things I want/need to do. Maybe this just needs more
> > advertising?  
> 
> I noticed your test. Do you plan it to be run regularly on CI too
> (a.k.a. a gating test)?

If it proves stable, most definitely yes, as it would have caught a
recent regression.

> 
> It is small, simple, run fast, it got quickly reviewed.
> 
> Nobody cared to review the COLO test 3 months, because it is
> more complex:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg727381.html

Well, that's because of the complexity, not because of the test
framework, isn't it?

> 
> >   
> >>  
> >>>> We want developers be
> >>>> able to contribute tests to the repository fearlessly.    
> >>>
> >>> You can always mark your test with @skipIf(os.getenv('GITLAB_CI')) if you
> >>> don't want to see it running in the gitlab-CI, so that's not a reason to be
> >>> afraid.    
> >>
> >> This was the idea here (opposite, tag jobs with 'gating-ci' to run
> >> them on GitLab):
> >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg756464.html  
> > 
> > I guess you want all of that:
> > - tag tests that you know to not work, so they're not run
> > - tag tests that you know to be stable, so they can be gating
> > - all non-tagged tests are expected to work usually, but there might be
> >   an occasional failure
> > ?  
> 
> Yes, but IIUC Alex didn't agree to use a gating tag for stable
> tests, he wants everything to be run on GitLab.
> 
> >   
> >>  
> >>>     
> >>>> If we don't change anything, we'll keep having CI failures due
> >>>> to Avocado design issues (artifacts removed from remote servers,
> >>>> etc...).    
> >>>
> >>> I fail to see the relation between Avocado and vanishing artifacts from 3rd
> >>> party servers... what do you plan to do instead if something gets (re-)moved
> >>> from a server that is not under our control?    
> >>
> >> Avocado tests and CI are orthogonal, but it will be painful to
> >> fix Avocado tests with the current Avocado CI jobs.  
> > 
> > What's the problem? Cryptic error messages when artifacts cannot be
> > fetched?  
> 
> More importantly to display a reproducible command line on failure.

Yes, it's sometimes hard to coax out of Avocado what exactly went
wrong, and how to reproduce it.

> 
> But there are some design issue (Avocado was not design to run on
> CI environment at all) that will be hard to solve if we have to keep
> what we current have.

I'll let the people more familiar with Avocado speak up here. However,
if we turn off the Avocado tests now without having replacements, we'll
get worse coverage.

> 
> >   
> >>  
> >>>     
> >>>> I haven't seen any attempt on this list to improve the current
> >>>> fragile situation, but better suggestions are certainly welcome.    
> >>>
> >>> At least I am hoping that the "check-acceptance" tests will break a little
> >>> bit less often once Peter uses the gitlab-CI for his gating tests, too. That
> >>> will at least prevent that one of the tests gets completely broken by a new
> >>> merged pull request. Of course there's still the risk that tests only fail
> >>> occasionally due to new bugs, but that can also happen for all other test
> >>> suites (unit, qtest, iotests, ...), too.    
> >>
> >> Or Peter (as other users) will get grumpy at these tests because they
> >> are unreliable, hard to understand what fail and debug.
> >>
> >> Thus the removal suggestion, so we can "fix" the missing Avocado parts
> >> before it is used heavily.  
> > 
> > I think disabling _all_ of them is way too harsh.  
> 
> I'm looking for ways to:
> 
> - allow developers to contribute integration tests easily,
>   not being blocked by CI.
> - allow maintainers to expand their default tests set (by
>   marking non-gating tests as gating for their workflow).
> - allow developers to disable tests they are not interested
>   in to reduce how many CI minutes they burn on each pipeline
> - figure out who to contact in case a job failed (runner
>   owner? test owner? breaking patch author?)
> 
> I thought marking our current jobs as optional would be simpler
> that introducing an Avocado2 framework and duplicating the tests,
> so I wanted to try the fast path first.

More fine-granular control is certainly desirable; however, if we make
too many tests opt-in, it becomes more likely that nobody runs them. I
generally want (as a maintainer) to run as many tests as possible, and
not waste any brain cycles on what I need to enable where to get more
coverage.


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Thomas Huth 3 years, 4 months ago
On 27/11/2020 19.46, Philippe Mathieu-Daudé wrote:
> On 11/27/20 7:29 PM, Thomas Huth wrote:
>> On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:
>>> On 11/27/20 6:47 PM, Thomas Huth wrote:
>>>> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
>>>>> We lately realized that the Avocado framework was not designed
>>>>> to be regularly run on CI environments. Therefore, as of 5.2
>>>>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
>>>>> current users, it is possible to keep the current behavior by
>>>>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
>>>>> (see [*]).
>>>>> From now on, using these jobs (or adding new tests to them)
>>>>> is strongly discouraged.
>>>>>
>>>>> Tests based on Avocado will be ported to new job schemes during
>>>>> the next releases, with better documentation and templates.
>>>>
>>>> Why should we disable the jobs by default as long as there is no replacement
>>>> available yet?
>>>
>>> Why keep it enabled if it is failing randomly
>>
>> We can still disable single jobs if they are not stable, but that's no
>> reason to disable all of them by default, is it?
>>
>>> if images hardcoded
>>> in tests are being removed from public servers, etc...?
>>
>> That's independent from Avocado, you'll always have that problem if you want
>> to test with external images, unless you mirror them into a repository on
>> the same server (ie. gitlab), which, however, might not always be possible...
>>
>>> They are not disabled, they are still runnable manually or setting
>>> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...
>>
>> And who do you think is going to set that variable? Hardly anybody, I guess.
> 
> Does that mean nobody cares about these tests?

It's like with all the other tests: Most of the people do not really care
about them (if they are not the author of a test) unless the test fails
during "make check" / the gating CI of Peter. So IMHO the right way to go is
to finally get these in the gating CI, otherwise, if you now even disable
them in the gitlab-CI by default, they will bitrot completely.

>>> We realized by default Avocado runs all tests on the CI jobs,
>>> triggering failures and complains. Developer stopped to contribute/
>>> review integration tests because of that.
>>
>> Did anybody really stop contributing "acceptance" test since they were
>> afraid of the gitlab-CI running them? That's new to me, do you have a pointer?
> 
> No, but alternatively, how many tests were contributed / reviewed
> last year?

I don't think this is related to the fact that we've seen some failures in
the gitlab-CI.

People might rather be either not aware of the "acceptance" tests yet, or
might be uncomfortable with Python, or might just not be interested in
writing tests at all.

Same problem also exists e.g. with the new qos-test framework since it was
introduced in 2018. Only very few people contributed new tests here, though
it is quite a powerful frameworks to test various combinations of devices.

I think if you want to promote a testing framework, you have to do some
lobby work ... advertise it in blog posts, make sure that there is proper
documentation and easy examples which can be used as a base for new tests, etc.

>>> We want developers be
>>> able to contribute tests to the repository fearlessly.
>>
>> You can always mark your test with @skipIf(os.getenv('GITLAB_CI')) if you
>> don't want to see it running in the gitlab-CI, so that's not a reason to be
>> afraid.
> 
> This was the idea here (opposite, tag jobs with 'gating-ci' to run
> them on GitLab):
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg756464.html

Well, maybe you should follow-up on that series instead?

>>> If we don't change anything, we'll keep having CI failures due
>>> to Avocado design issues (artifacts removed from remote servers,
>>> etc...).
>>
>> I fail to see the relation between Avocado and vanishing artifacts from 3rd
>> party servers... what do you plan to do instead if something gets (re-)moved
>> from a server that is not under our control?
> 
> Avocado tests and CI are orthogonal, but it will be painful to
> fix Avocado tests with the current Avocado CI jobs.

Well, but we do not have any other framework in place yet which could
replace the current one, so simply disabling the tests now will only allow
more regressions to creep in, and then you'll have a hard time to get to the
state again where we were before.

>>> I haven't seen any attempt on this list to improve the current
>>> fragile situation, but better suggestions are certainly welcome.
>>
>> At least I am hoping that the "check-acceptance" tests will break a little
>> bit less often once Peter uses the gitlab-CI for his gating tests, too. That
>> will at least prevent that one of the tests gets completely broken by a new
>> merged pull request. Of course there's still the risk that tests only fail
>> occasionally due to new bugs, but that can also happen for all other test
>> suites (unit, qtest, iotests, ...), too.
> 
> Or Peter (as other users) will get grumpy at these tests because they
> are unreliable, hard to understand what fail and debug.

Actually, that's true for me for all tests that are written in Python ...
<sarcasm>Maybe we should simply ditch all python code in the QEMU
repo?</sarcasm>

> Thus the removal suggestion, so we can "fix" the missing Avocado parts
> before it is used heavily.

Now you somewhat contradict yourself. You just claimed that there are hardly
any contributions to this part of the test suite, and now you're afraid that
it might get used too heavily before it can be replaced with something else?
No, sorry, that does not make much sense to me. Thus please make sure to
provide a better framework first before disabling the stuff that we
currently have.

 Thomas


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Philippe Mathieu-Daudé 3 years, 4 months ago
On 11/30/20 10:03 AM, Thomas Huth wrote:
> On 27/11/2020 19.46, Philippe Mathieu-Daudé wrote:
>> On 11/27/20 7:29 PM, Thomas Huth wrote:
>>> On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:
>>>> On 11/27/20 6:47 PM, Thomas Huth wrote:
>>>>> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
>>>>>> We lately realized that the Avocado framework was not designed
>>>>>> to be regularly run on CI environments. Therefore, as of 5.2
>>>>>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
>>>>>> current users, it is possible to keep the current behavior by
>>>>>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
>>>>>> (see [*]).
>>>>>> From now on, using these jobs (or adding new tests to them)
>>>>>> is strongly discouraged.
>>>>>>
>>>>>> Tests based on Avocado will be ported to new job schemes during
>>>>>> the next releases, with better documentation and templates.
>>>>>
>>>>> Why should we disable the jobs by default as long as there is no replacement
>>>>> available yet?
>>>>
>>>> Why keep it enabled if it is failing randomly
>>>
>>> We can still disable single jobs if they are not stable, but that's no
>>> reason to disable all of them by default, is it?
>>>
>>>> if images hardcoded
>>>> in tests are being removed from public servers, etc...?
>>>
>>> That's independent from Avocado, you'll always have that problem if you want
>>> to test with external images, unless you mirror them into a repository on
>>> the same server (ie. gitlab), which, however, might not always be possible...
>>>
>>>> They are not disabled, they are still runnable manually or setting
>>>> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...
>>>
>>> And who do you think is going to set that variable? Hardly anybody, I guess.
>>
>> Does that mean nobody cares about these tests?
> 
> It's like with all the other tests: Most of the people do not really care
> about them (if they are not the author of a test) unless the test fails
> during "make check" / the gating CI of Peter. So IMHO the right way to go is
> to finally get these in the gating CI, otherwise, if you now even disable
> them in the gitlab-CI by default, they will bitrot completely.

Maybe it is up to each maintainer? Some tests are hard to reproduce,
I'd prefer have them committed in the repository to allow someone to
run it later and eventually fix it if it bitrot, rather than having
pieces of code we don't know if they are tested, who use them, and
we are scared to remove.

>>>> We realized by default Avocado runs all tests on the CI jobs,
>>>> triggering failures and complains. Developer stopped to contribute/
>>>> review integration tests because of that.
>>>
>>> Did anybody really stop contributing "acceptance" test since they were
>>> afraid of the gitlab-CI running them? That's new to me, do you have a pointer?
>>
>> No, but alternatively, how many tests were contributed / reviewed
>> last year?
> 
> I don't think this is related to the fact that we've seen some failures in
> the gitlab-CI.
> 
> People might rather be either not aware of the "acceptance" tests yet, or
> might be uncomfortable with Python, or might just not be interested in
> writing tests at all.
> 
> Same problem also exists e.g. with the new qos-test framework since it was
> introduced in 2018. Only very few people contributed new tests here, though
> it is quite a powerful frameworks to test various combinations of devices.
> 
> I think if you want to promote a testing framework, you have to do some
> lobby work ... advertise it in blog posts, make sure that there is proper
> documentation and easy examples which can be used as a base for new tests, etc.

I certainly don't want to promote any particular framework. I don't
care what is used. What I want is to add more tests. And I would like
developers to focus on what feature they want to test, and not on
fixing refcount leak in the test itself.

If qos-test is sufficient to replace what we have in Python, I'm happy
if someone port the Python tests to it.

> 
>>>> We want developers be
>>>> able to contribute tests to the repository fearlessly.
>>>
>>> You can always mark your test with @skipIf(os.getenv('GITLAB_CI')) if you
>>> don't want to see it running in the gitlab-CI, so that's not a reason to be
>>> afraid.
>>
>> This was the idea here (opposite, tag jobs with 'gating-ci' to run
>> them on GitLab):
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg756464.html
> 
> Well, maybe you should follow-up on that series instead?

OK.

> 
>>>> If we don't change anything, we'll keep having CI failures due
>>>> to Avocado design issues (artifacts removed from remote servers,
>>>> etc...).
>>>
>>> I fail to see the relation between Avocado and vanishing artifacts from 3rd
>>> party servers... what do you plan to do instead if something gets (re-)moved
>>> from a server that is not under our control?
>>
>> Avocado tests and CI are orthogonal, but it will be painful to
>> fix Avocado tests with the current Avocado CI jobs.
> 
> Well, but we do not have any other framework in place yet which could
> replace the current one, so simply disabling the tests now will only allow
> more regressions to creep in, and then you'll have a hard time to get to the
> state again where we were before.
> 
>>>> I haven't seen any attempt on this list to improve the current
>>>> fragile situation, but better suggestions are certainly welcome.
>>>
>>> At least I am hoping that the "check-acceptance" tests will break a little
>>> bit less often once Peter uses the gitlab-CI for his gating tests, too. That
>>> will at least prevent that one of the tests gets completely broken by a new
>>> merged pull request. Of course there's still the risk that tests only fail
>>> occasionally due to new bugs, but that can also happen for all other test
>>> suites (unit, qtest, iotests, ...), too.
>>
>> Or Peter (as other users) will get grumpy at these tests because they
>> are unreliable, hard to understand what fail and debug.
> 
> Actually, that's true for me for all tests that are written in Python ...
> <sarcasm>Maybe we should simply ditch all python code in the QEMU
> repo?</sarcasm>

Also true with Perl and Rust. The community is wide, hard
to include everybody and keep people happy.
>> Thus the removal suggestion, so we can "fix" the missing Avocado parts
>> before it is used heavily.
> 
> Now you somewhat contradict yourself. You just claimed that there are hardly
> any contributions to this part of the test suite, and now you're afraid that
> it might get used too heavily before it can be replaced with something else?
> No, sorry, that does not make much sense to me. Thus please make sure to
> provide a better framework first before disabling the stuff that we
> currently have.

Fine, I'll follow the other alternatives:

- provide a better framework,
- do nothing.

Thanks,

Phil.


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Daniel P. Berrangé 3 years, 4 months ago
On Mon, Nov 30, 2020 at 10:03:35AM +0100, Thomas Huth wrote:
> On 27/11/2020 19.46, Philippe Mathieu-Daudé wrote:
> > On 11/27/20 7:29 PM, Thomas Huth wrote:
> >> On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:
> >>> On 11/27/20 6:47 PM, Thomas Huth wrote:
> >>>> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
> >>>>> We lately realized that the Avocado framework was not designed
> >>>>> to be regularly run on CI environments. Therefore, as of 5.2
> >>>>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> >>>>> current users, it is possible to keep the current behavior by
> >>>>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> >>>>> (see [*]).
> >>>>> From now on, using these jobs (or adding new tests to them)
> >>>>> is strongly discouraged.
> >>>>>
> >>>>> Tests based on Avocado will be ported to new job schemes during
> >>>>> the next releases, with better documentation and templates.
> >>>>
> >>>> Why should we disable the jobs by default as long as there is no replacement
> >>>> available yet?
> >>>
> >>> Why keep it enabled if it is failing randomly
> >>
> >> We can still disable single jobs if they are not stable, but that's no
> >> reason to disable all of them by default, is it?
> >>
> >>> if images hardcoded
> >>> in tests are being removed from public servers, etc...?
> >>
> >> That's independent from Avocado, you'll always have that problem if you want
> >> to test with external images, unless you mirror them into a repository on
> >> the same server (ie. gitlab), which, however, might not always be possible...
> >>
> >>> They are not disabled, they are still runnable manually or setting
> >>> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...
> >>
> >> And who do you think is going to set that variable? Hardly anybody, I guess.
> > 
> > Does that mean nobody cares about these tests?
> 
> It's like with all the other tests: Most of the people do not really care
> about them (if they are not the author of a test) unless the test fails
> during "make check" / the gating CI of Peter. So IMHO the right way to go is
> to finally get these in the gating CI, otherwise, if you now even disable
> them in the gitlab-CI by default, they will bitrot completely.

That people don't care, and ignore it until Peter hits the failure during
merge is a tragedy of the commons in itself.

I think we need to set expectations that caring about tests is a key part
of every contributor's responsibility, with subsystem maintainers leading
by example:

   https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg04897.html

We do need tests to be reliable though when we're treating them as gating.

Hiding unreliable tests behind an env variable you have to opt-in to
setting is not going to help that. IMHO unreliable tests should be
just disabled entirely. If someone genuinely does care about the test
then they can fix it and re-enable it at the same time. 

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: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Daniel P. Berrangé 3 years, 4 months ago
On Fri, Nov 27, 2020 at 07:29:31PM +0100, Thomas Huth wrote:
> On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:
> > On 11/27/20 6:47 PM, Thomas Huth wrote:
> >> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
> >>> We lately realized that the Avocado framework was not designed
> >>> to be regularly run on CI environments. Therefore, as of 5.2
> >>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> >>> current users, it is possible to keep the current behavior by
> >>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> >>> (see [*]).
> >>> From now on, using these jobs (or adding new tests to them)
> >>> is strongly discouraged.
> >>>
> >>> Tests based on Avocado will be ported to new job schemes during
> >>> the next releases, with better documentation and templates.
> >>
> >> Why should we disable the jobs by default as long as there is no replacement
> >> available yet?
> > 
> > Why keep it enabled if it is failing randomly
> 
> We can still disable single jobs if they are not stable, but that's no
> reason to disable all of them by default, is it?

Agreed, the jobs which are known to be broken or unreliable should
be unconditonally disabled in QEMU as a whole. This isn't specific
to gitlab config - the qemu build makefiles/mesonfiles should disable
the problem tests entirely, as we don't want developers wasting time
running them locally either if they're known to be broken/unreliable.

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: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Ademar Reis 3 years, 3 months ago
On Mon, Nov 30, 2020 at 10:31:09AM +0000, Daniel P. Berrangé wrote:
> On Fri, Nov 27, 2020 at 07:29:31PM +0100, Thomas Huth wrote:
> > On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:
> > > On 11/27/20 6:47 PM, Thomas Huth wrote:
> > >> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
> > >>> We lately realized that the Avocado framework was not designed
> > >>> to be regularly run on CI environments. Therefore, as of 5.2
> > >>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> > >>> current users, it is possible to keep the current behavior by
> > >>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> > >>> (see [*]).
> > >>> From now on, using these jobs (or adding new tests to them)
> > >>> is strongly discouraged.
> > >>>
> > >>> Tests based on Avocado will be ported to new job schemes during
> > >>> the next releases, with better documentation and templates.
> > >>
> > >> Why should we disable the jobs by default as long as there is no replacement
> > >> available yet?
> > > 
> > > Why keep it enabled if it is failing randomly
> > 
> > We can still disable single jobs if they are not stable, but that's no
> > reason to disable all of them by default, is it?
> 
> Agreed, the jobs which are known to be broken or unreliable should
> be unconditonally disabled in QEMU as a whole. This isn't specific
> to gitlab config - the qemu build makefiles/mesonfiles should disable
> the problem tests entirely, as we don't want developers wasting time
> running them locally either if they're known to be broken/unreliable.
> 

The problem is identifying when a test is broken/unreliable. No
complex test is 100% reliable: change the environment,
(configuration, build options, platform, etc) and any test complex
enough will start to fail. I've worked in projects orders of
magnitude simpler than QEMU and that was a golden rule. Testing QEMU
is *HARD*.

Which is why I defend test-automation separated from CI:

 * Have a stable CI with tests cherry-picked by whoever is
   maintaining a particular CI runner (we shouldn't have orphan
   runners).

 * Have as many tests as possible in the git repo: maintained,
   reviewed and run (outside of a CI) by people who care about them.

I absolutely agree with you that maintainers and developers should
care and our goal should be a gating CI. The question is how to
create a strategy and a plan to get there. Forcing people to care
rarely works.

Thanks.
   - Ademar

-- 
Ademar Reis Jr
Red Hat

^[:wq!


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Wainer dos Santos Moschetta 3 years, 3 months ago
On 11/27/20 3:29 PM, Thomas Huth wrote:
> On 27/11/2020 18.57, Philippe Mathieu-Daudé wrote:
>> On 11/27/20 6:47 PM, Thomas Huth wrote:
>>> On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
>>>> We lately realized that the Avocado framework was not designed
>>>> to be regularly run on CI environments. Therefore, as of 5.2
>>>> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
>>>> current users, it is possible to keep the current behavior by
>>>> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
>>>> (see [*]).
>>>>  From now on, using these jobs (or adding new tests to them)
>>>> is strongly discouraged.
>>>>
>>>> Tests based on Avocado will be ported to new job schemes during
>>>> the next releases, with better documentation and templates.
>>> Why should we disable the jobs by default as long as there is no replacement
>>> available yet?
>> Why keep it enabled if it is failing randomly
> We can still disable single jobs if they are not stable, but that's no
> reason to disable all of them by default, is it?
>
>> if images hardcoded
>> in tests are being removed from public servers, etc...?
> That's independent from Avocado, you'll always have that problem if you want
> to test with external images, unless you mirror them into a repository on
> the same server (ie. gitlab), which, however, might not always be possible...

Phil,

on commit 89e076f37d0020bfadb you changed fetch_asset to cancel the test 
if it cannot download a file. Is there anything else that could be done 
to mitigate that problem until we don't have a repository mirror for the 
files?

- Wainer


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Niek Linnenbank 3 years, 4 months ago
Hi Philippe, Thomas,

Op vr 27 nov. 2020 18:57 schreef Philippe Mathieu-Daudé <philmd@redhat.com>:

> On 11/27/20 6:47 PM, Thomas Huth wrote:
> > On 27/11/2020 18.41, Philippe Mathieu-Daudé wrote:
> >> We lately realized that the Avocado framework was not designed
> >> to be regularly run on CI environments. Therefore, as of 5.2
> >> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> >> current users, it is possible to keep the current behavior by
> >> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> >> (see [*]).
> >> From now on, using these jobs (or adding new tests to them)
> >> is strongly discouraged.
> >>
> >> Tests based on Avocado will be ported to new job schemes during
> >> the next releases, with better documentation and templates.
> >
> > Why should we disable the jobs by default as long as there is no
> replacement
> > available yet?
>
> Why keep it enabled if it is failing randomly, if images hardcoded
> in tests are being removed from public servers, etc...?
>
>
> They are not disabled, they are still runnable manually or setting
> QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE...
>
> We realized by default Avocado runs all tests on the CI jobs,
> triggering failures and complains. Developer stopped to contribute/
> review integration tests because of that. We want developers be
> able to contribute tests to the repository fearlessly.
>
> If we don't change anything, we'll keep having CI failures due
> to Avocado design issues (artifacts removed from remote servers,
> etc...).
>

I share your concern about the artifacts not being stored on a reliable
location that can be used for all Qemu acceptance tests. In particular for
the Orange Pi PC tests we have seen it happening, that the image we use was
removed from the armbian server.

As a temporary solution perhaps we can use github to store artifacts, until
we have a proper location?


> I haven't seen any attempt on this list to improve the current
> fragile situation, but better suggestions are certainly welcome.
>
> Thanks,
>
> Phil.
>
>
>
Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Wainer dos Santos Moschetta 3 years, 4 months ago
Hi Phil,

On 11/27/20 2:41 PM, Philippe Mathieu-Daudé wrote:
> We lately realized that the Avocado framework was not designed
> to be regularly run on CI environments. Therefore, as of 5.2
> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> current users, it is possible to keep the current behavior by
> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> (see [*]).


Could you please detail where have you seen the avocado based tests 
failing that much to justify their removal of CI?

See below some data that I generated from the pipelines for 
https://gitlab.com/qemu-project/qemu/. It seems that jobs which failed 
due timeout are marked as 'skip' and that explain why some pipelines 
don't have a list of failed jobs.

---

Failed 21 out of 99
Pipeline -- 221617704 (staging)
check-patch
Pipeline -- 219276598 (master)
pages
Pipeline -- 218121424 (master)
Pipeline -- 217995873 (staging)
acceptance-system-fedora
Pipeline -- 217995872 (master)
build-tci
Pipeline -- 217551771 (v5.2.0-rc2)
Pipeline -- 217503505 (master)
Pipeline -- 217362429 (master)
Pipeline -- 217328707 (master)
check-crypto-old-nettle
Pipeline -- 216770735 (staging)
acceptance-system-fedora
check-patch
Pipeline -- 215772908 (master)
Pipeline -- 215715025 (staging)
check-crypto-old-gcrypt
Pipeline -- 215715024 (master)
check-crypto-old-nettle
Pipeline -- 214944520 (master)
acceptance-system-debian
Pipeline -- 214460230 (v5.2.0-rc1)
Pipeline -- 214438601 (master)
check-crypto-only-gnutls
Pipeline -- 214273938 (master)
Pipeline -- 214183970 (master)
Pipeline -- 214140305 (master)
Pipeline -- 213892224 (master)
Pipeline -- 213871132 (master)
>  From now on, using these jobs (or adding new tests to them)
> is strongly discouraged.
>
> Tests based on Avocado will be ported to new job schemes during
> the next releases, with better documentation and templates.
>
> [*] https://docs.gitlab.com/ee/ci/variables/README.html#create-a-custom-variable-in-the-ui
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   .gitlab-ci.yml | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index d0173e82b16..2674407cd13 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -66,6 +66,15 @@ include:
>       - cd build
>       - python3 -c 'import json; r = json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for t in r["tests"] if t["status"] not in ("PASS", "SKIP", "CANCEL")]' | xargs cat
>       - du -chs ${CI_PROJECT_DIR}/avocado-cache
> +  rules:
> +  # As of QEMU 5.2, Avocado is not yet ready to run in CI environments, therefore
> +  # the jobs based on this template are not run automatically (except if the user
> +  # explicitly sets the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE environment
> +  # variable). Adding new jobs on top of this template is strongly discouraged.
> +  - if: $QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE == null
> +    when: manual
> +    allow_failure: true
> +  - when: always
>   
>   build-system-ubuntu:
>     <<: *native_build_job_definition


The script I used to generate the above metrics:

$ pip install --user python-gitlab

$ cat gitlab-stats
#!/bin/env python3

import gitlab

gl = gitlab.Gitlab('http://gitlab.com')
qemu_project = gl.projects.get(11167699)
pipelines = qemu_project.pipelines.list()

total=0
failed=[]
for pipeline in qemu_project.pipelines.list(page=1, per_page=300):
     if pipeline.status == 'running':
         continue
     elif pipeline.status == 'failed':
         failed.append(pipeline)
     total += 1

print("Failed %d out of %d" % (len(failed), total))
for pipeline in failed:
     print('Pipeline -- %d (%s)' % (pipeline.id, pipeline.ref))
     jobs_failed=[]
     for job in pipeline.jobs.list():
         if job.status == 'failed':
             print(job.name)


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Willian Rampazzo 3 years, 3 months ago
On Fri, Nov 27, 2020 at 2:41 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> We lately realized that the Avocado framework was not designed
> to be regularly run on CI environments. Therefore, as of 5.2
> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> current users, it is possible to keep the current behavior by
> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> (see [*]).
> From now on, using these jobs (or adding new tests to them)
> is strongly discouraged.

When you say, "Avocado framework was not designed to be regularly run
on CI environments", I feel your pain. Avocado is a really nice test
framework, and I agree with you that running it locally is a little
easier than running inside a CI environment. Debugging a job failure
in the CI is not user-friendly; finding the command to reproduce a job
failure locally is not user-friendly. I understand why you would like
to remove the CI's acceptance tests, but I think your proposal is
missing some arguments and some planning.

If I read correctly, we share the same view that the CI and the
software tests are two different things. Here you are proposing that
we temporarily remove the CI's acceptance tests because it is not
user-friendly to the devs. This does not mean the tests will be lost.
It will be possible to run them locally or in the CI using the
QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable.

>
> Tests based on Avocado will be ported to new job schemes during
> the next releases, with better documentation and templates.

I understand you intend to make a more reliable and stable CI. Some
wording on why the new job scheme will be better than what we have now
and some planning on enabling the acceptance tests again in the CI may
help evaluate if it is feasible or just the same as what we have
today.

It would be nice to hear from other subsystem maintainers their pain
points about using the CI and how we can improve it. I hear you that
Avocado needs to improve its interface to be more user friendly. As an
Avocado developer, I would also like to hear from others where we can
improve Avocado to make it less painful for the QEMU developers'.


Re: [RFC PATCH-for-5.2] gitlab-ci: Do not automatically run Avocado integration tests anymore
Posted by Cleber Rosa 3 years, 3 months ago
On Fri, Nov 27, 2020 at 06:41:10PM +0100, Philippe Mathieu-Daudé wrote:
> We lately realized that the Avocado framework was not designed
> to be regularly run on CI environments. Therefore, as of 5.2

Hi Phil,

First of all, let me say that I understand your overall goal, and
although I don't agree with the strategy, I believe we're in agreement
wrt the destination.

The main issue that you seem to address here is the fact that some CI
tests may fail more often than others, which will lead to jobs that
will fail more than others, which will ultimately taint the overall CI
status.  Does that sound like an appropriate overall grasp of your
motivation?

Assuming I got it right, let me say that having an "always green CI"
is a noble goal, but it can also be extremely frustrating if one
doesn't apply some safeguarding measures.  More on that later.

As you certainly know, I'm also interested in understanding the "not
designed to be regularly run on CI environments" part.  The best
correlation I could make was to link that to the these two points you
raised elsewhere:

 1) Failing randomly
 2) Images hardcoded in tests are being removed from public servers

With regards to point #1, this is probably unavoidable as a whole.
I've had some experience running dedicated test jobs for close to a
decade, and maybe the only way to get close to avoid random failures
on integration tests, is to run close to nothing in those jobs.
Running "/bin/true" has a very low chance of failing randomly.

In my own experience, the only way to address point #1, is to
babysit jobs.  That means:

 a) assume they will produce some messy stuff at no particular time
 b) act as quickly and effectively as possible
 c) be compassionate, that is, waive the unavoidable mess incidents

Building on the previous analogy, if you decide to not have a baby,
but a plant, you'll probably need to to a lot less of those.
If you get a pet, than some more.  Now a human baby will probably
(I guess) require a whole lot more of those.  And as those age
and reach maturity, they'll (hopefully) require less babysitting,
but they can still mess up at any given time.

Analogies and jokes aside, the urgent *action item* here has been
discussed both publicly, and internally at Red Hat. It consists of
having an "always on" maintainer for those jobs.  In the specific case
of the "Acceptance" jobs, Willian Rampazzo has volunteered to,
initially, be this person.  He'll manage all related information
on job's issues.

We're still discussing the tools to use to give the visibility that
the QEMU projects needs.  I personally would be happy enough to start
with a publicly accessible spreadsheet that builds upon the
information produced by GitLab.  A proper application is also being
considered.  A sample of the requirements include:

   I) waive failures (say a job and tests failed because of a
      network outage)
  II) build trends (show how stable all executions of test "foo"
      were during the last: week, month, eternity, etc).
 III) keep a list of the known issues and relate them to waivers
      and currently skipped tests

Getting back to point #2, I have two main points about it.  First is
that I've had a lot of experience with tests having copies of images,
both on local filesystems and in on close by NFS servers.  Local
filesystems would fail at provisioning/sync time. And NFS based ones
would fail every now and then for various network or NFS server
issues.  It goes back to my point about not being able to escape the
babysitting ever.

Second is that this is somehow related to features and improvements
that could/should be added to whathever supporting code (aka
framework) we use.  Right now, we have some specific features
scheduled to be introduced in Avocado 84.0 (due in ~2 weeks).  They
can be seen with the "customer:QEMU" label:

  https://github.com/avocado-framework/avocado/issues?q=is%3Aissue+is%3Aopen+label%3Acustomer%3AQEMU

A number of other features have already landed on previous versions,
but I was unable to send patches in time for 5.2, so my expectation is
to bundle more of them and bump Avocado to 84.0 at once (instead of
82.0 or 83.0).

> we deprecate the gitlab-ci jobs using Avocado. To not disrupt
> current users, it is possible to keep the current behavior by
> setting the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE variable
> (see [*]).
> From now on, using these jobs (or adding new tests to them)
> is strongly discouraged.
>

These jobs run `make check-acceptance`, which will pickup new tests.
So how do you suggest to *not adding new tests* to those jobs?  Are
you suggesting that no new acceptance test be added?

> Tests based on Avocado will be ported to new job schemes during
> the next releases, with better documentation and templates.
>

This is a very good approach to move forward.  For what is worth,
Avocado has invested in an API specifically for that, the Job API.
The goal is to have smarter jobs for different purposes that behave
appropriattely and account for the environment (such as host platform,
CI, etc).  Example of my upcoming "job-kvm-only.py":

------------------------------------------------------------------------------

#!/usr/bin/env python

import os
import sys

from qemu.accel import kvm_available
from avocado.core.job import Job


def main():
    if not kvm_available():
        sys.exit(0)

    config = {'run.references': ['tests/acceptance/'],
              'filter.by_tags.tags': ['accel:kvm,arch:%s' % os.uname()[4]]}
    with Job.from_config(config) as job:
        return job.run()


if __name__ == '__main__':
    sys.exit(main())

------------------------------------------------------------------------------

Other examples of Jobs using this API can be seen here:

   https://github.com/avocado-framework/avocado/tree/master/examples/jobs

And the documentation on the features one can use by setting
configuration keys can be found here:

   https://avocado-framework.readthedocs.io/en/83.0/config/index.html

So for example, if one wants to ignore errors while fetching assets in a job,
there is this:

   https://avocado-framework.readthedocs.io/en/83.0/config/index.html#assets-fetch-ignore-errors

> [*] https://docs.gitlab.com/ee/ci/variables/README.html#create-a-custom-variable-in-the-ui
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  .gitlab-ci.yml | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index d0173e82b16..2674407cd13 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -66,6 +66,15 @@ include:
>      - cd build
>      - python3 -c 'import json; r = json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for t in r["tests"] if t["status"] not in ("PASS", "SKIP", "CANCEL")]' | xargs cat
>      - du -chs ${CI_PROJECT_DIR}/avocado-cache
> +  rules:
> +  # As of QEMU 5.2, Avocado is not yet ready to run in CI environments, therefore
> +  # the jobs based on this template are not run automatically (except if the user
> +  # explicitly sets the QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE environment
> +  # variable). Adding new jobs on top of this template is strongly discouraged.
> +  - if: $QEMU_CI_INTEGRATION_JOBS_PRE_5_2_RELEASE == null
> +    when: manual
> +    allow_failure: true
> +  - when: always
>

I believe the best way to move forward is a bit different than what
you propose here.  I'd go with the babysitting approach, and
agressively disable tests on first sign of failure, instead of
"muting" all of them at once.  My perception is that without the
babysitting and quick actions, new jobs will end up in the same
situation given enough tests are added.

Anyway, thanks for bringing up the discussion here.

- Cleber.

>  build-system-ubuntu:
>    <<: *native_build_job_definition
> -- 
> 2.26.2
>