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

[Zbigniew Kempczyński]

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

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