Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests

[Eduardo Habkost]

On Fri, May 24, 2019 at 03:45:56PM +0200, Aleksandar Markovic wrote:
> On May 23, 2019 11:31 PM, "Eduardo Habkost" <address@hidden> wrote:
> >
> > On Thu, May 23, 2019 at 09:28:00AM -0400, Cleber Rosa wrote:
> > >
> > >
> > > ----- Original Message -----
> > > > From: "Philippe Mathieu-Daudé" <address@hidden>
> > > > To: "Eduardo Habkost" <address@hidden>, "Cleber Rosa" <
> address@hidden>
> > > > Cc: "Aleksandar Rikalo" <address@hidden>, "Philippe
> Mathieu-Daudé" <address@hidden>, "Wainer dos Santos
> > > > Moschetta" <address@hidden>, address@hidden, "Aleksandar
> Markovic" <address@hidden>,
> > > > "Aleksandar Markovic" <address@hidden>, "Aurelien Jarno" <
> address@hidden>
> > > > Sent: Thursday, May 23, 2019 5:38:34 AM
> > > > Subject: Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests
> > > >
> > > > On 5/23/19 1:07 AM, Eduardo Habkost wrote:
> > > > > On Wed, May 22, 2019 at 05:46:06PM -0400, Cleber Rosa wrote:
> > > > >> ----- Original Message -----
> > > > >>> From: "Eduardo Habkost" <address@hidden>
> > > > >>> On Tue, May 21, 2019 at 01:19:06AM +0200, Philippe Mathieu-Daudé
> wrote:
> > > > >>>> Hi,
> > > > >>>>
> > > > >>>> It was a rainy week-end here, so I invested it to automatize some
> > > > >>>> of my MIPS tests.
> > > > >>>>
> > > > >>>> The BootLinuxSshTest is not Global warming friendly, it is not
> > > > >>>> meant to run on a CI system but rather on a workstation previous
> > > > >>>> to post a pull request.
> > > > >>>> It can surely be improved, but it is a good starting point.
> > > > >>>
> > > > >>> Until we actually have a mechanism to exclude the test case on
> > > > >>> travis-ci, I will remove patch 4/4 from the queue.  Aleksandar,
> > > > >>> please don't merge patch 4/4 yet or it will break travis-ci.
> > > > >>>
> > > > >>> Cleber, Wainer, is it already possible to make "avocado run" skip
> > > > >>> tests tagged with "slow"?
> > > > >>>
> > > > >>
> > > > >> The mechanism exists, but we haven't tagged any test so far as
> slow.
> > > > >>
> > > > >> Should we define/document a criteria for a test to be slow?  Given
> > > > >> that this is highly subjective, we have to think of:
> > > > >>
> > > > >>  * Will we consider the average or maximum run time (the timeout
> > > > >>    definition)?
> > > > >>
> > > > >>  * For a single test, what is "slow"? Some rough numbers from
> Travis
> > > > >>    CI[1] to help us with guidelines:
> > > > >>    - boot_linux_console.py:BootLinuxConsole.test_x86_64_pc:  PASS
> (6.04 s)
> > > > >>    - boot_linux_console.py:BootLinuxConsole.test_arm_virt:  PASS
> (2.91 s)
> > > > >>    -
> > > > >>
> linux_initrd.py:LinuxInitrd.test_with_2gib_file_should_work_with_linux_v4_16:
> > > > >>    PASS (18.14 s)
> > > > >>    - boot_linux.py:BootLinuxAarch64.test_virt:  PASS (396.88 s)
> > > > >
> > > > > I don't think we need to overthink this.  Whatever objective
> > > > > criteria we choose, I'm sure we'll have to adapt them later due
> > > > > to real world problems.
> > > > >
> > > > > e.g.: is 396 seconds too slow?  I don't know, it depends: does it
> > > > > break Travis and other CI systems often because of timeouts?  If
> > > > > yes, then we should probably tag it as slow.
> > > > >
> > > > > If having subjective criteria is really a problem (I don't think
> > > > > it is), then we can call the tag "skip_travis", and stop worrying
> > > > > about defining what exactly is "slow".
> > > >
> > > > I'd go with a simpler "tags:travis-ci" whitelisting any job expecting
> to
> > > > run smoothly there.
> > > >
> > >
> > > My concern is what becomes of "make check-acceptance".  Should we
> introduce
> > > another target, say, "make check-acceptance-ci" or just change its
> meaning
> > > and reuse it?
> >
> > What about "make check-acceptance TAG=travis-ci"?
> >
> > >
> > > > Then we can add "slow" tests without having to worry about
> blacklisting
> > > > for Travis CI.
> > > > Also, Other CI can set different timeouts.
> > > >
> > > > I'd like maintainers to add as many tests as they want to upstream, so
> > > > these tests can eventually run by anyone, then each maintainer is free
> > > > to select which particular set he wants to run as default.
> > > >
> > >
> > > OK, so this matches the idea of carefully curating a set of tests for
> > > CI.  WRT white or blacklisting, I favor the approach that requires the
> > > least effort from the developer to have its test enabled, so I'd go
> > > with blacklisting.  I fear that simple tests will just sit on the repo
> > > without being properly exercised if we need to whitelist them.
> > >
> >
> > I agree.  I'd prefer the default case to be simple and not
> > require extra tags.  (i.e. tests without any tags would be run in
> > Travis by default).
> >
> 
> Eduardo,
> 
> You are confusing me here.
> 
> You first suggest:
> 
> > What about "make check-acceptance TAG=travis-ci"?
> 

I was just trying to suggest using make variables as input to
check-acceptance, instead of creating separate makefile rules for
each set of test cases.  But you are right:

> ... and then say:
> 
> > ...tests without any tags would be run in Travis by default.
> 
> For casual observers like me it is contradictory, I must be missing
> something here, no?

Yes, if we use tags to exclude tests, the command line would look
different.  Maybe something like:

  make check-acceptance EXCLUDE_TAGS=skip-travis

The exact format of the arguments don't matter to me, as long as
we don't require people to write new makefile rules just because
they want to run a different set of test cases.

-- 
Eduardo