Re: [igt-dev] [PATCH i-g-t 1/1] tests: Remove tools_test

Daniel Vetter
2018-09-14T15:26:19Z

On Fri, Sep 14, 2018 at 04:27:14PM +0300, Jani Nikula wrote:
> On Fri, 14 Sep 2018, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Wed, Sep 12, 2018 at 09:56:24AM +0300, Arkadiusz Hiler wrote:
> >> On Wed, Sep 12, 2018 at 09:51:24AM +0300, Petri Latvala wrote:
> >> > The test is basically just testing if the tools work properly with
> >> > little relevance to testing if the kernel works properly. If the
> >> > purpose was to test the kernel (or hardware), actual tests will be
> >> > better suited for the purpose. If the purpose was to test if the tools
> >> > work, sanity checks somewhere else for all tools instead of just two
> >> > is better suited for the purpose.
> >> >
> >> > In a nutshell, tools_test as such is fairly useless and has thus far
> >> > only revealed problems in testing setups (incorrect paths etc) instead
> >> > of problems in kernel or hardware.
> >> >
> >> > Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> >> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> >> Acked-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> >
> > Hm, not terribly happy about this. We have blown up the tools in trivial
> > ways before, that's why I wrote these. And yes it's incomplete coverage.
> > But we're also not going around deleting other igt tests, just because
> > they're not yet fully covering a given area.
>
> I think there's value in ensuring we don't break the tools from an ABI
> perspective. But at the same time I don't think we need to test the
> tools for every patch posted on the list. I.e. tool testing should be
> separated from ABI testing.
>
> So here's an idea. Extend the tools with a test subcommand or --test
> parameter, and have the tools themselves check that the ABI they need
> works. Centralize that information in the tools.
>
> For example, 'intel_reg dump' is a ridiculously slow and potentially
> dangerous way to test if intel_reg works. I've overlooked that in
> tools_test; I would never have added something like that in intel_reg.

It's not so much the kernel abi, but libdrm that we've blown up. Or is
that what you mean?

But yeah, a more systematic approach to testing this stuff could be good.
A also honestly wonder how much use the reg dumpers still have, since we
never managed to upstream Damien's gen9+ reg dump tooling afaik :-/
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation