Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 1/2] lib/igt_kmod: place KUnit tests on a subtest
@ 2023-06-05 10:47 Mauro Carvalho Chehab
  2023-06-05 10:47 ` [igt-dev] [PATCH i-g-t 2/2] kunit tests: add an optional name for the selftests Mauro Carvalho Chehab
  2023-06-05 12:10 ` [igt-dev] ✗ Fi.CI.BUILD: failure for series starting with [i-g-t,1/2] lib/igt_kmod: place KUnit tests on a subtest Patchwork
  0 siblings, 2 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-05 10:47 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

There's a hidden bug at KUnit implementation: as it doesn't
place tests inside a subtest, trying to use it with igt_main
causes a crash:

	$ ./build/tests/drm_mm --list
	skipping is allowed only in fixtures, subtests or igt_simple_main
	please refer to lib/igt_core documentation
	drm_mm: ../lib/igt_core.c:437: internal_assert: Assertion `0' failed.
	Received signal SIGABRT.
	Stack trace:
	 #0 [fatal_sig_handler+0x17b]
	 #1 [__sigaction+0x50]
	 #2 [__pthread_kill_implementation+0x114]
	 #3 [gsignal+0x1e]
	 #4 [abort+0xdf]
	 #5 [__assert_fail_base.cold+0xe]
	 #6 [__assert_fail+0x47]
	 #7 [internal_assert+0xe5]
	 #8 [igt_skip+0x169]
	 #9 [__igt_skip_check+0x1bb]
	 #10 [igt_ktest_begin+0xa6]
	 #11 [igt_kunit+0x70]
	 #12 [main+0x2a]
	 #13 [__libc_start_call_main+0x7a]
	 #14 [__libc_start_main+0x8b]
	 #15 [_start+0x25]

Fix it by using igt_subtests() before actually implememnting
KUnit logic.

After the patch, it should now report subtests:

	$ ./build/tests/drm_mm --list
	all-tests

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_kmod.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 8cb9cb2e90b9..1309ab212b11 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -754,7 +754,7 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
  *
  * Returns: IGT default codes
  */
-int igt_kunit(const char *module_name, const char *opts)
+static int __igt_kunit(const char *module_name, const char *opts)
 {
 	struct igt_ktest tst;
 	struct kmod_module *kunit_kmod;
@@ -852,6 +852,20 @@ unload:
 	return ret;
 }
 
+int igt_kunit(const char *module_name, const char *opts)
+{
+	/*
+	 * We need to use igt_subtest here, as otherwise it may crash with:
+	 *  skipping is allowed only in fixtures, subtests or igt_simple_main
+	 * if used on igt_main. This is also needed in order to provide
+	 * proper namespace for dynamic subtests, with is required for CI
+	 * and for documentation.
+	 */
+	igt_subtest_with_dynamic("all-tests")
+		return __igt_kunit(module_name, opts);
+	return 0;
+}
+
 static int open_parameters(const char *module_name)
 {
 	char path[256];
-- 
2.40.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [igt-dev] [PATCH i-g-t 2/2] kunit tests: add an optional name for the selftests
  2023-06-05 10:47 [igt-dev] [PATCH i-g-t 1/2] lib/igt_kmod: place KUnit tests on a subtest Mauro Carvalho Chehab
@ 2023-06-05 10:47 ` Mauro Carvalho Chehab
  2023-06-05 12:10 ` [igt-dev] ✗ Fi.CI.BUILD: failure for series starting with [i-g-t,1/2] lib/igt_kmod: place KUnit tests on a subtest Patchwork
  1 sibling, 0 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-05 10:47 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

When multiple KUnit tests are called by the same program, it is
interesting to group them with a name. This would allow IGT
namespace to better refer to the KUnit tests and will give some
filtering capability to it.

After those changes, the IGT kUnit tests will be better named:

	$ for i in kms_selftest drm_buddy drm_mm; do echo $i:; build/tests/$i --list; echo; done
	kms_selftest:
	drm_cmdline
	drm_damage
	drm_dp_mst
	drm_format_helper
	drm_format
	framebuffer
	drm_plane
	all-tests

	drm_buddy:
	all-tests

	drm_mm:
	all-tests

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_kmod.c       |  7 +++++--
 lib/igt_kmod.h       |  2 +-
 tests/drm_buddy.c    |  2 +-
 tests/drm_mm.c       |  3 ++-
 tests/kms_selftest.c | 23 +++++++++++++++++------
 5 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 1309ab212b11..c62eb97a19a1 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -852,7 +852,7 @@ unload:
 	return ret;
 }
 
-int igt_kunit(const char *module_name, const char *opts)
+int igt_kunit(const char *module_name, const char *name, const char *opts)
 {
 	/*
 	 * We need to use igt_subtest here, as otherwise it may crash with:
@@ -861,7 +861,10 @@ int igt_kunit(const char *module_name, const char *opts)
 	 * proper namespace for dynamic subtests, with is required for CI
 	 * and for documentation.
 	 */
-	igt_subtest_with_dynamic("all-tests")
+	if (name == NULL)
+		name = "all-tests";
+
+	igt_subtest_with_dynamic(name)
 		return __igt_kunit(module_name, opts);
 	return 0;
 }
diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
index ce17c714e292..248955475a90 100644
--- a/lib/igt_kmod.h
+++ b/lib/igt_kmod.h
@@ -71,7 +71,7 @@ static inline int igt_xe_driver_unload(void)
 int igt_amdgpu_driver_load(const char *opts);
 int igt_amdgpu_driver_unload(void);
 
-int igt_kunit(const char *module_name, const char *opts);
+int igt_kunit(const char *module_name, const char *name, const char *opts);
 
 void igt_kselftests(const char *module_name,
 		    const char *module_options,
diff --git a/tests/drm_buddy.c b/tests/drm_buddy.c
index 3261f0d61dfa..09feaf635311 100644
--- a/tests/drm_buddy.c
+++ b/tests/drm_buddy.c
@@ -10,7 +10,7 @@ IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator (struct drm_bu
 
 igt_main
 {
-	int ret = igt_kunit("drm_buddy_test", NULL);
+	int ret = igt_kunit("drm_buddy_test", NULL, NULL);
 	if (ret != 0 && ret != IGT_EXIT_ABORT)
 		igt_kselftests("test-drm_buddy", NULL, NULL, NULL);
 }
diff --git a/tests/drm_mm.c b/tests/drm_mm.c
index 88f76a57cc0c..ada8cb936919 100644
--- a/tests/drm_mm.c
+++ b/tests/drm_mm.c
@@ -156,7 +156,8 @@ IGT_TEST_DESCRIPTION("Basic sanity check of DRM's range manager (struct drm_mm)"
 
 igt_main
 {
-	int ret = igt_kunit("drm_mm_test", NULL);
+	int ret = igt_kunit("drm_mm_test", NULL, NULL);
+
 	if (ret != 0 && ret != IGT_EXIT_ABORT)
 		igt_kselftests("test-drm_mm", NULL, NULL, NULL);
 }
diff --git a/tests/kms_selftest.c b/tests/kms_selftest.c
index b27f60fb3a54..d83e5ff4b5e7 100644
--- a/tests/kms_selftest.c
+++ b/tests/kms_selftest.c
@@ -26,15 +26,26 @@
 
 IGT_TEST_DESCRIPTION("Basic sanity check of KMS selftests.");
 
+struct kms_kunittests {
+	const char *kunit;
+	const char *name;
+};
+
 igt_main
 {
-	static const char *kunit_subtests[] = { "drm_cmdline_parser_test", "drm_damage_helper_test",
-						"drm_dp_mst_helper_test", "drm_format_helper_test",
-						"drm_format_test", "drm_framebuffer_test",
-						"drm_plane_helper_test", NULL };
+	static const struct kms_kunittests kunit_subtests[] = {
+		{ "drm_cmdline_parser_test",	"drm_cmdline" },
+		{ "drm_damage_helper_test",	"drm_damage" },
+		{ "drm_dp_mst_helper_test",	"drm_dp_mst" },
+		{ "drm_format_helper_test",	"drm_format_helper" },
+		{ "drm_format_test",		"drm_format" },
+		{ "drm_framebuffer_test",	"framebuffer" },
+		{ "drm_plane_helper_test",	"drm_plane" },
+		{ NULL, NULL}
+	};
 
-	for (int i = 0; kunit_subtests[i] != NULL; i++)
-		igt_kunit(kunit_subtests[i], NULL);
+	for (int i = 0; kunit_subtests[i].kunit != NULL; i++)
+		igt_kunit(kunit_subtests[i].kunit, kunit_subtests[i].name, NULL);
 
 	igt_kselftests("test-drm_modeset", NULL, NULL, NULL);
 }
-- 
2.40.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [igt-dev] ✗ Fi.CI.BUILD: failure for series starting with [i-g-t,1/2] lib/igt_kmod: place KUnit tests on a subtest
  2023-06-05 10:47 [igt-dev] [PATCH i-g-t 1/2] lib/igt_kmod: place KUnit tests on a subtest Mauro Carvalho Chehab
  2023-06-05 10:47 ` [igt-dev] [PATCH i-g-t 2/2] kunit tests: add an optional name for the selftests Mauro Carvalho Chehab
@ 2023-06-05 12:10 ` Patchwork
  1 sibling, 0 replies; 3+ messages in thread
From: Patchwork @ 2023-06-05 12:10 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/2] lib/igt_kmod: place KUnit tests on a subtest
URL   : https://patchwork.freedesktop.org/series/118858/
State : failure

== Summary ==

Applying: lib/igt_kmod: place KUnit tests on a subtest
Using index info to reconstruct a base tree...
M	lib/igt_kmod.c
Falling back to patching base and 3-way merge...
Auto-merging lib/igt_kmod.c
CONFLICT (content): Merge conflict in lib/igt_kmod.c
Patch failed at 0001 lib/igt_kmod: place KUnit tests on a subtest
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-06-05 12:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-05 10:47 [igt-dev] [PATCH i-g-t 1/2] lib/igt_kmod: place KUnit tests on a subtest Mauro Carvalho Chehab
2023-06-05 10:47 ` [igt-dev] [PATCH i-g-t 2/2] kunit tests: add an optional name for the selftests Mauro Carvalho Chehab
2023-06-05 12:10 ` [igt-dev] ✗ Fi.CI.BUILD: failure for series starting with [i-g-t,1/2] lib/igt_kmod: place KUnit tests on a subtest Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox