| Message ID | 20240313150158.204455-2-ivan.orlov0322@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Move tests to the 'tests' directory | expand |
On Wed, Mar 13, 2024 at 8:32 PM Ivan Orlov <ivan.orlov0322@gmail.com> wrote: > > Move all of the SBIUnit-related code into the lib/sbi/tests directory. > Update 'Makefile' to index objects from the tests subdirectory. > > I don't think creating the full separate list of Makefile variables > (libsbitests-objs-path-y, libsbitests-object-mks, etc. as it is done for > libsbiutils) is necessary for the tests because: > > 1) `lib/sbi/tests/objects.mk` is already indexed into > 'libsbi-objects-mks' since the find expression for the libsbi-object-mks > variable looks for objects.mk files in the nested directories as well). > > 2) Tests are tightly coupled with the `lib/sbi/` sources, therefore it > may be reasonable to store the list of lib/sbi and lib/sbi/tests object > files together in the libsbi-objs-path-y variable. > > Additionally, update relative paths in the tests where necessary. > > Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com> > --- > Makefile | 2 ++ > lib/sbi/objects.mk | 6 ------ > lib/sbi/sbi_console.c | 2 +- > lib/sbi/tests/objects.mk | 6 ++++++ > lib/sbi/{ => tests}/sbi_bitmap_test.c | 0 > lib/sbi/{ => tests}/sbi_console_test.c | 0 > lib/sbi/{ => tests}/sbi_unit_test.c | 0 > lib/sbi/{ => tests}/sbi_unit_tests.carray | 0 > 8 files changed, 9 insertions(+), 7 deletions(-) > create mode 100644 lib/sbi/tests/objects.mk > rename lib/sbi/{ => tests}/sbi_bitmap_test.c (100%) > rename lib/sbi/{ => tests}/sbi_console_test.c (100%) > rename lib/sbi/{ => tests}/sbi_unit_test.c (100%) > rename lib/sbi/{ => tests}/sbi_unit_tests.carray (100%) > > diff --git a/Makefile b/Makefile > index 680c19a..eef321e 100644 > --- a/Makefile > +++ b/Makefile > @@ -247,6 +247,8 @@ include $(firmware-object-mks) > > # Setup list of objects > libsbi-objs-path-y=$(foreach obj,$(libsbi-objs-y),$(build_dir)/lib/sbi/$(obj)) > +# Index unit tests > +libsbi-objs-path-y+=$(foreach obj,$(libsbitests-objs-y),$(build_dir)/lib/sbi/tests/$(obj)) No need for changing top-level Makefile. > ifdef PLATFORM > libsbiutils-objs-path-y=$(foreach obj,$(libsbiutils-objs-y),$(platform_build_dir)/lib/utils/$(obj)) > platform-objs-path-y=$(foreach obj,$(platform-objs-y),$(platform_build_dir)/$(obj)) > diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk > index 2bed7f3..5d06d25 100644 > --- a/lib/sbi/objects.mk > +++ b/lib/sbi/objects.mk > @@ -11,12 +11,6 @@ libsbi-objs-y += riscv_asm.o > libsbi-objs-y += riscv_atomic.o > libsbi-objs-y += riscv_hardfp.o > libsbi-objs-y += riscv_locks.o > -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o > -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o > - > -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o > -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite > -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite > > libsbi-objs-y += sbi_ecall.o > libsbi-objs-y += sbi_ecall_exts.o > diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c > index d1229d0..8d1ad2e 100644 > --- a/lib/sbi/sbi_console.c > +++ b/lib/sbi/sbi_console.c > @@ -490,5 +490,5 @@ int sbi_console_init(struct sbi_scratch *scratch) > } > > #ifdef CONFIG_SBIUNIT > -#include "sbi_console_test.c" > +#include "tests/sbi_console_test.c" > #endif We can simply drop including "tests/sbi_console_test.c" by relaxing the check in sbi_console_set_device(). > diff --git a/lib/sbi/tests/objects.mk b/lib/sbi/tests/objects.mk > new file mode 100644 > index 0000000..0397172 > --- /dev/null > +++ b/lib/sbi/tests/objects.mk > @@ -0,0 +1,6 @@ > +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o > +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o > + > +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o We just need "tests/" prefix to above objects. Just like we do in various objects.mk under utils directory. > +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite > +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite > diff --git a/lib/sbi/sbi_bitmap_test.c b/lib/sbi/tests/sbi_bitmap_test.c > similarity index 100% > rename from lib/sbi/sbi_bitmap_test.c > rename to lib/sbi/tests/sbi_bitmap_test.c > diff --git a/lib/sbi/sbi_console_test.c b/lib/sbi/tests/sbi_console_test.c > similarity index 100% > rename from lib/sbi/sbi_console_test.c > rename to lib/sbi/tests/sbi_console_test.c > diff --git a/lib/sbi/sbi_unit_test.c b/lib/sbi/tests/sbi_unit_test.c > similarity index 100% > rename from lib/sbi/sbi_unit_test.c > rename to lib/sbi/tests/sbi_unit_test.c > diff --git a/lib/sbi/sbi_unit_tests.carray b/lib/sbi/tests/sbi_unit_tests.carray > similarity index 100% > rename from lib/sbi/sbi_unit_tests.carray > rename to lib/sbi/tests/sbi_unit_tests.carray > -- > 2.34.1 > I have taken care of the above minor issues at the time of merging this patch. Reviewed-by: Anup Patel <anup@brainfault.org> Applied this patch to the riscv/opensbi repo. Thanks, Anup
On 3/19/24 05:55, Anup Patel wrote: > On Wed, Mar 13, 2024 at 8:32 PM Ivan Orlov <ivan.orlov0322@gmail.com> wrote: >> >> Move all of the SBIUnit-related code into the lib/sbi/tests directory. >> Update 'Makefile' to index objects from the tests subdirectory. >> >> I don't think creating the full separate list of Makefile variables >> (libsbitests-objs-path-y, libsbitests-object-mks, etc. as it is done for >> libsbiutils) is necessary for the tests because: >> >> 1) `lib/sbi/tests/objects.mk` is already indexed into >> 'libsbi-objects-mks' since the find expression for the libsbi-object-mks >> variable looks for objects.mk files in the nested directories as well). >> >> 2) Tests are tightly coupled with the `lib/sbi/` sources, therefore it >> may be reasonable to store the list of lib/sbi and lib/sbi/tests object >> files together in the libsbi-objs-path-y variable. >> >> Additionally, update relative paths in the tests where necessary. >> >> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com> >> --- >> Makefile | 2 ++ >> lib/sbi/objects.mk | 6 ------ >> lib/sbi/sbi_console.c | 2 +- >> lib/sbi/tests/objects.mk | 6 ++++++ >> lib/sbi/{ => tests}/sbi_bitmap_test.c | 0 >> lib/sbi/{ => tests}/sbi_console_test.c | 0 >> lib/sbi/{ => tests}/sbi_unit_test.c | 0 >> lib/sbi/{ => tests}/sbi_unit_tests.carray | 0 >> 8 files changed, 9 insertions(+), 7 deletions(-) >> create mode 100644 lib/sbi/tests/objects.mk >> rename lib/sbi/{ => tests}/sbi_bitmap_test.c (100%) >> rename lib/sbi/{ => tests}/sbi_console_test.c (100%) >> rename lib/sbi/{ => tests}/sbi_unit_test.c (100%) >> rename lib/sbi/{ => tests}/sbi_unit_tests.carray (100%) >> >> diff --git a/Makefile b/Makefile >> index 680c19a..eef321e 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -247,6 +247,8 @@ include $(firmware-object-mks) >> >> # Setup list of objects >> libsbi-objs-path-y=$(foreach obj,$(libsbi-objs-y),$(build_dir)/lib/sbi/$(obj)) >> +# Index unit tests >> +libsbi-objs-path-y+=$(foreach obj,$(libsbitests-objs-y),$(build_dir)/lib/sbi/tests/$(obj)) > > No need for changing top-level Makefile. > >> ifdef PLATFORM >> libsbiutils-objs-path-y=$(foreach obj,$(libsbiutils-objs-y),$(platform_build_dir)/lib/utils/$(obj)) >> platform-objs-path-y=$(foreach obj,$(platform-objs-y),$(platform_build_dir)/$(obj)) >> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk >> index 2bed7f3..5d06d25 100644 >> --- a/lib/sbi/objects.mk >> +++ b/lib/sbi/objects.mk >> @@ -11,12 +11,6 @@ libsbi-objs-y += riscv_asm.o >> libsbi-objs-y += riscv_atomic.o >> libsbi-objs-y += riscv_hardfp.o >> libsbi-objs-y += riscv_locks.o >> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o >> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o >> - >> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o >> -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite >> -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite >> >> libsbi-objs-y += sbi_ecall.o >> libsbi-objs-y += sbi_ecall_exts.o >> diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c >> index d1229d0..8d1ad2e 100644 >> --- a/lib/sbi/sbi_console.c >> +++ b/lib/sbi/sbi_console.c >> @@ -490,5 +490,5 @@ int sbi_console_init(struct sbi_scratch *scratch) >> } >> >> #ifdef CONFIG_SBIUNIT >> -#include "sbi_console_test.c" >> +#include "tests/sbi_console_test.c" >> #endif > > We can simply drop including "tests/sbi_console_test.c" by > relaxing the check in sbi_console_set_device(). > >> diff --git a/lib/sbi/tests/objects.mk b/lib/sbi/tests/objects.mk >> new file mode 100644 >> index 0000000..0397172 >> --- /dev/null >> +++ b/lib/sbi/tests/objects.mk >> @@ -0,0 +1,6 @@ >> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o >> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o >> + >> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o > > We just need "tests/" prefix to above objects. Just like we do > in various objects.mk under utils directory. > >> +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite >> +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite >> diff --git a/lib/sbi/sbi_bitmap_test.c b/lib/sbi/tests/sbi_bitmap_test.c >> similarity index 100% >> rename from lib/sbi/sbi_bitmap_test.c >> rename to lib/sbi/tests/sbi_bitmap_test.c >> diff --git a/lib/sbi/sbi_console_test.c b/lib/sbi/tests/sbi_console_test.c >> similarity index 100% >> rename from lib/sbi/sbi_console_test.c >> rename to lib/sbi/tests/sbi_console_test.c >> diff --git a/lib/sbi/sbi_unit_test.c b/lib/sbi/tests/sbi_unit_test.c >> similarity index 100% >> rename from lib/sbi/sbi_unit_test.c >> rename to lib/sbi/tests/sbi_unit_test.c >> diff --git a/lib/sbi/sbi_unit_tests.carray b/lib/sbi/tests/sbi_unit_tests.carray >> similarity index 100% >> rename from lib/sbi/sbi_unit_tests.carray >> rename to lib/sbi/tests/sbi_unit_tests.carray >> -- >> 2.34.1 >> > > I have taken care of the above minor issues at the time of merging > this patch. > > Reviewed-by: Anup Patel <anup@brainfault.org> > > Applied this patch to the riscv/opensbi repo. > Hi Anup, Thank you so much for the review and for fixing these issues. Now the documentation should be updated as well, correspondingly with the updates you made: currently, 'writing_tests.md' doc specifies the wrong Makefile variable name for the tests (libsbitests-... instead of libsbi-...). I'll fix it and send the patch today.
On Tue, Mar 19, 2024 at 9:14 PM Ivan Orlov <ivan.orlov0322@gmail.com> wrote: > > On 3/19/24 05:55, Anup Patel wrote: > > On Wed, Mar 13, 2024 at 8:32 PM Ivan Orlov <ivan.orlov0322@gmail.com> wrote: > >> > >> Move all of the SBIUnit-related code into the lib/sbi/tests directory. > >> Update 'Makefile' to index objects from the tests subdirectory. > >> > >> I don't think creating the full separate list of Makefile variables > >> (libsbitests-objs-path-y, libsbitests-object-mks, etc. as it is done for > >> libsbiutils) is necessary for the tests because: > >> > >> 1) `lib/sbi/tests/objects.mk` is already indexed into > >> 'libsbi-objects-mks' since the find expression for the libsbi-object-mks > >> variable looks for objects.mk files in the nested directories as well). > >> > >> 2) Tests are tightly coupled with the `lib/sbi/` sources, therefore it > >> may be reasonable to store the list of lib/sbi and lib/sbi/tests object > >> files together in the libsbi-objs-path-y variable. > >> > >> Additionally, update relative paths in the tests where necessary. > >> > >> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com> > >> --- > >> Makefile | 2 ++ > >> lib/sbi/objects.mk | 6 ------ > >> lib/sbi/sbi_console.c | 2 +- > >> lib/sbi/tests/objects.mk | 6 ++++++ > >> lib/sbi/{ => tests}/sbi_bitmap_test.c | 0 > >> lib/sbi/{ => tests}/sbi_console_test.c | 0 > >> lib/sbi/{ => tests}/sbi_unit_test.c | 0 > >> lib/sbi/{ => tests}/sbi_unit_tests.carray | 0 > >> 8 files changed, 9 insertions(+), 7 deletions(-) > >> create mode 100644 lib/sbi/tests/objects.mk > >> rename lib/sbi/{ => tests}/sbi_bitmap_test.c (100%) > >> rename lib/sbi/{ => tests}/sbi_console_test.c (100%) > >> rename lib/sbi/{ => tests}/sbi_unit_test.c (100%) > >> rename lib/sbi/{ => tests}/sbi_unit_tests.carray (100%) > >> > >> diff --git a/Makefile b/Makefile > >> index 680c19a..eef321e 100644 > >> --- a/Makefile > >> +++ b/Makefile > >> @@ -247,6 +247,8 @@ include $(firmware-object-mks) > >> > >> # Setup list of objects > >> libsbi-objs-path-y=$(foreach obj,$(libsbi-objs-y),$(build_dir)/lib/sbi/$(obj)) > >> +# Index unit tests > >> +libsbi-objs-path-y+=$(foreach obj,$(libsbitests-objs-y),$(build_dir)/lib/sbi/tests/$(obj)) > > > > No need for changing top-level Makefile. > > > >> ifdef PLATFORM > >> libsbiutils-objs-path-y=$(foreach obj,$(libsbiutils-objs-y),$(platform_build_dir)/lib/utils/$(obj)) > >> platform-objs-path-y=$(foreach obj,$(platform-objs-y),$(platform_build_dir)/$(obj)) > >> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk > >> index 2bed7f3..5d06d25 100644 > >> --- a/lib/sbi/objects.mk > >> +++ b/lib/sbi/objects.mk > >> @@ -11,12 +11,6 @@ libsbi-objs-y += riscv_asm.o > >> libsbi-objs-y += riscv_atomic.o > >> libsbi-objs-y += riscv_hardfp.o > >> libsbi-objs-y += riscv_locks.o > >> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o > >> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o > >> - > >> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o > >> -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite > >> -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite > >> > >> libsbi-objs-y += sbi_ecall.o > >> libsbi-objs-y += sbi_ecall_exts.o > >> diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c > >> index d1229d0..8d1ad2e 100644 > >> --- a/lib/sbi/sbi_console.c > >> +++ b/lib/sbi/sbi_console.c > >> @@ -490,5 +490,5 @@ int sbi_console_init(struct sbi_scratch *scratch) > >> } > >> > >> #ifdef CONFIG_SBIUNIT > >> -#include "sbi_console_test.c" > >> +#include "tests/sbi_console_test.c" > >> #endif > > > > We can simply drop including "tests/sbi_console_test.c" by > > relaxing the check in sbi_console_set_device(). > > > >> diff --git a/lib/sbi/tests/objects.mk b/lib/sbi/tests/objects.mk > >> new file mode 100644 > >> index 0000000..0397172 > >> --- /dev/null > >> +++ b/lib/sbi/tests/objects.mk > >> @@ -0,0 +1,6 @@ > >> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o > >> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o > >> + > >> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o > > > > We just need "tests/" prefix to above objects. Just like we do > > in various objects.mk under utils directory. > > > >> +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite > >> +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite > >> diff --git a/lib/sbi/sbi_bitmap_test.c b/lib/sbi/tests/sbi_bitmap_test.c > >> similarity index 100% > >> rename from lib/sbi/sbi_bitmap_test.c > >> rename to lib/sbi/tests/sbi_bitmap_test.c > >> diff --git a/lib/sbi/sbi_console_test.c b/lib/sbi/tests/sbi_console_test.c > >> similarity index 100% > >> rename from lib/sbi/sbi_console_test.c > >> rename to lib/sbi/tests/sbi_console_test.c > >> diff --git a/lib/sbi/sbi_unit_test.c b/lib/sbi/tests/sbi_unit_test.c > >> similarity index 100% > >> rename from lib/sbi/sbi_unit_test.c > >> rename to lib/sbi/tests/sbi_unit_test.c > >> diff --git a/lib/sbi/sbi_unit_tests.carray b/lib/sbi/tests/sbi_unit_tests.carray > >> similarity index 100% > >> rename from lib/sbi/sbi_unit_tests.carray > >> rename to lib/sbi/tests/sbi_unit_tests.carray > >> -- > >> 2.34.1 > >> > > > > I have taken care of the above minor issues at the time of merging > > this patch. > > > > Reviewed-by: Anup Patel <anup@brainfault.org> > > > > Applied this patch to the riscv/opensbi repo. > > > > Hi Anup, > > Thank you so much for the review and for fixing these issues. > > Now the documentation should be updated as well, correspondingly with > the updates you made: currently, 'writing_tests.md' doc specifies the > wrong Makefile variable name for the tests (libsbitests-... instead of > libsbi-...). I'll fix it and send the patch today. Ahh, my bad. I will wait for your patch. Thanks, Anup > > -- > Kind regards, > Ivan Orlov > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
On 3/19/24 15:47, Anup Patel wrote: > On Tue, Mar 19, 2024 at 9:14 PM Ivan Orlov <ivan.orlov0322@gmail.com> wrote: >> >> On 3/19/24 05:55, Anup Patel wrote: >>> On Wed, Mar 13, 2024 at 8:32 PM Ivan Orlov <ivan.orlov0322@gmail.com> wrote: >>>> >>>> Move all of the SBIUnit-related code into the lib/sbi/tests directory. >>>> Update 'Makefile' to index objects from the tests subdirectory. >>>> >>>> I don't think creating the full separate list of Makefile variables >>>> (libsbitests-objs-path-y, libsbitests-object-mks, etc. as it is done for >>>> libsbiutils) is necessary for the tests because: >>>> >>>> 1) `lib/sbi/tests/objects.mk` is already indexed into >>>> 'libsbi-objects-mks' since the find expression for the libsbi-object-mks >>>> variable looks for objects.mk files in the nested directories as well). >>>> >>>> 2) Tests are tightly coupled with the `lib/sbi/` sources, therefore it >>>> may be reasonable to store the list of lib/sbi and lib/sbi/tests object >>>> files together in the libsbi-objs-path-y variable. >>>> >>>> Additionally, update relative paths in the tests where necessary. >>>> >>>> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com> >>>> --- >>>> Makefile | 2 ++ >>>> lib/sbi/objects.mk | 6 ------ >>>> lib/sbi/sbi_console.c | 2 +- >>>> lib/sbi/tests/objects.mk | 6 ++++++ >>>> lib/sbi/{ => tests}/sbi_bitmap_test.c | 0 >>>> lib/sbi/{ => tests}/sbi_console_test.c | 0 >>>> lib/sbi/{ => tests}/sbi_unit_test.c | 0 >>>> lib/sbi/{ => tests}/sbi_unit_tests.carray | 0 >>>> 8 files changed, 9 insertions(+), 7 deletions(-) >>>> create mode 100644 lib/sbi/tests/objects.mk >>>> rename lib/sbi/{ => tests}/sbi_bitmap_test.c (100%) >>>> rename lib/sbi/{ => tests}/sbi_console_test.c (100%) >>>> rename lib/sbi/{ => tests}/sbi_unit_test.c (100%) >>>> rename lib/sbi/{ => tests}/sbi_unit_tests.carray (100%) >>>> >>>> diff --git a/Makefile b/Makefile >>>> index 680c19a..eef321e 100644 >>>> --- a/Makefile >>>> +++ b/Makefile >>>> @@ -247,6 +247,8 @@ include $(firmware-object-mks) >>>> >>>> # Setup list of objects >>>> libsbi-objs-path-y=$(foreach obj,$(libsbi-objs-y),$(build_dir)/lib/sbi/$(obj)) >>>> +# Index unit tests >>>> +libsbi-objs-path-y+=$(foreach obj,$(libsbitests-objs-y),$(build_dir)/lib/sbi/tests/$(obj)) >>> >>> No need for changing top-level Makefile. >>> >>>> ifdef PLATFORM >>>> libsbiutils-objs-path-y=$(foreach obj,$(libsbiutils-objs-y),$(platform_build_dir)/lib/utils/$(obj)) >>>> platform-objs-path-y=$(foreach obj,$(platform-objs-y),$(platform_build_dir)/$(obj)) >>>> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk >>>> index 2bed7f3..5d06d25 100644 >>>> --- a/lib/sbi/objects.mk >>>> +++ b/lib/sbi/objects.mk >>>> @@ -11,12 +11,6 @@ libsbi-objs-y += riscv_asm.o >>>> libsbi-objs-y += riscv_atomic.o >>>> libsbi-objs-y += riscv_hardfp.o >>>> libsbi-objs-y += riscv_locks.o >>>> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o >>>> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o >>>> - >>>> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o >>>> -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite >>>> -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite >>>> >>>> libsbi-objs-y += sbi_ecall.o >>>> libsbi-objs-y += sbi_ecall_exts.o >>>> diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c >>>> index d1229d0..8d1ad2e 100644 >>>> --- a/lib/sbi/sbi_console.c >>>> +++ b/lib/sbi/sbi_console.c >>>> @@ -490,5 +490,5 @@ int sbi_console_init(struct sbi_scratch *scratch) >>>> } >>>> >>>> #ifdef CONFIG_SBIUNIT >>>> -#include "sbi_console_test.c" >>>> +#include "tests/sbi_console_test.c" >>>> #endif >>> >>> We can simply drop including "tests/sbi_console_test.c" by >>> relaxing the check in sbi_console_set_device(). >>> >>>> diff --git a/lib/sbi/tests/objects.mk b/lib/sbi/tests/objects.mk >>>> new file mode 100644 >>>> index 0000000..0397172 >>>> --- /dev/null >>>> +++ b/lib/sbi/tests/objects.mk >>>> @@ -0,0 +1,6 @@ >>>> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o >>>> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o >>>> + >>>> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o >>> >>> We just need "tests/" prefix to above objects. Just like we do >>> in various objects.mk under utils directory. >>> >>>> +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite >>>> +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite >>>> diff --git a/lib/sbi/sbi_bitmap_test.c b/lib/sbi/tests/sbi_bitmap_test.c >>>> similarity index 100% >>>> rename from lib/sbi/sbi_bitmap_test.c >>>> rename to lib/sbi/tests/sbi_bitmap_test.c >>>> diff --git a/lib/sbi/sbi_console_test.c b/lib/sbi/tests/sbi_console_test.c >>>> similarity index 100% >>>> rename from lib/sbi/sbi_console_test.c >>>> rename to lib/sbi/tests/sbi_console_test.c >>>> diff --git a/lib/sbi/sbi_unit_test.c b/lib/sbi/tests/sbi_unit_test.c >>>> similarity index 100% >>>> rename from lib/sbi/sbi_unit_test.c >>>> rename to lib/sbi/tests/sbi_unit_test.c >>>> diff --git a/lib/sbi/sbi_unit_tests.carray b/lib/sbi/tests/sbi_unit_tests.carray >>>> similarity index 100% >>>> rename from lib/sbi/sbi_unit_tests.carray >>>> rename to lib/sbi/tests/sbi_unit_tests.carray >>>> -- >>>> 2.34.1 >>>> >>> >>> I have taken care of the above minor issues at the time of merging >>> this patch. >>> >>> Reviewed-by: Anup Patel <anup@brainfault.org> >>> >>> Applied this patch to the riscv/opensbi repo. >>> >> >> Hi Anup, >> >> Thank you so much for the review and for fixing these issues. >> >> Now the documentation should be updated as well, correspondingly with >> the updates you made: currently, 'writing_tests.md' doc specifies the >> wrong Makefile variable name for the tests (libsbitests-... instead of >> libsbi-...). I'll fix it and send the patch today. > > Ahh, my bad. I will wait for your patch. > No worries, give me 15 minutes :)
On 3/19/24 15:52, Ivan Orlov wrote: > On 3/19/24 15:47, Anup Patel wrote: >> On Tue, Mar 19, 2024 at 9:14 PM Ivan Orlov <ivan.orlov0322@gmail.com> >> wrote: >>> >>> On 3/19/24 05:55, Anup Patel wrote: >>>> On Wed, Mar 13, 2024 at 8:32 PM Ivan Orlov >>>> <ivan.orlov0322@gmail.com> wrote: >>>>> >>>>> Move all of the SBIUnit-related code into the lib/sbi/tests directory. >>>>> Update 'Makefile' to index objects from the tests subdirectory. >>>>> >>>>> I don't think creating the full separate list of Makefile variables >>>>> (libsbitests-objs-path-y, libsbitests-object-mks, etc. as it is >>>>> done for >>>>> libsbiutils) is necessary for the tests because: >>>>> >>>>> 1) `lib/sbi/tests/objects.mk` is already indexed into >>>>> 'libsbi-objects-mks' since the find expression for the >>>>> libsbi-object-mks >>>>> variable looks for objects.mk files in the nested directories as >>>>> well). >>>>> >>>>> 2) Tests are tightly coupled with the `lib/sbi/` sources, therefore it >>>>> may be reasonable to store the list of lib/sbi and lib/sbi/tests >>>>> object >>>>> files together in the libsbi-objs-path-y variable. >>>>> >>>>> Additionally, update relative paths in the tests where necessary. >>>>> >>>>> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com> >>>>> --- >>>>> Makefile | 2 ++ >>>>> lib/sbi/objects.mk | 6 ------ >>>>> lib/sbi/sbi_console.c | 2 +- >>>>> lib/sbi/tests/objects.mk | 6 ++++++ >>>>> lib/sbi/{ => tests}/sbi_bitmap_test.c | 0 >>>>> lib/sbi/{ => tests}/sbi_console_test.c | 0 >>>>> lib/sbi/{ => tests}/sbi_unit_test.c | 0 >>>>> lib/sbi/{ => tests}/sbi_unit_tests.carray | 0 >>>>> 8 files changed, 9 insertions(+), 7 deletions(-) >>>>> create mode 100644 lib/sbi/tests/objects.mk >>>>> rename lib/sbi/{ => tests}/sbi_bitmap_test.c (100%) >>>>> rename lib/sbi/{ => tests}/sbi_console_test.c (100%) >>>>> rename lib/sbi/{ => tests}/sbi_unit_test.c (100%) >>>>> rename lib/sbi/{ => tests}/sbi_unit_tests.carray (100%) >>>>> >>>>> diff --git a/Makefile b/Makefile >>>>> index 680c19a..eef321e 100644 >>>>> --- a/Makefile >>>>> +++ b/Makefile >>>>> @@ -247,6 +247,8 @@ include $(firmware-object-mks) >>>>> >>>>> # Setup list of objects >>>>> libsbi-objs-path-y=$(foreach >>>>> obj,$(libsbi-objs-y),$(build_dir)/lib/sbi/$(obj)) >>>>> +# Index unit tests >>>>> +libsbi-objs-path-y+=$(foreach >>>>> obj,$(libsbitests-objs-y),$(build_dir)/lib/sbi/tests/$(obj)) >>>> >>>> No need for changing top-level Makefile. >>>> >>>>> ifdef PLATFORM >>>>> libsbiutils-objs-path-y=$(foreach >>>>> obj,$(libsbiutils-objs-y),$(platform_build_dir)/lib/utils/$(obj)) >>>>> platform-objs-path-y=$(foreach >>>>> obj,$(platform-objs-y),$(platform_build_dir)/$(obj)) >>>>> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk >>>>> index 2bed7f3..5d06d25 100644 >>>>> --- a/lib/sbi/objects.mk >>>>> +++ b/lib/sbi/objects.mk >>>>> @@ -11,12 +11,6 @@ libsbi-objs-y += riscv_asm.o >>>>> libsbi-objs-y += riscv_atomic.o >>>>> libsbi-objs-y += riscv_hardfp.o >>>>> libsbi-objs-y += riscv_locks.o >>>>> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o >>>>> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o >>>>> - >>>>> -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o >>>>> -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite >>>>> -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite >>>>> >>>>> libsbi-objs-y += sbi_ecall.o >>>>> libsbi-objs-y += sbi_ecall_exts.o >>>>> diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c >>>>> index d1229d0..8d1ad2e 100644 >>>>> --- a/lib/sbi/sbi_console.c >>>>> +++ b/lib/sbi/sbi_console.c >>>>> @@ -490,5 +490,5 @@ int sbi_console_init(struct sbi_scratch *scratch) >>>>> } >>>>> >>>>> #ifdef CONFIG_SBIUNIT >>>>> -#include "sbi_console_test.c" >>>>> +#include "tests/sbi_console_test.c" >>>>> #endif >>>> >>>> We can simply drop including "tests/sbi_console_test.c" by >>>> relaxing the check in sbi_console_set_device(). >>>> >>>>> diff --git a/lib/sbi/tests/objects.mk b/lib/sbi/tests/objects.mk >>>>> new file mode 100644 >>>>> index 0000000..0397172 >>>>> --- /dev/null >>>>> +++ b/lib/sbi/tests/objects.mk >>>>> @@ -0,0 +1,6 @@ >>>>> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o >>>>> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o >>>>> + >>>>> +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o >>>> >>>> We just need "tests/" prefix to above objects. Just like we do >>>> in various objects.mk under utils directory. >>>> >>>>> +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite >>>>> +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite >>>>> diff --git a/lib/sbi/sbi_bitmap_test.c >>>>> b/lib/sbi/tests/sbi_bitmap_test.c >>>>> similarity index 100% >>>>> rename from lib/sbi/sbi_bitmap_test.c >>>>> rename to lib/sbi/tests/sbi_bitmap_test.c >>>>> diff --git a/lib/sbi/sbi_console_test.c >>>>> b/lib/sbi/tests/sbi_console_test.c >>>>> similarity index 100% >>>>> rename from lib/sbi/sbi_console_test.c >>>>> rename to lib/sbi/tests/sbi_console_test.c >>>>> diff --git a/lib/sbi/sbi_unit_test.c b/lib/sbi/tests/sbi_unit_test.c >>>>> similarity index 100% >>>>> rename from lib/sbi/sbi_unit_test.c >>>>> rename to lib/sbi/tests/sbi_unit_test.c >>>>> diff --git a/lib/sbi/sbi_unit_tests.carray >>>>> b/lib/sbi/tests/sbi_unit_tests.carray >>>>> similarity index 100% >>>>> rename from lib/sbi/sbi_unit_tests.carray >>>>> rename to lib/sbi/tests/sbi_unit_tests.carray >>>>> -- >>>>> 2.34.1 >>>>> >>>> >>>> I have taken care of the above minor issues at the time of merging >>>> this patch. >>>> >>>> Reviewed-by: Anup Patel <anup@brainfault.org> >>>> >>>> Applied this patch to the riscv/opensbi repo. >>>> >>> >>> Hi Anup, >>> >>> Thank you so much for the review and for fixing these issues. >>> >>> Now the documentation should be updated as well, correspondingly with >>> the updates you made: currently, 'writing_tests.md' doc specifies the >>> wrong Makefile variable name for the tests (libsbitests-... instead of >>> libsbi-...). I'll fix it and send the patch today. >> >> Ahh, my bad. I will wait for your patch. >> > > No worries, give me 15 minutes :) > Done
diff --git a/Makefile b/Makefile index 680c19a..eef321e 100644 --- a/Makefile +++ b/Makefile @@ -247,6 +247,8 @@ include $(firmware-object-mks) # Setup list of objects libsbi-objs-path-y=$(foreach obj,$(libsbi-objs-y),$(build_dir)/lib/sbi/$(obj)) +# Index unit tests +libsbi-objs-path-y+=$(foreach obj,$(libsbitests-objs-y),$(build_dir)/lib/sbi/tests/$(obj)) ifdef PLATFORM libsbiutils-objs-path-y=$(foreach obj,$(libsbiutils-objs-y),$(platform_build_dir)/lib/utils/$(obj)) platform-objs-path-y=$(foreach obj,$(platform-objs-y),$(platform_build_dir)/$(obj)) diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk index 2bed7f3..5d06d25 100644 --- a/lib/sbi/objects.mk +++ b/lib/sbi/objects.mk @@ -11,12 +11,6 @@ libsbi-objs-y += riscv_asm.o libsbi-objs-y += riscv_atomic.o libsbi-objs-y += riscv_hardfp.o libsbi-objs-y += riscv_locks.o -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o - -libsbi-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite -carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite libsbi-objs-y += sbi_ecall.o libsbi-objs-y += sbi_ecall_exts.o diff --git a/lib/sbi/sbi_console.c b/lib/sbi/sbi_console.c index d1229d0..8d1ad2e 100644 --- a/lib/sbi/sbi_console.c +++ b/lib/sbi/sbi_console.c @@ -490,5 +490,5 @@ int sbi_console_init(struct sbi_scratch *scratch) } #ifdef CONFIG_SBIUNIT -#include "sbi_console_test.c" +#include "tests/sbi_console_test.c" #endif diff --git a/lib/sbi/tests/objects.mk b/lib/sbi/tests/objects.mk new file mode 100644 index 0000000..0397172 --- /dev/null +++ b/lib/sbi/tests/objects.mk @@ -0,0 +1,6 @@ +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_test.o +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_unit_tests.o + +libsbitests-objs-$(CONFIG_SBIUNIT) += sbi_bitmap_test.o +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += bitmap_test_suite +carray-sbi_unit_tests-$(CONFIG_SBIUNIT) += console_test_suite diff --git a/lib/sbi/sbi_bitmap_test.c b/lib/sbi/tests/sbi_bitmap_test.c similarity index 100% rename from lib/sbi/sbi_bitmap_test.c rename to lib/sbi/tests/sbi_bitmap_test.c diff --git a/lib/sbi/sbi_console_test.c b/lib/sbi/tests/sbi_console_test.c similarity index 100% rename from lib/sbi/sbi_console_test.c rename to lib/sbi/tests/sbi_console_test.c diff --git a/lib/sbi/sbi_unit_test.c b/lib/sbi/tests/sbi_unit_test.c similarity index 100% rename from lib/sbi/sbi_unit_test.c rename to lib/sbi/tests/sbi_unit_test.c diff --git a/lib/sbi/sbi_unit_tests.carray b/lib/sbi/tests/sbi_unit_tests.carray similarity index 100% rename from lib/sbi/sbi_unit_tests.carray rename to lib/sbi/tests/sbi_unit_tests.carray
Move all of the SBIUnit-related code into the lib/sbi/tests directory. Update 'Makefile' to index objects from the tests subdirectory. I don't think creating the full separate list of Makefile variables (libsbitests-objs-path-y, libsbitests-object-mks, etc. as it is done for libsbiutils) is necessary for the tests because: 1) `lib/sbi/tests/objects.mk` is already indexed into 'libsbi-objects-mks' since the find expression for the libsbi-object-mks variable looks for objects.mk files in the nested directories as well). 2) Tests are tightly coupled with the `lib/sbi/` sources, therefore it may be reasonable to store the list of lib/sbi and lib/sbi/tests object files together in the libsbi-objs-path-y variable. Additionally, update relative paths in the tests where necessary. Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com> --- Makefile | 2 ++ lib/sbi/objects.mk | 6 ------ lib/sbi/sbi_console.c | 2 +- lib/sbi/tests/objects.mk | 6 ++++++ lib/sbi/{ => tests}/sbi_bitmap_test.c | 0 lib/sbi/{ => tests}/sbi_console_test.c | 0 lib/sbi/{ => tests}/sbi_unit_test.c | 0 lib/sbi/{ => tests}/sbi_unit_tests.carray | 0 8 files changed, 9 insertions(+), 7 deletions(-) create mode 100644 lib/sbi/tests/objects.mk rename lib/sbi/{ => tests}/sbi_bitmap_test.c (100%) rename lib/sbi/{ => tests}/sbi_console_test.c (100%) rename lib/sbi/{ => tests}/sbi_unit_test.c (100%) rename lib/sbi/{ => tests}/sbi_unit_tests.carray (100%)