Now that we have the VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT macros,
update the instructions to stop recommending including .c files.
Signed-off-by: Arthur Grillo <[email protected]>
---
Changes in v2:
- Fix #if condition
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Documentation/dev-tools/kunit/usage.rst | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index c27e1646ecd9..f095c6bb76ff 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -671,19 +671,22 @@ Testing Static Functions
------------------------
If we do not want to expose functions or variables for testing, one option is to
-conditionally ``#include`` the test file at the end of your .c file. For
-example:
+conditionally export the used symbol.
.. code-block:: c
/* In my_file.c */
- static int do_interesting_thing();
+ VISIBLE_IF_KUNIT int do_interesting_thing();
+ EXPORT_SYMBOL_IF_KUNIT(do_interesting_thing);
- #ifdef CONFIG_MY_KUNIT_TEST
- #include "my_kunit_test.c"
+ /* In my_file.h */
+
+ #if IS_ENABLED(CONFIG_KUNIT)
+ int do_interesting_thing(void);
#endif
+
Injecting Test-Only Code
------------------------
---
base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
change-id: 20240108-kunit-doc-export-eec1f910ab67
Best regards,
--
Arthur Grillo <[email protected]>
On Tue, 9 Jan 2024 at 04:24, Arthur Grillo <[email protected]> wrote:
>
> Now that we have the VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT macros,
> update the instructions to stop recommending including .c files.
>
> Signed-off-by: Arthur Grillo <[email protected]>
> ---
> Changes in v2:
> - Fix #if condition
> - Link to v1: https://lore.kernel.org/r/[email protected]
> ---
Thanks very much: I think we definitely should be recommending
VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT more.
I do wonder, though, whether we should also keep the conditional
``#include`` example. There are some tests already using it, and it
can be more convenient than exporting lots of symbols in some cases. I
still think we should encourage the
VISIBLE_IF_KUNIT/EXPORT_SYMBOL_IF_KUNIT features more, but maybe we
leave the existing documentation there underneath. (e.g.
"Alternatively, we can conditionally…")
Otherwise, this looks good, and if people think that we should avoid
recommending the conditional-#include method (which _is_ ugly), then
I'm happy to accept this as-is.
Thoughts?
-- David
On 09/01/24 02:44, David Gow wrote:
> On Tue, 9 Jan 2024 at 04:24, Arthur Grillo <[email protected]> wrote:
>>
>> Now that we have the VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT macros,
>> update the instructions to stop recommending including .c files.
>>
>> Signed-off-by: Arthur Grillo <[email protected]>
>> ---
>> Changes in v2:
>> - Fix #if condition
>> - Link to v1: https://lore.kernel.org/r/[email protected]
>> ---
>
> Thanks very much: I think we definitely should be recommending
> VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT more.
>
> I do wonder, though, whether we should also keep the conditional
> ``#include`` example. There are some tests already using it, and it
> can be more convenient than exporting lots of symbols in some cases. I
> still think we should encourage the
> VISIBLE_IF_KUNIT/EXPORT_SYMBOL_IF_KUNIT features more, but maybe we
> leave the existing documentation there underneath. (e.g.
> "Alternatively, we can conditionally…")
I agree that, in some cases, the include way can be convenient. So, if
it's not discouraged/deprecated, I think it's better to keep the old
way.
I sent this patch because of a comment in a patch that I sent[1]. That
was when I discovered these macros and noticed the absence of
documentation on them.
[1]: https://lore.kernel.org/all/5z66ivuhfrzrnuzt6lwjfm5fuozxlgqsco3qb5rfzyf6mil5ms@2svqtlcncyjj/
~Arthur Grillo
>
> Otherwise, this looks good, and if people think that we should avoid
> recommending the conditional-#include method (which _is_ ugly), then
> I'm happy to accept this as-is.
>
> Thoughts?
>
> -- David