[igt-dev] [PATCH i-g-t] tests/i915/gem_sync: igt_require to check availability of engine in list_engines

[Melkaveri, Arjun]

On Mon, Aug 23, 2021 at 05:26:08AM +0200, Zbigniew Kempczyński wrote:
> On Sun, Aug 22, 2021 at 05:46:26PM +0530, Arjun Melkaveri wrote:
> > Replaced igt_assert with igt_require in list_engines , to
> > avoid false failure of test case if engine is not supported
> > or found .
> > Test will return Test requirement not met.
> > 
> > Signed-off-by: Arjun Melkaveri <arjun.melkaveri at intel.com>
> > Cc: Ashutosh Dixit <ashutosh.dixit at intel.com>
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > ---
> >  tests/i915/gem_sync.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> > index 6cb00c40..4ad264a2 100644
> > --- a/tests/i915/gem_sync.c
> > +++ b/tests/i915/gem_sync.c
> > @@ -106,9 +106,9 @@ list_engines(int fd, const intel_ctx_t *ctx, unsigned ring)
> >  		ied = intel_engine_list_for_ctx_cfg(fd, &ctx->cfg);
> >  	} else {
> >  		if (ctx->cfg.num_engines)
> > -			igt_assert(ring < ctx->cfg.num_engines);
> > +			igt_require(ring < ctx->cfg.num_engines);
> >  		else
> > -			igt_assert(gem_has_ring(fd, ring));
> > +			igt_require(gem_has_ring(fd, ring));
> 
> To be honest I don't like this change. If we have invalid ring
> here it smells like bug in the code. If caller is passed 
> ctx it should be created over cfg, so it contains valid (imo)
> engine set. Thus if someone passed invalid ring I would assume
> this is bug in the code and assert is correct bahavior here.
> 
> What are scenarios you're trying to handle?
> 
> --
> Zbigniew
This test is asserting for render engine in platform that doesnt support it.
It used to skip before test was converted to use intel_ctx_t .
(https://patchwork.freedesktop.org/patch/442850/)

-Arjun 
> 
> >  
> >  		ied.engines[ied.nengines].flags = ring;
> >  		strcpy(ied.engines[ied.nengines].name, " ");
> > -- 
> > 2.25.1
> >