diff mbox series

[v2,3/5] kunit: string-stream-test: Make it a separate module

Message ID 20240618170331.264851-4-ivan.orlov0322@gmail.com
State New
Headers show
Series Reorganize string-stream and assert tests | expand

Commit Message

Ivan Orlov June 18, 2024, 5:03 p.m. UTC
Currently, the only way to build string-stream-test is by setting
CONFIG_KUNIT_TEST=y. However, CONFIG_KUNIT_TEST is a config option for
a different test (`kunit-test.c`).

Introduce a new Kconfig entry in order to be able to build the
string-stream-test test as a separate module. Import the KUnit namespace
in the test so we could have string-stream functions accessible.

Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
V1 -> V2:
- No changes

 lib/kunit/Kconfig              | 8 ++++++++
 lib/kunit/Makefile             | 2 +-
 lib/kunit/string-stream-test.c | 2 ++
 3 files changed, 11 insertions(+), 1 deletion(-)

Comments

Jeff Johnson June 19, 2024, 6:09 p.m. UTC | #1
On 6/18/24 10:03, Ivan Orlov wrote:
> Currently, the only way to build string-stream-test is by setting
> CONFIG_KUNIT_TEST=y. However, CONFIG_KUNIT_TEST is a config option for
> a different test (`kunit-test.c`).
> 
> Introduce a new Kconfig entry in order to be able to build the
> string-stream-test test as a separate module. Import the KUnit namespace
> in the test so we could have string-stream functions accessible.
> 
> Reviewed-by: David Gow <davidgow@google.com>
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> ---
> V1 -> V2:
> - No changes
> 
>   lib/kunit/Kconfig              | 8 ++++++++
>   lib/kunit/Makefile             | 2 +-
>   lib/kunit/string-stream-test.c | 2 ++
>   3 files changed, 11 insertions(+), 1 deletion(-)
> 
...

> diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
> index 7511442ea98f..d03cac934e04 100644
> --- a/lib/kunit/string-stream-test.c
> +++ b/lib/kunit/string-stream-test.c
> @@ -534,3 +534,5 @@ static struct kunit_suite string_stream_test_suite = {
>   	.init = string_stream_test_init,
>   };
>   kunit_test_suites(&string_stream_test_suite);
> +MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
> +MODULE_LICENSE("GPL");

missing MODULE_DESCRIPTION()
this will cause a warning with make W=1
Rae Moar June 21, 2024, 9:07 p.m. UTC | #2
On Tue, Jun 18, 2024 at 1:03 PM Ivan Orlov <ivan.orlov0322@gmail.com> wrote:
>
> Currently, the only way to build string-stream-test is by setting
> CONFIG_KUNIT_TEST=y. However, CONFIG_KUNIT_TEST is a config option for
> a different test (`kunit-test.c`).
>
> Introduce a new Kconfig entry in order to be able to build the
> string-stream-test test as a separate module. Import the KUnit namespace
> in the test so we could have string-stream functions accessible.
>
> Reviewed-by: David Gow <davidgow@google.com>
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>

Hello!

This is looking good to me other than the module description as noted
by Jeff. That could be a separate patch since the rest of the series
is looking good.

There is the checkpatch warning on the module description. But as
David mentioned, the description looks ok to me. If there is a new
version of this patch, it may be worth trying to get rid of the
warning by lengthening the description.

But I am happy with this patch as is.

Reviewed-by: Rae Moar <rmoar@google.com>

Thanks!
-Rae

