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

[Eduardo Habkost]

On Fri, May 24, 2019 at 10:32:36PM +0200, Aleksandar Markovic wrote:
> On May 24, 2019 9:40 PM, "Eduardo Habkost" <address@hidden> wrote:
> >
> > On Thu, May 23, 2019 at 07:27:35PM +0200, Philippe Mathieu-Daudé wrote:
> > > On 5/23/19 7:11 PM, Aleksandar Markovic wrote:
> > > > On May 23, 2019 3:45 PM, "Cleber Rosa" <address@hidden> wrote:
> > > >>> From: "Aleksandar Markovic" <address@hidden>
> > > >>> On May 22, 2019 11:46 PM, "Cleber Rosa" <address@hidden> wrote:
> > > >>>>> From: "Eduardo Habkost" <address@hidden>
> > > >>>>>
> > > >>>>> 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.
> > > >>>>
> > > >>>
> > > >>> Cleber,
> > > >>>
> > > >>> For the test from patch 4/4, there is no dilemma - it should be in
> the
> > > >>> “slow” group, as Philippe envisioned and said, so that it is not
> > > > humpered
> > > >>> with stricter requirements for “fast” (default) group. Could you
> > > > explain us
> > > >>> how to do it, so that we can hopefully finally proceed?
> > > >>>
> > > >>
> > > >> Hi Aleksandar,
> > > >>
> > > >> The point is that there's no "group" definition at this point.  This
> is
> > > > the
> > > >> core of the discussion.
> > > >>
> > > >> I think we're close to converging to something simple and effective.
> > > > Please
> > > >> let us know what you think of the proposals given.
> > > >>
> > > >> Thanks!
> > > >> - Cleber.
> > > >>
> > > >
> > > > Cleber, hi.
> > > >
> > > > Thanks for responding.
> > > >
> > > > My views are very similar to Philippe's, but I will provide you with
> more
> > > > details of our (mips) perspective.
> > > >
> > > > As far as black/whitelist issues that is a moot point for us - we
> only want
> > > > to be able to have a way to tag a test within the test itself (so,
> without
> > > > updating some common files, external lists,etc.)
> > > >
> > > > In general, we would like to have a test environment where we would
> be able
> > > > to test what WE deem suitable for us, without feeling that we bother
> you or
> > > > anybody else, or that we are bothered by others.
> > > >
> > > > Let me give you a little extreme example: Let's say we want a complex
> test
> > > > that downloads components from let's say fifty internet location,
> executes
> > > > zillion test cases, and last two days. I wouldn't like anybody to ask
> me
> > > > “Why would you that?” or tell me “You can't do this.” or say “No, we
> did
> > > > not anticipate such tests, patch rejected.” I (we, people from mips)
> should
> > > > be able to define what I (we) need.
> > >
> > > Maybe we can use subdirectory like we do for the TCG tests (Aleksandar
> > > maintains tests/tcg/mips/). We should try to keep contribution upstream,
> > > so good idea/pattern can be reused by others.
> > >
> > > What I'd like to have with those tests is, at least:
> > >
> > > 1/ we don't need to run all the tests (but there is a set of 'quick'
> > > tests we can run on daily basis)
> > >
> > > 2/ maintainers can run their default tests easily (using a combination
> > > of Avocado tags)
> > >
> > > 3/ if a developer working on the PCI subsystem has to modify the MIPS
> > > subsystem (for example), he should be able to run the MIPS tests before
> > > sending his series.
> >
> > Keeping the test cases organized in subdirectories are a good
> > idea, but don't think this is going to help us when we need to
> > quickly enable/disable specific test cases on some CI systems.
> >
> 
> Well, Eduardo, nobody said that directory locations should be used for
> enabling/disabling or tagging/untagging tests in the first place. I think
> it was clear for everybody from the outset that these features should have
> their own mechanisms, which Cleber says already exist, but can't be used
> because the test group still can't figure out (in some hamletesque way)
> whether to blacklist or to whitelist, or how to name the tag for travis,
> and tag for not travis, and if such tags should even exist, etc. - that is
> my layman impression from recent discussions. And now when Philippe
> suggested (in my opinion logical and reasonable) subdirectory, an endless
> discussion begins: “To subdirectory or not to subdirectory? That is the
> question.” Meanwhile, 4.1 is inexorably getting closer and closer, and with
> each day, the value of any potential tests is decreasing.

I understand that seeing the discussions going on and the patches
taking too long to be included might be frustrating.

These discussions shouldn't get into the way of addressing other
problems.  We don't need to wait until all discussions have
finished before proposing new patches or before merging patches
that are considered good.


> 
> Directory structure should be used in its usual and basic way: for
> clustering files of similar nature, purpose, or origin, and I do certainly
> support any reasonable subdirectory organization for your directory - and
> you should think about it, and probably while doing that consult a little
> bit other people from all walks of QEMU. We are ready to comply with your
> final decision.

About subdirectories, specifically, note that I explicitly said
it was a good idea.  If somebody wants to send patches, they are
welcome.

If I'm doing something else that could be blocking people from
getting work done, I'd like to fix that.  I'm aware that
sometimes I take too long to review patches, but I hope other
developers can help us on the review work.

> 
> The good thing is that nothing is set in stone, everything can be changed
> and improved, moving files is easy in git.
> 
> All that said, many thanks for reviewing patch 4/4.
> 
> Aleksandar
> 
> 
> 
> > Disabling a test case (or an entire category of test cases) known
> > to be failing on some CI systems should require a one line patch,
> > not moving files to a separate directory.
> >
> > >
> > > > Having such test would be a big deal for me, not only that I could
> run it
> > > > manually or automatically every weekend, but I could ask submitters of
> > > > critical changes: “Did you run this test that we have in Avocado
> dir?”,
> > > > without specifying test details, procedures, etc. All this is a BIG
> deal
> > > > for me.
> > > >
> > > > On the other hand, I agree that certain group of tests (envisioned for
> > > > daily or so Travis CI) should have some stricter limitations and
> structure.
> > > > But right now I feel humpered by it, and this is counterproductive.
> > > >
> > > > So, we want freedom, responsibility and ownersheep of our tests.
> Please
> > > > give us the opportunity to get down on business and start writing
> tests and
> > > > start testing.
> > > >
> > > > Yours,
> > > > Aleksandar
> >
> > --
> > Eduardo

-- 
Eduardo