[igt-dev] ✗ Fi.CI.BAT: failure for new engine discovery interface (rev7)

[Petri Latvala]

On Mon, Feb 25, 2019 at 01:22:02PM +0000, Tvrtko Ursulin wrote:
> 
> On 15/02/2019 09:50, Petri Latvala wrote:
> > On Tue, Feb 12, 2019 at 08:43:24AM +0000, Tvrtko Ursulin wrote:
> > > Is generating subtest names without accessing the driver an absolute
> > > requirement?
> > 
> > Yes.
> 
> I don't like how this creates a coupling between CI _implementation_ and IGT
> codebase and think can be improved.


I see the underlying difference in our thinking now. To me, having
subtest lists statically available _enables_ decoupling IGT codebase
from systems that run the tests.



> 
> > 
> > > This particular failure would be fixable if the engine list was initialized
> > > on the first call to __for_each_engine_class_instance, as called by the
> > > perf_pmu above. And the fixture block is already ran in test enumeration,
> > > where the driver is opened. I think a lot of tests are like this. So in this
> > > context a simple query ioctl on top doesn't sound bad.
> > 
> > Fixtures are not executed when enumerating tests.
> 
> Okay.
> 
> > 
> > > I think it would be simpler if we didn't have to maintain a separate static
> > > list of all possible engines. To make that work we would need to detect the
> > > mode of execution (list vs execute) in the engine iterator as well. So gain,
> > > to me it sounds preferable that tests would be allowed to enumerate
> > > dynamically.
> > 
> > Can the tests be refactored to have a subtest per engine _class_?
> > That's a static list, isn't it? Within the subtest they would loop
> > over instances of the class. Kind of like tests that loop over
> > connectors of $type.
> 
> Sounds quite invasive, fundamental and limiting (how to run a test against a
> single engine in this world?) change.


How often does this requirement come up?

CI, by definition, wants to run all tests there are. FSVO all.

Developers and validation people might want to execute tests for a
single engine?

Two ways of executing, one machinery doesn't fit all, one needs to do
something special. Why not an environment variable that says
IGT_ONLY_ENGINES_IN_I915_ARE=rcs2,bsd1?


>How about instead, which is more or
> less copy & paste from my reply to Arek:
> 
> Would it be feasible to generate the test list on sharded platforms?
> 
> It could be a "list tests" job which you could send to each of the shard
> platforms (one machine of each platforms), executed before randomizing and
> chunking.

Feasible sure, but I can't say anything how hard that is to make
happen. There are advantages to this, but I don't see how engine
enumeration is one, when CI wants to anyway execute tests on all of
them.


> 
> Advantage there would be that you don't send chunks containing tests which a
> particular platform cannot run.


This would indeed be nice.

I have been fermenting some thoughts about refactoring the tests to
express their requirements more declaratively instead of with
imperative code. Having the possibility of skipping a whole bunch of
tests as soon as you know a declared requirement is not available
could tie well with this kind of a feature.


> And of course another advantage is that we remove unnecessary coupling
> between two components so IGT maintenance becomes easier. (No two sets of
> engines, no two ways to iterate those sets, and no need to remember when to
> use each.)


The thing is, this kind of a decoupling IGT's test enumeration from
CI's usage of the enumeration keeps current CI working but hinders
other usages. I regularly run IGT with ezbench, which does bisecting
automatically for me, and having tests just come and go depending on
the running kernel on occasion breaks things in an irritating way. We
already have those, in the form of kernel selftests.

I'm not completely opposed of changing the philosophy of having
statically enumerated subtests, but changing that will require the
rest of the world to be ready for the change. Currently, I don't even
have an exhaustive list of the world changes needed.


-- 
Petri Latvala