> ---
> V1 -> V2:
> - No changes
>
>  lib/kunit/Kconfig              | 8 ++++++++
>  lib/kunit/Makefile             | 2 +-
>  lib/kunit/string-stream-test.c | 2 ++
>  3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> index 34d7242d526d..b0713c0f9265 100644
> --- a/lib/kunit/Kconfig
> +++ b/lib/kunit/Kconfig
> @@ -45,6 +45,14 @@ config KUNIT_TEST
>           purposes by developers interested in testing that KUnit works as
>           expected.
>
> +config KUNIT_STRING_STREAM_TEST
> +       tristate "KUnit test for string-stream" if !KUNIT_ALL_TESTS
> +       default KUNIT_ALL_TESTS
> +       help
> +         Enables the KUnit test for the string-stream (C++ stream style string
> +         builder used in KUnit for building messages). For the string-stream
> +         implementation, see lib/kunit/string-stream.c.
> +
>  config KUNIT_EXAMPLE_TEST
>         tristate "Example test for KUnit" if !KUNIT_ALL_TESTS
>         default KUNIT_ALL_TESTS
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index 30f6bbf04a4a..478beb536dc9 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -19,10 +19,10 @@ endif
>  obj-y +=                               hooks.o
>
>  obj-$(CONFIG_KUNIT_TEST) +=            kunit-test.o
> +obj-$(CONFIG_KUNIT_STRING_STREAM_TEST) += string-stream-test.o
>
>  # string-stream-test compiles built-in only.
>  ifeq ($(CONFIG_KUNIT_TEST),y)
> -obj-$(CONFIG_KUNIT_TEST) +=            string-stream-test.o
>  obj-$(CONFIG_KUNIT_TEST) +=            assert_test.o
>  endif
>
> diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
> index 7511442ea98f..d03cac934e04 100644
> --- a/lib/kunit/string-stream-test.c
> +++ b/lib/kunit/string-stream-test.c
> @@ -534,3 +534,5 @@ static struct kunit_suite string_stream_test_suite = {
>         .init = string_stream_test_init,
>  };
>  kunit_test_suites(&string_stream_test_suite);
> +MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
>
Ivan Orlov June 27, 2024, 8:51 p.m. UTC | #3
On 6/21/24 22:07, 'Rae Moar' via KUnit Development wrote:
> On Tue, Jun 18, 2024 at 1:03 PM Ivan Orlov <ivan.orlov0322@gmail.com> wrote:
>>
>> Currently, the only way to build string-stream-test is by setting
>> CONFIG_KUNIT_TEST=y. However, CONFIG_KUNIT_TEST is a config option for
>> a different test (`kunit-test.c`).
>>
>> Introduce a new Kconfig entry in order to be able to build the
>> string-stream-test test as a separate module. Import the KUnit namespace
>> in the test so we could have string-stream functions accessible.
>>
>> Reviewed-by: David Gow <davidgow@google.com>
>> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> 
> Hello!
> 
> This is looking good to me other than the module description as noted
> by Jeff. That could be a separate patch since the rest of the series
> is looking good.
> 
> There is the checkpatch warning on the module description. But as
> David mentioned, the description looks ok to me. If there is a new
> version of this patch, it may be worth trying to get rid of the
> warning by lengthening the description.
> 
> But I am happy with this patch as is.
> 
> Reviewed-by: Rae Moar <rmoar@google.com>
> 
> Thanks!
> -Rae

Hi Rae,

Thank you for the review. I believe I'm going to send the V3 and add the 
module description there, to make the whole series as good as possible 
before it gets merged :)

Thank you so much for reviewing the series once again.
diff mbox series

Patch

diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
index 34d7242d526d..b0713c0f9265 100644
--- a/lib/kunit/Kconfig
+++ b/lib/kunit/Kconfig
@@ -45,6 +45,14 @@  config KUNIT_TEST
 	  purposes by developers interested in testing that KUnit works as
 	  expected.
 
+config KUNIT_STRING_STREAM_TEST
+	tristate "KUnit test for string-stream" if !KUNIT_ALL_TESTS
+	default KUNIT_ALL_TESTS
+	help
+	  Enables the KUnit test for the string-stream (C++ stream style string
+	  builder used in KUnit for building messages). For the string-stream
+	  implementation, see lib/kunit/string-stream.c.
+
 config KUNIT_EXAMPLE_TEST
 	tristate "Example test for KUnit" if !KUNIT_ALL_TESTS
 	default KUNIT_ALL_TESTS
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 30f6bbf04a4a..478beb536dc9 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -19,10 +19,10 @@  endif
 obj-y +=				hooks.o
 
 obj-$(CONFIG_KUNIT_TEST) +=		kunit-test.o
+obj-$(CONFIG_KUNIT_STRING_STREAM_TEST) += string-stream-test.o
 
 # string-stream-test compiles built-in only.
 ifeq ($(CONFIG_KUNIT_TEST),y)
-obj-$(CONFIG_KUNIT_TEST) +=		string-stream-test.o
 obj-$(CONFIG_KUNIT_TEST) +=		assert_test.o
 endif
 
diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
index 7511442ea98f..d03cac934e04 100644
--- a/lib/kunit/string-stream-test.c
+++ b/lib/kunit/string-stream-test.c
@@ -534,3 +534,5 @@  static struct kunit_suite string_stream_test_suite = {
 	.init = string_stream_test_init,
 };
 kunit_test_suites(&string_stream_test_suite);
+MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
+MODULE_LICENSE("GPL");