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

[Cornelia Huck]

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.