| Message ID | 20240618170331.264851-4-ivan.orlov0322@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | Reorganize string-stream and assert tests | expand |
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
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 >
On 6/19/24 19:09, Jeff Johnson wrote: > 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 > Hi Jeff, Thank you for noticing this, I'm going to add the MODULE_DESCRIPTION in the V3 of this patch series.
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 --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");