[Intel-xe] [PATCH 1/2] drm/xe: Group normal kunit tests in a single module

[Lucas De Marchi]

On Wed, Nov 29, 2023 at 08:29:12PM +0100, Michal Wajdeczko wrote:
>
>
>On 22.11.2023 21:31, Lucas De Marchi wrote:
>> Creating one module for each compilation unit to be tested seems
>> excessive as the number of tests increase. Group them all in a single
>> kunit test module called xe_test.ko.
>
>maybe even we don't need a separate test module ?
>
>mostly we are testing low level internals, pure kunit tests cases can be
>included by respective .c files and we can avoid need to export many
>functions/structs that could stay static.
>
>IMO separate module is more beneficial for closed-box testing, not sure
>which of our tests fits into that category

I think there's a small benefit of letting people think twice before
spreading test things throughout the codebase and then not knowing
anymore what's there just for tests...

>
>but anyway, single module is better than many, so

let's start with the transition to 1 module and if it's too cumbersome,
we can move to no extra modules in future.

>
>Acked-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>
>with one nit below
>
>>
>> The tests requiring the physical device, aka "live" tests, are still
>> kept in separate modules since they are normally triggered via igt,
>> and not via kunit.py. After igt is converted, those can be merged in
>> a single module as well.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/xe/tests/Makefile      |  7 ++++++-
>>  drivers/gpu/drm/xe/tests/xe_pci_test.c |  5 -----
>>  drivers/gpu/drm/xe/tests/xe_rtp_test.c |  5 -----
>>  drivers/gpu/drm/xe/tests/xe_test_mod.c | 10 ++++++++++
>>  drivers/gpu/drm/xe/tests/xe_wa_test.c  |  5 -----
>>  5 files changed, 16 insertions(+), 16 deletions(-)
>>  create mode 100644 drivers/gpu/drm/xe/tests/xe_test_mod.c
>>
>> diff --git a/drivers/gpu/drm/xe/tests/Makefile b/drivers/gpu/drm/xe/tests/Makefile
>> index 39d8a0892274..9d1d88af8b2f 100644
>> --- a/drivers/gpu/drm/xe/tests/Makefile
>> +++ b/drivers/gpu/drm/xe/tests/Makefile
>> @@ -1,10 +1,15 @@
>>  # SPDX-License-Identifier: GPL-2.0
>>
>> +# "live" kunit tests
>>  obj-$(CONFIG_DRM_XE_KUNIT_TEST) += \
>>  	xe_bo_test.o \
>>  	xe_dma_buf_test.o \
>>  	xe_migrate_test.o \
>> -	xe_mocs_test.o \
>> +	xe_mocs_test.o
>> +
>> +# Normal kunit tests
>> +obj-$(CONFIG_DRM_XE_KUNIT_TEST) += xe_test.o
>> +xe_test-y = xe_test_mod.o \
>>  	xe_pci_test.o \
>>  	xe_rtp_test.o \
>>  	xe_wa_test.o
>> diff --git a/drivers/gpu/drm/xe/tests/xe_pci_test.c b/drivers/gpu/drm/xe/tests/xe_pci_test.c
>> index daf652517608..59e29a148478 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_pci_test.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_pci_test.c
>> @@ -67,8 +67,3 @@ static struct kunit_suite xe_pci_test_suite = {
>>  };
>>
>>  kunit_test_suite(xe_pci_test_suite);
>> -
>> -MODULE_AUTHOR("Intel Corporation");
>> -MODULE_LICENSE("GPL");
>> -MODULE_DESCRIPTION("xe_pci kunit test");
>> -MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
>> diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
>> index a1d204133cc1..83b0ba097a3e 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
>> @@ -310,8 +310,3 @@ static struct kunit_suite xe_rtp_test_suite = {
>>  };
>>
>>  kunit_test_suite(xe_rtp_test_suite);
>> -
>> -MODULE_AUTHOR("Intel Corporation");
>> -MODULE_LICENSE("GPL");
>> -MODULE_DESCRIPTION("xe_rtp kunit test");
>> -MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
>> diff --git a/drivers/gpu/drm/xe/tests/xe_test_mod.c b/drivers/gpu/drm/xe/tests/xe_test_mod.c
>> new file mode 100644
>> index 000000000000..875f3e6f965e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/tests/xe_test_mod.c
>
>nit: can't this be just named as xe_test.c ?

no, I tried that before deciding by the _mod, but that creates a
circular dep on the Makefile.


obj-$(CONFIG_DRM_XE_KUNIT_TEST) += xe_test.o
xe_test-y = xe_test.o \
	...

make[8]: Circular drivers/gpu/drm/xe/tests/xe_test.o <- drivers/gpu/drm/xe/tests/xe_test.o dependency dropped.


Lucas De Marchi

>
>> @@ -0,0 +1,10 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +#include <linux/module.h>
>> +
>> +MODULE_AUTHOR("Intel Corporation");
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("xe kunit tests");
>> +MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
>> diff --git a/drivers/gpu/drm/xe/tests/xe_wa_test.c b/drivers/gpu/drm/xe/tests/xe_wa_test.c
>> index 01ea974591ea..4eb8e1e8a76c 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_wa_test.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_wa_test.c
>> @@ -135,8 +135,3 @@ static struct kunit_suite xe_rtp_test_suite = {
>>  };
>>
>>  kunit_test_suite(xe_rtp_test_suite);
>> -
>> -MODULE_AUTHOR("Intel Corporation");
>> -MODULE_LICENSE("GPL");
>> -MODULE_DESCRIPTION("xe_wa kunit test");
>> -MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);