[igt-dev] [PATCH i-g-t] tests: Avoid the name "all"

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Tue Jul 26 09:01:32 UTC 2022


On 26/07/2022 08:28, Petri Latvala wrote:
> If using a subtest or a dynamic subtest name "all", the piglit format
> name will contain the string "@all". When filing bugs for test
> failures, one needs extra care not to accidentally ping every
> developer on gitlab.

Three questions:

1)
What if someone adds a subtest called all tomorrow or next year?

2)
Are there other keywords with unintended side-effects?

3)
Is there a component sitting between the IGT output and putting it into 
GitLab which would be ideally placed to sanitize and do that reliably 
and with less maintenance effort?

Regards,

Tvrtko

> Along with having that extra care, try to avoid the possibility of
> accidents by renaming everything with just "all" in their name to a
> more descriptive form that explains what the list of "all" contains.
> 
> Closes: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/121
> Signed-off-by: Petri Latvala <petri.latvala at intel.com>
> ---
>   lib/igt_kmod.c                        | 2 +-
>   tests/i915/gem_busy.c                 | 2 +-
>   tests/i915/gem_ctx_engines.c          | 2 +-
>   tests/i915/gem_exec_gttfill.c         | 2 +-
>   tests/i915/gem_exec_reloc.c           | 2 +-
>   tests/i915/gem_sync.c                 | 2 +-
>   tests/i915/gem_wait.c                 | 2 +-
>   tests/i915/sysfs_clients.c            | 2 +-
>   tests/intel-ci/fast-feedback.testlist | 8 ++++----
>   9 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index dfdcfcc5..05484f76 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -1015,7 +1015,7 @@ void igt_kselftests(const char *module_name,
>   		igt_require(igt_kselftest_begin(&tst) == 0);
>   
>   	igt_kselftest_get_tests(tst.kmod, filter, &tests);
> -	igt_subtest_with_dynamic(filter ?: "all") {
> +	igt_subtest_with_dynamic(filter ?: "all-tests") {
>   		igt_list_for_each_entry_safe(tl, tn, &tests, link) {
>   			unsigned long taints;
>   
> diff --git a/tests/i915/gem_busy.c b/tests/i915/gem_busy.c
> index 3f104206..f11fa877 100644
> --- a/tests/i915/gem_busy.c
> +++ b/tests/i915/gem_busy.c
> @@ -449,7 +449,7 @@ igt_main
>   
>   		igt_describe("Basic test to check busyness of each engine.");
>   		igt_subtest_with_dynamic("busy") {
> -			igt_dynamic("all") {
> +			igt_dynamic("all-engines") {
>   				gem_quiescent_gpu(fd);
>   				all(fd, ctx);
>   			}
> diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
> index 4b8e5145..53b04358 100644
> --- a/tests/i915/gem_ctx_engines.c
> +++ b/tests/i915/gem_ctx_engines.c
> @@ -631,7 +631,7 @@ igt_main
>   			igt_dynamic_f("%s", e->name)
>   				independent(i915, ctx, e);
>   		}
> -		igt_dynamic("all")
> +		igt_dynamic("all-engines")
>   			independent_all(i915, ctx);
>   	}
>   
> diff --git a/tests/i915/gem_exec_gttfill.c b/tests/i915/gem_exec_gttfill.c
> index f9f244d6..c9a7ab50 100644
> --- a/tests/i915/gem_exec_gttfill.c
> +++ b/tests/i915/gem_exec_gttfill.c
> @@ -254,7 +254,7 @@ igt_main
>   
>   	igt_describe("Stress test check behaviour/correctness of handling"
>   		     " batches to fill gtt");
> -	igt_subtest("all")
> +	igt_subtest("all-engines")
>   		fillgtt(i915, ctx, ALL_ENGINES, 20);
>   
>   	igt_fixture {
> diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
> index 1f5d13e4..7a354a32 100644
> --- a/tests/i915/gem_exec_reloc.c
> +++ b/tests/i915/gem_exec_reloc.c
> @@ -1145,7 +1145,7 @@ igt_main
>   		from_gpu(fd);
>   
>   	igt_subtest_with_dynamic("basic-active") {
> -		igt_dynamic("all")
> +		igt_dynamic("all-engines")
>   			active(fd, ctx, ALL_ENGINES);
>   
>   		for_each_ctx_engine(fd, ctx, e) {
> diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> index 8ee33c77..07cabf7a 100644
> --- a/tests/i915/gem_sync.c
> +++ b/tests/i915/gem_sync.c
> @@ -1303,7 +1303,7 @@ igt_main
>   		store_all(fd, ctx, 1, 2);
>   
>   	igt_describe("Extended version of existing basic-all test.");
> -	igt_subtest("all")
> +	igt_subtest("wait-all")
>   		sync_all(fd, ctx, 1, 20);
>   	igt_describe("Extended version of existing basic-store-all test.");
>   	igt_subtest("store-all")
> diff --git a/tests/i915/gem_wait.c b/tests/i915/gem_wait.c
> index 230304aa..27d084af 100644
> --- a/tests/i915/gem_wait.c
> +++ b/tests/i915/gem_wait.c
> @@ -160,7 +160,7 @@ static void test_all_engines(const char *name, int i915, const intel_ctx_t *ctx,
>   	const struct intel_execution_engine2 *e;
>   
>   	igt_subtest_with_dynamic(name) {
> -		igt_dynamic("all") {
> +		igt_dynamic("all-engines") {
>   			gem_quiescent_gpu(i915);
>   			basic(i915, ctx, ALL_ENGINES, test);
>   			gem_quiescent_gpu(i915);
> diff --git a/tests/i915/sysfs_clients.c b/tests/i915/sysfs_clients.c
> index ab79ff18..b49e6a55 100644
> --- a/tests/i915/sysfs_clients.c
> +++ b/tests/i915/sysfs_clients.c
> @@ -970,7 +970,7 @@ static void test_busy(int i915, int clients)
>   			}
>   		}
>   
> -		igt_dynamic("all") {
> +		igt_dynamic("all-engines") {
>   			gem_quiescent_gpu(i915);
>   			igt_fork(child, 1)
>   				busy_all(i915, clients, &cfg);
> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> index bd5538a0..541c1882 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -12,7 +12,7 @@ igt at fbdev@write
>   igt at gem_basic@bad-close
>   igt at gem_basic@create-close
>   igt at gem_basic@create-fd-close
> -igt at gem_busy@busy at all
> +igt at gem_busy@busy at all-engines
>   igt at gem_close_race@basic-process
>   igt at gem_close_race@basic-threads
>   igt at gem_ctx_create@basic
> @@ -47,8 +47,8 @@ igt at gem_sync@basic-each
>   igt at gem_tiled_blits@basic
>   igt at gem_tiled_fence_blits@basic
>   igt at gem_tiled_pread_basic
> -igt at gem_wait@busy at all
> -igt at gem_wait@wait at all
> +igt at gem_wait@busy at all-engines
> +igt at gem_wait@wait at all-engines
>   igt at i915_getparams_basic@basic-eu-total
>   igt at i915_getparams_basic@basic-subslice-total
>   igt at i915_hangman@error-state-basic
> @@ -166,7 +166,7 @@ igt at i915_pm_rpm@module-reload
>   
>   # Kernel selftests
>   igt at i915_selftest@live
> -igt at dmabuf@all
> +igt at dmabuf@all-tests
>   
>   # System wide suspend tests
>   igt at i915_suspend@basic-s2idle-without-i915


More information about the igt-dev mailing list