[PATCH v3 0/1] Add KUnit tests for llist

Artur Alves posted 1 patch 2 months, 1 week ago
lib/Kconfig.debug       |  11 ++
lib/tests/Makefile      |   1 +
lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 370 insertions(+)
create mode 100644 lib/tests/llist_kunit.c
[PATCH v3 0/1] Add KUnit tests for llist
Posted by Artur Alves 2 months, 1 week ago
Hi all,

This is part of a hackathon organized by LKCAMP[1], focused on writing
tests using KUnit. We reached out a while ago asking for advice on what
would be a useful contribution[2] and ended up choosing data structures
that did not yet have tests. 

This patch adds tests for the llist data structure, defined in 
include/linux/llist.h, and is inspired by the KUnit tests for the doubly
linked list in lib/list-test.c[3].

It is important to note that this patch depends on the patch referenced
in [4], as it utilizes the newly created lib/tests/ subdirectory.

[1] https://lkcamp.dev/about/
[2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
[3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
[4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/

---
Changes in v3:
    - Resolved checkpatch warnings:
        - Renamed tests for macros starting with 'for_each'
        - Removed link from commit message
    - Replaced hardcoded constants with ENTRIES_SIZE
    - Updated initialization of llist_node array
    - Fixed typos
    - Update Kconfig.debug message for llist_kunit 

Changes in v2:
    - Add MODULE_DESCRIPTION()
    - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
    - Change the license from "GPL v2" to "GPL"

Artur Alves (1):
  lib/llist_kunit.c: add KUnit tests for llist

 lib/Kconfig.debug       |  11 ++
 lib/tests/Makefile      |   1 +
 lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 370 insertions(+)
 create mode 100644 lib/tests/llist_kunit.c

-- 
2.46.0
Re: [PATCH v3 0/1] Add KUnit tests for llist
Posted by Shuah Khan 2 months, 1 week ago
On 9/16/24 18:51, Artur Alves wrote:
> Hi all,
> 
> This is part of a hackathon organized by LKCAMP[1], focused on writing
> tests using KUnit. We reached out a while ago asking for advice on what
> would be a useful contribution[2] and ended up choosing data structures
> that did not yet have tests.
> 
> This patch adds tests for the llist data structure, defined in
> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
> linked list in lib/list-test.c[3].
> 
> It is important to note that this patch depends on the patch referenced
> in [4], as it utilizes the newly created lib/tests/ subdirectory.
> 
> [1] https://lkcamp.dev/about/
> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
> 
> ---
> Changes in v3:
>      - Resolved checkpatch warnings:
>          - Renamed tests for macros starting with 'for_each'

Shouldn't this a separate patch to make it easy to review?

>          - Removed link from commit message
>      - Replaced hardcoded constants with ENTRIES_SIZE

Shouldn't this a separate patch to make it easy to review?

>      - Updated initialization of llist_node array
>      - Fixed typos
>      - Update Kconfig.debug message for llist_kunit

Are these changes to existing code or warnings on your added code?
> 
> Changes in v2:
>      - Add MODULE_DESCRIPTION()
>      - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
>      - Change the license from "GPL v2" to "GPL"
> 
> Artur Alves (1):
>    lib/llist_kunit.c: add KUnit tests for llist
> 
>   lib/Kconfig.debug       |  11 ++
>   lib/tests/Makefile      |   1 +
>   lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 370 insertions(+)
>   create mode 100644 lib/tests/llist_kunit.c
> 

You are combining lot of changes in one single patch. Each change as a separate
patch will help reviewers.

Adding new test should be a separate patch.

- renaming as a separate patch

thanks,
-- Shuah
Re: [PATCH v3 0/1] Add KUnit tests for llist
Posted by David Gow 2 months, 1 week ago
On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 9/16/24 18:51, Artur Alves wrote:
> > Hi all,
> >
> > This is part of a hackathon organized by LKCAMP[1], focused on writing
> > tests using KUnit. We reached out a while ago asking for advice on what
> > would be a useful contribution[2] and ended up choosing data structures
> > that did not yet have tests.
> >
> > This patch adds tests for the llist data structure, defined in
> > include/linux/llist.h, and is inspired by the KUnit tests for the doubly
> > linked list in lib/list-test.c[3].
> >
> > It is important to note that this patch depends on the patch referenced
> > in [4], as it utilizes the newly created lib/tests/ subdirectory.
> >
> > [1] https://lkcamp.dev/about/
> > [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
> > [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
> > [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
> >
> > ---
> > Changes in v3:
> >      - Resolved checkpatch warnings:
> >          - Renamed tests for macros starting with 'for_each'
>
> Shouldn't this a separate patch to make it easy to review?
>

I think that, if this were renaming these in an already existing test
(like the confusingly similar list test), then yes. But since it's
only a change from v2, I think we're okay.

> >          - Removed link from commit message
> >      - Replaced hardcoded constants with ENTRIES_SIZE
>
> Shouldn't this a separate patch to make it easy to review?

Again, if we want to change this in other tests (list, hlist) we
should split it into a separate patch, but I think it's okay for llist
to go in with these already cleaned up.

>
> >      - Updated initialization of llist_node array
> >      - Fixed typos
> >      - Update Kconfig.debug message for llist_kunit
>
> Are these changes to existing code or warnings on your added code?

I think these are all changes to the added code since v2. Artur, is that right?

> >
> > Changes in v2:
> >      - Add MODULE_DESCRIPTION()
> >      - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
> >      - Change the license from "GPL v2" to "GPL"
> >
> > Artur Alves (1):
> >    lib/llist_kunit.c: add KUnit tests for llist
> >
> >   lib/Kconfig.debug       |  11 ++
> >   lib/tests/Makefile      |   1 +
> >   lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 370 insertions(+)
> >   create mode 100644 lib/tests/llist_kunit.c
> >
>
> You are combining lot of changes in one single patch. Each change as a separate
> patch will help reviewers.
>
> Adding new test should be a separate patch.
>
> - renaming as a separate patch
>

I think given that these are just changes between patch versions, not
renaming/modifying already committed code, that this is okay to go in
as one patch?

The actual patch is only doing one thing: adding a test suite for the
llist structure. I don't see the point in committing a version of it
only to immediately rename things and clean bits up separately in this
case.


Cheers,
-- David
Re: [PATCH v3 0/1] Add KUnit tests for llist
Posted by Artur Alves Cavalcante de Barros 2 months, 1 week ago
On 9/20/24 4:10 AM, David Gow wrote:
> On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 9/16/24 18:51, Artur Alves wrote:
>>> Hi all,
>>>
>>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>>> tests using KUnit. We reached out a while ago asking for advice on what
>>> would be a useful contribution[2] and ended up choosing data structures
>>> that did not yet have tests.
>>>
>>> This patch adds tests for the llist data structure, defined in
>>> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
>>> linked list in lib/list-test.c[3].
>>>
>>> It is important to note that this patch depends on the patch referenced
>>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>>
>>> [1] https://lkcamp.dev/about/
>>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>>> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>>>
>>> ---
>>> Changes in v3:
>>>       - Resolved checkpatch warnings:
>>>           - Renamed tests for macros starting with 'for_each'
>>
>> Shouldn't this a separate patch to make it easy to review?
>>
> 
> I think that, if this were renaming these in an already existing test
> (like the confusingly similar list test), then yes. But since it's
> only a change from v2, I think we're okay.
> 

Yes, the renaming refers to some test cases from the test suite that I'm 
adding, with the purpose of resolving some checkpatch warnings, as 
suggested by Rae Moar's review[1].

>>>           - Removed link from commit message
>>>       - Replaced hardcoded constants with ENTRIES_SIZE
>>
>> Shouldn't this a separate patch to make it easy to review?
> 
> Again, if we want to change this in other tests (list, hlist) we
> should split it into a separate patch, but I think it's okay for llist
> to go in with these already cleaned up.
> 
>>
>>>       - Updated initialization of llist_node array
>>>       - Fixed typos
>>>       - Update Kconfig.debug message for llist_kunit
>>
>> Are these changes to existing code or warnings on your added code?
> 
> I think these are all changes to the added code since v2. Artur, is that right?
> 

This is the case! All changes are in the added code, so it doesn't 
introduce any checkpatch warnings that were present in v2.

>>>
>>> Changes in v2:
>>>       - Add MODULE_DESCRIPTION()
>>>       - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
>>>       - Change the license from "GPL v2" to "GPL"
>>>
>>> Artur Alves (1):
>>>     lib/llist_kunit.c: add KUnit tests for llist
>>>
>>>    lib/Kconfig.debug       |  11 ++
>>>    lib/tests/Makefile      |   1 +
>>>    lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
>>>    3 files changed, 370 insertions(+)
>>>    create mode 100644 lib/tests/llist_kunit.c
>>>
>>
>> You are combining lot of changes in one single patch. Each change as a separate
>> patch will help reviewers.
>>
>> Adding new test should be a separate patch.
>>
>> - renaming as a separate patch
>>
> 
> I think given that these are just changes between patch versions, not
> renaming/modifying already committed code, that this is okay to go in
> as one patch?
> 
> The actual patch is only doing one thing: adding a test suite for the
> llist structure. I don't see the point in committing a version of it
> only to immediately rename things and clean bits up separately in this
> case.
> 
> 
> Cheers,
> -- David

Thanks for replying!

I'd like to reaffirm that the patch is, in fact, doing one thing: adding 
tests for the llist data structure. All the changes in V2 and V3 refer 
to the code that I'm adding. I'm not modifying any existing list tests, 
only adding new ones.

[1] 
https://lore.kernel.org/all/20240903214027.77533-1-arturacb@gmail.com/T/#mc29a53b120d2f8589f8bd882ab972d15c8a3d202

Best regards,
- Artur
Re: [PATCH v3 0/1] Add KUnit tests for llist
Posted by Shuah Khan 2 months ago
On 9/20/24 20:49, Artur Alves Cavalcante de Barros wrote:
> On 9/20/24 4:10 AM, David Gow wrote:
>> On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>>
>>> On 9/16/24 18:51, Artur Alves wrote:
>>>> Hi all,
>>>>
>>>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>>>> tests using KUnit. We reached out a while ago asking for advice on what
>>>> would be a useful contribution[2] and ended up choosing data structures
>>>> that did not yet have tests.
>>>>
>>>> This patch adds tests for the llist data structure, defined in
>>>> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
>>>> linked list in lib/list-test.c[3].
>>>>
>>>> It is important to note that this patch depends on the patch referenced
>>>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>>>
>>>> [1] https://lkcamp.dev/about/
>>>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>>>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>>>> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>>>>
>>>> ---
>>>> Changes in v3:
>>>>       - Resolved checkpatch warnings:
>>>>           - Renamed tests for macros starting with 'for_each'
>>>
>>> Shouldn't this a separate patch to make it easy to review?
>>>
>>
>> I think that, if this were renaming these in an already existing test
>> (like the confusingly similar list test), then yes. But since it's
>> only a change from v2, I think we're okay.
>>
> 
> Yes, the renaming refers to some test cases from the test suite that I'm adding, with the purpose of resolving some checkpatch warnings, as suggested by Rae Moar's review[1].
> 
>>>>           - Removed link from commit message
>>>>       - Replaced hardcoded constants with ENTRIES_SIZE
>>>
>>> Shouldn't this a separate patch to make it easy to review?
>>
>> Again, if we want to change this in other tests (list, hlist) we
>> should split it into a separate patch, but I think it's okay for llist
>> to go in with these already cleaned up.
>>
>>>
>>>>       - Updated initialization of llist_node array
>>>>       - Fixed typos
>>>>       - Update Kconfig.debug message for llist_kunit
>>>
>>> Are these changes to existing code or warnings on your added code?
>>
>> I think these are all changes to the added code since v2. Artur, is that right?
>>
> 
> This is the case! All changes are in the added code, so it doesn't introduce any checkpatch warnings that were present in v2.
> 
>>>>
>>>> Changes in v2:
>>>>       - Add MODULE_DESCRIPTION()
>>>>       - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
>>>>       - Change the license from "GPL v2" to "GPL"
>>>>
>>>> Artur Alves (1):
>>>>     lib/llist_kunit.c: add KUnit tests for llist
>>>>
>>>>    lib/Kconfig.debug       |  11 ++
>>>>    lib/tests/Makefile      |   1 +
>>>>    lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 370 insertions(+)
>>>>    create mode 100644 lib/tests/llist_kunit.c
>>>>
>>>
>>> You are combining lot of changes in one single patch. Each change as a separate
>>> patch will help reviewers.
>>>
>>> Adding new test should be a separate patch.
>>>
>>> - renaming as a separate patch
>>>
>>
>> I think given that these are just changes between patch versions, not
>> renaming/modifying already committed code, that this is okay to go in
>> as one patch?
>>
>> The actual patch is only doing one thing: adding a test suite for the
>> llist structure. I don't see the point in committing a version of it
>> only to immediately rename things and clean bits up separately in this
>> case.
>>
>>
>> Cheers,
>> -- David
> 
> Thanks for replying!
> 
> I'd like to reaffirm that the patch is, in fact, doing one thing: adding tests for the llist data structure. All the changes in V2 and V3 refer to the code that I'm adding. I'm not modifying any existing list tests, only adding new ones.
> 
> [1] https://lore.kernel.org/all/20240903214027.77533-1-arturacb@gmail.com/T/#mc29a53b120d2f8589f8bd882ab972d15c8a3d202
> 
> Best regards,
> - Artur

Sounds good to me. It was a bit confusing because the v2 and v3 change
new and code which wasn't clear to me.

thanks,
-- Shuah


Re: [PATCH v3 0/1] Add KUnit tests for llist
Posted by Shuah Khan 2 months, 1 week ago
On 9/20/24 01:10, David Gow wrote:
> On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 9/16/24 18:51, Artur Alves wrote:
>>> Hi all,
>>>
>>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>>> tests using KUnit. We reached out a while ago asking for advice on what
>>> would be a useful contribution[2] and ended up choosing data structures
>>> that did not yet have tests.
>>>
>>> This patch adds tests for the llist data structure, defined in
>>> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
>>> linked list in lib/list-test.c[3].
>>>
>>> It is important to note that this patch depends on the patch referenced
>>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>>
>>> [1] https://lkcamp.dev/about/
>>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>>> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>>>
>>> ---
>>> Changes in v3:
>>>       - Resolved checkpatch warnings:
>>>           - Renamed tests for macros starting with 'for_each'
>>
>> Shouldn't this a separate patch to make it easy to review?
>>
> 
> I think that, if this were renaming these in an already existing test
> (like the confusingly similar list test), then yes. But since it's
> only a change from v2, I think we're okay.
> 
>>>           - Removed link from commit message
>>>       - Replaced hardcoded constants with ENTRIES_SIZE
>>
>> Shouldn't this a separate patch to make it easy to review?
> 
> Again, if we want to change this in other tests (list, hlist) we
> should split it into a separate patch, but I think it's okay for llist
> to go in with these already cleaned up.
> 
>>
>>>       - Updated initialization of llist_node array
>>>       - Fixed typos
>>>       - Update Kconfig.debug message for llist_kunit
>>
>> Are these changes to existing code or warnings on your added code?
> 
> I think these are all changes to the added code since v2. Artur, is that right?
> 
>>>
>>> Changes in v2:
>>>       - Add MODULE_DESCRIPTION()
>>>       - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
>>>       - Change the license from "GPL v2" to "GPL"
>>>
>>> Artur Alves (1):
>>>     lib/llist_kunit.c: add KUnit tests for llist
>>>
>>>    lib/Kconfig.debug       |  11 ++
>>>    lib/tests/Makefile      |   1 +
>>>    lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
>>>    3 files changed, 370 insertions(+)
>>>    create mode 100644 lib/tests/llist_kunit.c
>>>
>>
>> You are combining lot of changes in one single patch. Each change as a separate
>> patch will help reviewers.
>>
>> Adding new test should be a separate patch.
>>
>> - renaming as a separate patch
>>
> 
> I think given that these are just changes between patch versions, not
> renaming/modifying already committed code, that this is okay to go in
> as one patch?
> 
> The actual patch is only doing one thing: adding a test suite for the
> llist structure. I don't see the point in committing a version of it
> only to immediately rename things and clean bits up separately in this
> case.

I do think it will help to separate the renaming and adding a new test.
It makes it easier to follow.

thanks,
-- Shuah
Re: [PATCH v3 0/1] Add KUnit tests for llist
Posted by Artur Alves Cavalcante de Barros 2 months, 1 week ago
On 9/20/24 12:10 PM, Shuah Khan wrote:
> On 9/20/24 01:10, David Gow wrote:
>> On Fri, 20 Sept 2024 at 00:01, Shuah Khan <skhan@linuxfoundation.org> 
>> wrote:
>>>
>>> On 9/16/24 18:51, Artur Alves wrote:
>>>> Hi all,
>>>>
>>>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>>>> tests using KUnit. We reached out a while ago asking for advice on what
>>>> would be a useful contribution[2] and ended up choosing data structures
>>>> that did not yet have tests.
>>>>
>>>> This patch adds tests for the llist data structure, defined in
>>>> include/linux/llist.h, and is inspired by the KUnit tests for the 
>>>> doubly
>>>> linked list in lib/list-test.c[3].
>>>>
>>>> It is important to note that this patch depends on the patch referenced
>>>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>>>
>>>> [1] https://lkcamp.dev/about/
>>>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>>>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>>>> [4] https://lore.kernel.org/all/20240720181025.work.002- 
>>>> kees@kernel.org/
>>>>
>>>> ---
>>>> Changes in v3:
>>>>       - Resolved checkpatch warnings:
>>>>           - Renamed tests for macros starting with 'for_each'
>>>
>>> Shouldn't this a separate patch to make it easy to review?
>>>
>>
>> I think that, if this were renaming these in an already existing test
>> (like the confusingly similar list test), then yes. But since it's
>> only a change from v2, I think we're okay.
>>
>>>>           - Removed link from commit message
>>>>       - Replaced hardcoded constants with ENTRIES_SIZE
>>>
>>> Shouldn't this a separate patch to make it easy to review?
>>
>> Again, if we want to change this in other tests (list, hlist) we
>> should split it into a separate patch, but I think it's okay for llist
>> to go in with these already cleaned up.
>>
>>>
>>>>       - Updated initialization of llist_node array
>>>>       - Fixed typos
>>>>       - Update Kconfig.debug message for llist_kunit
>>>
>>> Are these changes to existing code or warnings on your added code?
>>
>> I think these are all changes to the added code since v2. Artur, is 
>> that right?
>>
>>>>
>>>> Changes in v2:
>>>>       - Add MODULE_DESCRIPTION()
>>>>       - Move the tests from lib/llist_kunit.c to lib/tests/ 
>>>> llist_kunit.c
>>>>       - Change the license from "GPL v2" to "GPL"
>>>>
>>>> Artur Alves (1):
>>>>     lib/llist_kunit.c: add KUnit tests for llist
>>>>
>>>>    lib/Kconfig.debug       |  11 ++
>>>>    lib/tests/Makefile      |   1 +
>>>>    lib/tests/llist_kunit.c | 358 +++++++++++++++++++++++++++++++++++ 
>>>> +++++
>>>>    3 files changed, 370 insertions(+)
>>>>    create mode 100644 lib/tests/llist_kunit.c
>>>>
>>>
>>> You are combining lot of changes in one single patch. Each change as 
>>> a separate
>>> patch will help reviewers.
>>>
>>> Adding new test should be a separate patch.
>>>
>>> - renaming as a separate patch
>>>
>>
>> I think given that these are just changes between patch versions, not
>> renaming/modifying already committed code, that this is okay to go in
>> as one patch?
>>
>> The actual patch is only doing one thing: adding a test suite for the
>> llist structure. I don't see the point in committing a version of it
>> only to immediately rename things and clean bits up separately in this
>> case.
> 
> I do think it will help to separate the renaming and adding a new test.
> It makes it easier to follow.
> 
> thanks,
> -- Shuah
> 

Hi, Shuah!

The renaming is in the test suite that I'm adding, as suggested by Rae 
Moar[1]. I'm not modifying any existing code; all my changes are in the 
new code that I'm adding.

I'm sorry, but it isn't clear to me. Could you please provide an example 
of what you're suggesting?

Would you prefer that I undo the renaming, submit the patch with the 
checkpatch warnings, and then follow up with a new patch to rename the 
test cases and resolve the warnings?

[1]
https://lore.kernel.org/all/20240903214027.77533-1-arturacb@gmail.com/T/#mc29a53b120d2f8589f8bd882ab972d15c8a3d202

Thanks for your time,
- Artur
Re: [PATCH v3 0/1] Add KUnit tests for llist
Posted by Artur Alves Cavalcante de Barros 2 months, 1 week ago
On 9/19/24 1:01 PM, Shuah Khan wrote:
> On 9/16/24 18:51, Artur Alves wrote:
>> Hi all,
>>
>> This is part of a hackathon organized by LKCAMP[1], focused on writing
>> tests using KUnit. We reached out a while ago asking for advice on what
>> would be a useful contribution[2] and ended up choosing data structures
>> that did not yet have tests.
>>
>> This patch adds tests for the llist data structure, defined in
>> include/linux/llist.h, and is inspired by the KUnit tests for the doubly
>> linked list in lib/list-test.c[3].
>>
>> It is important to note that this patch depends on the patch referenced
>> in [4], as it utilizes the newly created lib/tests/ subdirectory.
>>
>> [1] https://lkcamp.dev/about/
>> [2] https://lore.kernel.org/all/Zktnt7rjKryTh9-N@arch/
>> [3] https://elixir.bootlin.com/linux/latest/source/lib/list-test.c
>> [4] https://lore.kernel.org/all/20240720181025.work.002-kees@kernel.org/
>>
>> ---
>> Changes in v3:
>>      - Resolved checkpatch warnings:
>>          - Renamed tests for macros starting with 'for_each'
> 
> Shouldn't this a separate patch to make it easy to review?
> 
>>          - Removed link from commit message
>>      - Replaced hardcoded constants with ENTRIES_SIZE
> 
> Shouldn't this a separate patch to make it easy to review?
> 
>>      - Updated initialization of llist_node array
>>      - Fixed typos
>>      - Update Kconfig.debug message for llist_kunit
> 
> Are these changes to existing code or warnings on your added code?
>>
>> Changes in v2:
>>      - Add MODULE_DESCRIPTION()
>>      - Move the tests from lib/llist_kunit.c to lib/tests/llist_kunit.c
>>      - Change the license from "GPL v2" to "GPL"
>>
>> Artur Alves (1):
>>    lib/llist_kunit.c: add KUnit tests for llist
>>
>>   lib/Kconfig.debug       |  11 ++
>>   lib/tests/Makefile      |   1 +
>>   lib/tests/llist_kunit.c | 358 ++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 370 insertions(+)
>>   create mode 100644 lib/tests/llist_kunit.c
>>
> 
> You are combining lot of changes in one single patch. Each change as a 
> separate
> patch will help reviewers.
> 
> Adding new test should be a separate patch.
> 
> - renaming as a separate patch
> 
> thanks,
> -- Shuah

Hi, thanks for the reply!

I'm not sure if I understood your concerns ...

In this patch, I'm adding the entire test suite for the lock-less list 
data structure, which is the primary reason for its larger size. The 
changes in V2 and V3 were made in response to code review suggestions 
from previous iterations.

However, as a big patch I see how this cause an annoyance to review. I'm 
open to any suggestions on how I can reduce its size or make the review 
process more manageable.

Best regards,
- Artur