| Message ID | 20240108-kunit-doc-export-v2-1-8f2dd3395fed@riseup.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] Documentation: KUnit: Update the instructions on how to test static functions | expand |
On Tue, 9 Jan 2024 at 04:24, Arthur Grillo <arthurgrillo@riseup.net> 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 <arthurgrillo@riseup.net> > --- > Changes in v2: > - Fix #if condition > - Link to v1: https://lore.kernel.org/r/20240108-kunit-doc-export-v1-1-119368df0d96@riseup.net > --- 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 <arthurgrillo@riseup.net> 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 <arthurgrillo@riseup.net> >> --- >> Changes in v2: >> - Fix #if condition >> - Link to v1: https://lore.kernel.org/r/20240108-kunit-doc-export-v1-1-119368df0d96@riseup.net >> --- > > 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
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 ------------------------
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 <arthurgrillo@riseup.net> --- Changes in v2: - Fix #if condition - Link to v1: https://lore.kernel.org/r/20240108-kunit-doc-export-v1-1-119368df0d96@riseup.net --- Documentation/dev-tools/kunit/usage.rst | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) --- base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f change-id: 20240108-kunit-doc-export-eec1f910ab67 Best regards,