diff mbox series

[v3,4/4] lib: tests: Add sbi_console test

Message ID 20240301160046.267814-5-ivan.orlov0322@gmail.com
State Superseded
Headers show
Series SBIUnit: cover OpenSBI with tests | expand

Commit Message

Ivan Orlov March 1, 2024, 4 p.m. UTC 
Add the test suite covering some of the functions from
lib/sbi/sbi_console.c: putc, puts and printf. The test covers a variety
of format specifiers for printf and different strings and characters for
putc and puts.

In order to do that, the test "mocks" the sbi_console_device structure
by setting the 'console_dev' variable to the virtual console.

Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
V1 -> V2:
- Rewrite using the carray functionality
- Replace CONSOLE_DO and CONSOLE_DO_RET macros with two inline
functions: one of them "mocks" the default console device, and
the second one restores the old console device.
- Fix codestyle issues (comments, etc.)
- Remove incorrect 'puts' test
- Use updated SBIUNIT_ASSERT_STREQ API
V2 -> V3:
- Remove unused include statement
- Rename "new_dev" => "test_console_dev"
- Use SBIUNIT_END_CASE macro in the test cases list instead of '{}'

 lib/sbi/objects.mk         |   1 +
 lib/sbi/sbi_console.c      |   4 ++
 lib/sbi/sbi_console_test.c | 101 +++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+)
 create mode 100644 lib/sbi/sbi_console_test.c

Comments

Andrew Jones March 4, 2024, 1:44 p.m. UTC | #1 
On Fri, Mar 01, 2024 at 04:00:45PM +0000, Ivan Orlov wrote:
> Add the test suite covering some of the functions from
> lib/sbi/sbi_console.c: putc, puts and printf. The test covers a variety
> of format specifiers for printf and different strings and characters for
> putc and puts.
> 
> In order to do that, the test "mocks" the sbi_console_device structure
> by setting the 'console_dev' variable to the virtual console.
> 
> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> ---
> V1 -> V2:
> - Rewrite using the carray functionality
> - Replace CONSOLE_DO and CONSOLE_DO_RET macros with two inline
> functions: one of them "mocks" the default console device, and
> the second one restores the old console device.
> - Fix codestyle issues (comments, etc.)
> - Remove incorrect 'puts' test
> - Use updated SBIUNIT_ASSERT_STREQ API
> V2 -> V3:
> - Remove unused include statement
> - Rename "new_dev" => "test_console_dev"
> - Use SBIUNIT_END_CASE macro in the test cases list instead of '{}'
> 
>  lib/sbi/objects.mk         |   1 +
>  lib/sbi/sbi_console.c      |   4 ++
>  lib/sbi/sbi_console_test.c | 101 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 106 insertions(+)
>  create mode 100644 lib/sbi/sbi_console_test.c
> 
> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> index b4c273f..9d065fa 100644
> --- a/lib/sbi/objects.mk
> +++ b/lib/sbi/objects.mk
> @@ -16,6 +16,7 @@ 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 ab09a5c..d1229d0 100644
> --- a/lib/sbi/sbi_console.c
> +++ b/lib/sbi/sbi_console.c
> @@ -488,3 +488,7 @@ int sbi_console_init(struct sbi_scratch *scratch)
>  
>  	return rc;
>  }
> +
> +#ifdef CONFIG_SBIUNIT
> +#include "sbi_console_test.c"
> +#endif
> diff --git a/lib/sbi/sbi_console_test.c b/lib/sbi/sbi_console_test.c
> new file mode 100644
> index 0000000..734a68c
> --- /dev/null
> +++ b/lib/sbi/sbi_console_test.c
> @@ -0,0 +1,101 @@
> +/*
> + * SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Author: Ivan Orlov <ivan.orlov0322@gmail.com>
> + */
> +#include <sbi/sbi_unit_test.h>
> +
> +#define TEST_CONSOLE_BUF_LEN 1024
> +
> +static const struct sbi_console_device *old_dev;
> +static char test_console_buf[TEST_CONSOLE_BUF_LEN];
> +static u32 test_console_buf_pos;
> +
> +static void test_console_putc(char c)
> +{
> +	test_console_buf[test_console_buf_pos] = c;
> +	test_console_buf_pos = (test_console_buf_pos + 1) % TEST_CONSOLE_BUF_LEN;
> +}
> +
> +static void clear_test_console_buf(void)
> +{
> +	test_console_buf_pos = 0;
> +	test_console_buf[0] = '\0';
> +}
> +
> +static const struct sbi_console_device test_console_dev = {
> +	.name = "Test console device",
> +	.console_putc = test_console_putc,
> +};
> +
> +/* Mock the console device */
> +static inline void test_console_begin(const struct sbi_console_device *device)
> +{
> +	old_dev = console_dev;
> +	console_dev = device;
> +}
> +
> +static inline void test_console_end(void)
> +{
> +	console_dev = old_dev;
> +}
> +
> +static void putc_test(struct sbiunit_test_case *test)
> +{
> +	clear_test_console_buf();
> +

stray blank line

> +	test_console_begin(&test_console_dev);
> +	sbi_putc('a');
> +	test_console_end();
> +	SBIUNIT_ASSERT_EQ(test, test_console_buf[0], 'a');
> +}
> +
> +#define PUTS_TEST(test, expected, str) do {			\
> +	clear_test_console_buf();				\
> +	test_console_begin(&test_console_dev);			\
> +	sbi_puts(str);						\
> +	test_console_end();					\
> +	SBIUNIT_ASSERT_STREQ(test, test_console_buf, expected,	\
> +			     sbi_strlen(expected));		\
> +} while (0)
> +
> +static void puts_test(struct sbiunit_test_case *test)
> +{
> +	PUTS_TEST(test, "Hello, OpenSBI!", "Hello, OpenSBI!");
> +	PUTS_TEST(test, "Hello,\r\nOpenSBI!", "Hello,\nOpenSBI!");
> +}
> +
> +#define PRINTF_TEST(test, expected, format, ...) do {		\
> +	clear_test_console_buf();				\
> +	test_console_begin(&test_console_dev);			\
> +	size_t __res = sbi_printf(format, ##__VA_ARGS__);	\
> +	test_console_end();					\
> +	SBIUNIT_ASSERT_EQ(test, __res, sbi_strlen(expected));	\
> +	SBIUNIT_ASSERT_STREQ(test, test_console_buf, expected,	\
> +			     sbi_strlen(expected));		\
> +} while (0)
> +
> +static void printf_test(struct sbiunit_test_case *test)
> +{
> +	PRINTF_TEST(test, "Hello", "Hello");
> +	PRINTF_TEST(test, "3 5 7", "%d %d %d", 3, 5, 7);
> +	PRINTF_TEST(test, "Hello", "%s", "Hello");
> +	PRINTF_TEST(test, "-1", "%d", -1);
> +	PRINTF_TEST(test, "FF", "%X", 255);
> +	PRINTF_TEST(test, "ff", "%x", 255);
> +	PRINTF_TEST(test, "A", "%c", 'A');
> +	PRINTF_TEST(test, "1fe", "%p", (void *)0x1fe);
> +	PRINTF_TEST(test, "4294967295", "%u", 4294967295U);
> +	PRINTF_TEST(test, "-2147483647", "%ld", -2147483647l);
> +	PRINTF_TEST(test, "-9223372036854775807", "%lld", -9223372036854775807LL);
> +	PRINTF_TEST(test, "18446744073709551615", "%llu", 18446744073709551615ULL);
> +}
> +
> +static struct sbiunit_test_case console_test_cases[] = {
> +	SBIUNIT_TEST_CASE(putc_test),
> +	SBIUNIT_TEST_CASE(puts_test),
> +	SBIUNIT_TEST_CASE(printf_test),
> +	SBIUNIT_END_CASE,
> +};
> +
> +SBIUNIT_TEST_SUITE(console_test_suite, console_test_cases);
> -- 
> 2.34.1
>

Will there ever be a chance that more than one hart will run these tests
simultaneously? If so, we need to take a lock in PUTS_TEST and
PRINTF_TEST.

Thanks,
drew
Ivan Orlov March 4, 2024, 3:46 p.m. UTC | #2 
On 3/4/24 13:44, Andrew Jones wrote:
> On Fri, Mar 01, 2024 at 04:00:45PM +0000, Ivan Orlov wrote:
>> Add the test suite covering some of the functions from
>> lib/sbi/sbi_console.c: putc, puts and printf. The test covers a variety
>> of format specifiers for printf and different strings and characters for
>> putc and puts.
>>
>> In order to do that, the test "mocks" the sbi_console_device structure
>> by setting the 'console_dev' variable to the virtual console.
>>
>> Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
>> ---
>> V1 -> V2:
>> - Rewrite using the carray functionality
>> - Replace CONSOLE_DO and CONSOLE_DO_RET macros with two inline
>> functions: one of them "mocks" the default console device, and
>> the second one restores the old console device.
>> - Fix codestyle issues (comments, etc.)
>> - Remove incorrect 'puts' test
>> - Use updated SBIUNIT_ASSERT_STREQ API
>> V2 -> V3:
>> - Remove unused include statement
>> - Rename "new_dev" => "test_console_dev"
>> - Use SBIUNIT_END_CASE macro in the test cases list instead of '{}'
>>
>>   lib/sbi/objects.mk         |   1 +
>>   lib/sbi/sbi_console.c      |   4 ++
>>   lib/sbi/sbi_console_test.c | 101 +++++++++++++++++++++++++++++++++++++
>>   3 files changed, 106 insertions(+)
>>   create mode 100644 lib/sbi/sbi_console_test.c
>>
>> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
>> index b4c273f..9d065fa 100644
>> --- a/lib/sbi/objects.mk
>> +++ b/lib/sbi/objects.mk
>> @@ -16,6 +16,7 @@ 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 ab09a5c..d1229d0 100644
>> --- a/lib/sbi/sbi_console.c
>> +++ b/lib/sbi/sbi_console.c
>> @@ -488,3 +488,7 @@ int sbi_console_init(struct sbi_scratch *scratch)
>>   
>>   	return rc;
>>   }
>> +
>> +#ifdef CONFIG_SBIUNIT
>> +#include "sbi_console_test.c"
>> +#endif
>> diff --git a/lib/sbi/sbi_console_test.c b/lib/sbi/sbi_console_test.c
>> new file mode 100644
>> index 0000000..734a68c
>> --- /dev/null
>> +++ b/lib/sbi/sbi_console_test.c
>> @@ -0,0 +1,101 @@
>> +/*
>> + * SPDX-License-Identifier: BSD-2-Clause
>> + *
>> + * Author: Ivan Orlov <ivan.orlov0322@gmail.com>
>> + */
>> +#include <sbi/sbi_unit_test.h>
>> +
>> +#define TEST_CONSOLE_BUF_LEN 1024
>> +
>> +static const struct sbi_console_device *old_dev;
>> +static char test_console_buf[TEST_CONSOLE_BUF_LEN];
>> +static u32 test_console_buf_pos;
>> +
>> +static void test_console_putc(char c)
>> +{
>> +	test_console_buf[test_console_buf_pos] = c;
>> +	test_console_buf_pos = (test_console_buf_pos + 1) % TEST_CONSOLE_BUF_LEN;
>> +}
>> +
>> +static void clear_test_console_buf(void)
>> +{
>> +	test_console_buf_pos = 0;
>> +	test_console_buf[0] = '\0';
>> +}
>> +
>> +static const struct sbi_console_device test_console_dev = {
>> +	.name = "Test console device",
>> +	.console_putc = test_console_putc,
>> +};
>> +
>> +/* Mock the console device */
>> +static inline void test_console_begin(const struct sbi_console_device *device)
>> +{
>> +	old_dev = console_dev;
>> +	console_dev = device;
>> +}
>> +
>> +static inline void test_console_end(void)
>> +{
>> +	console_dev = old_dev;
>> +}
>> +
>> +static void putc_test(struct sbiunit_test_case *test)
>> +{
>> +	clear_test_console_buf();
>> +
> 
> stray blank line
> 
>> +	test_console_begin(&test_console_dev);
>> +	sbi_putc('a');
>> +	test_console_end();
>> +	SBIUNIT_ASSERT_EQ(test, test_console_buf[0], 'a');
>> +}
>> +
>> +#define PUTS_TEST(test, expected, str) do {			\
>> +	clear_test_console_buf();				\
>> +	test_console_begin(&test_console_dev);			\
>> +	sbi_puts(str);						\
>> +	test_console_end();					\
>> +	SBIUNIT_ASSERT_STREQ(test, test_console_buf, expected,	\
>> +			     sbi_strlen(expected));		\
>> +} while (0)
>> +
>> +static void puts_test(struct sbiunit_test_case *test)
>> +{
>> +	PUTS_TEST(test, "Hello, OpenSBI!", "Hello, OpenSBI!");
>> +	PUTS_TEST(test, "Hello,\r\nOpenSBI!", "Hello,\nOpenSBI!");
>> +}
>> +
>> +#define PRINTF_TEST(test, expected, format, ...) do {		\
>> +	clear_test_console_buf();				\
>> +	test_console_begin(&test_console_dev);			\
>> +	size_t __res = sbi_printf(format, ##__VA_ARGS__);	\
>> +	test_console_end();					\
>> +	SBIUNIT_ASSERT_EQ(test, __res, sbi_strlen(expected));	\
>> +	SBIUNIT_ASSERT_STREQ(test, test_console_buf, expected,	\
>> +			     sbi_strlen(expected));		\
>> +} while (0)
>> +
>> +static void printf_test(struct sbiunit_test_case *test)
>> +{
>> +	PRINTF_TEST(test, "Hello", "Hello");
>> +	PRINTF_TEST(test, "3 5 7", "%d %d %d", 3, 5, 7);
>> +	PRINTF_TEST(test, "Hello", "%s", "Hello");
>> +	PRINTF_TEST(test, "-1", "%d", -1);
>> +	PRINTF_TEST(test, "FF", "%X", 255);
>> +	PRINTF_TEST(test, "ff", "%x", 255);
>> +	PRINTF_TEST(test, "A", "%c", 'A');
>> +	PRINTF_TEST(test, "1fe", "%p", (void *)0x1fe);
>> +	PRINTF_TEST(test, "4294967295", "%u", 4294967295U);
>> +	PRINTF_TEST(test, "-2147483647", "%ld", -2147483647l);
>> +	PRINTF_TEST(test, "-9223372036854775807", "%lld", -9223372036854775807LL);
>> +	PRINTF_TEST(test, "18446744073709551615", "%llu", 18446744073709551615ULL);
>> +}
>> +
>> +static struct sbiunit_test_case console_test_cases[] = {
>> +	SBIUNIT_TEST_CASE(putc_test),
>> +	SBIUNIT_TEST_CASE(puts_test),
>> +	SBIUNIT_TEST_CASE(printf_test),
>> +	SBIUNIT_END_CASE,
>> +};
>> +
>> +SBIUNIT_TEST_SUITE(console_test_suite, console_test_cases);
>> -- 
>> 2.34.1
>>
> 
> Will there ever be a chance that more than one hart will run these tests
> simultaneously? If so, we need to take a lock in PUTS_TEST and
> PRINTF_TEST.
> 

Hi Andrew,

We call 'run_all_tests' function in 'init_coldboot', which runs on the 
coldboot hart only. If I understand it correctly (maybe I'm wrong), 
there could be only one coldboot hart in the system, so there is no 
chance that we will run tests on two harts simultaneously.

How do you think, should the reason why we don't use locking in the 
tests be documented somewhere? Or maybe we should use the locking 
anyway, just to make it more robust and avoid errors if we decide to run 
tests on multiple cores in the future? (but it is pretty hard for me to 
imagine why we should do that)
Andrew Jones March 4, 2024, 4:33 p.m. UTC | #3 
On Mon, Mar 04, 2024 at 03:46:01PM +0000, Ivan Orlov wrote:
> On 3/4/24 13:44, Andrew Jones wrote:
> > On Fri, Mar 01, 2024 at 04:00:45PM +0000, Ivan Orlov wrote:
> > > Add the test suite covering some of the functions from
> > > lib/sbi/sbi_console.c: putc, puts and printf. The test covers a variety
> > > of format specifiers for printf and different strings and characters for
> > > putc and puts.
> > > 
> > > In order to do that, the test "mocks" the sbi_console_device structure
> > > by setting the 'console_dev' variable to the virtual console.
> > > 
> > > Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
> > > ---
> > > V1 -> V2:
> > > - Rewrite using the carray functionality
> > > - Replace CONSOLE_DO and CONSOLE_DO_RET macros with two inline
> > > functions: one of them "mocks" the default console device, and
> > > the second one restores the old console device.
> > > - Fix codestyle issues (comments, etc.)
> > > - Remove incorrect 'puts' test
> > > - Use updated SBIUNIT_ASSERT_STREQ API
> > > V2 -> V3:
> > > - Remove unused include statement
> > > - Rename "new_dev" => "test_console_dev"
> > > - Use SBIUNIT_END_CASE macro in the test cases list instead of '{}'
> > > 
> > >   lib/sbi/objects.mk         |   1 +
> > >   lib/sbi/sbi_console.c      |   4 ++
> > >   lib/sbi/sbi_console_test.c | 101 +++++++++++++++++++++++++++++++++++++
> > >   3 files changed, 106 insertions(+)
> > >   create mode 100644 lib/sbi/sbi_console_test.c
> > > 
> > > diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> > > index b4c273f..9d065fa 100644
> > > --- a/lib/sbi/objects.mk
> > > +++ b/lib/sbi/objects.mk
> > > @@ -16,6 +16,7 @@ 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 ab09a5c..d1229d0 100644
> > > --- a/lib/sbi/sbi_console.c
> > > +++ b/lib/sbi/sbi_console.c
> > > @@ -488,3 +488,7 @@ int sbi_console_init(struct sbi_scratch *scratch)
> > >   	return rc;
> > >   }
> > > +
> > > +#ifdef CONFIG_SBIUNIT
> > > +#include "sbi_console_test.c"
> > > +#endif
> > > diff --git a/lib/sbi/sbi_console_test.c b/lib/sbi/sbi_console_test.c
> > > new file mode 100644
> > > index 0000000..734a68c
> > > --- /dev/null
> > > +++ b/lib/sbi/sbi_console_test.c
> > > @@ -0,0 +1,101 @@
> > > +/*
> > > + * SPDX-License-Identifier: BSD-2-Clause
> > > + *
> > > + * Author: Ivan Orlov <ivan.orlov0322@gmail.com>
> > > + */
> > > +#include <sbi/sbi_unit_test.h>
> > > +
> > > +#define TEST_CONSOLE_BUF_LEN 1024
> > > +
> > > +static const struct sbi_console_device *old_dev;
> > > +static char test_console_buf[TEST_CONSOLE_BUF_LEN];
> > > +static u32 test_console_buf_pos;
> > > +
> > > +static void test_console_putc(char c)
> > > +{
> > > +	test_console_buf[test_console_buf_pos] = c;
> > > +	test_console_buf_pos = (test_console_buf_pos + 1) % TEST_CONSOLE_BUF_LEN;
> > > +}
> > > +
> > > +static void clear_test_console_buf(void)
> > > +{
> > > +	test_console_buf_pos = 0;
> > > +	test_console_buf[0] = '\0';
> > > +}
> > > +
> > > +static const struct sbi_console_device test_console_dev = {
> > > +	.name = "Test console device",
> > > +	.console_putc = test_console_putc,
> > > +};
> > > +
> > > +/* Mock the console device */
> > > +static inline void test_console_begin(const struct sbi_console_device *device)
> > > +{
> > > +	old_dev = console_dev;
> > > +	console_dev = device;
> > > +}
> > > +
> > > +static inline void test_console_end(void)
> > > +{
> > > +	console_dev = old_dev;
> > > +}
> > > +
> > > +static void putc_test(struct sbiunit_test_case *test)
> > > +{
> > > +	clear_test_console_buf();
> > > +
> > 
> > stray blank line
> > 
> > > +	test_console_begin(&test_console_dev);
> > > +	sbi_putc('a');
> > > +	test_console_end();
> > > +	SBIUNIT_ASSERT_EQ(test, test_console_buf[0], 'a');
> > > +}
> > > +
> > > +#define PUTS_TEST(test, expected, str) do {			\
> > > +	clear_test_console_buf();				\
> > > +	test_console_begin(&test_console_dev);			\
> > > +	sbi_puts(str);						\
> > > +	test_console_end();					\
> > > +	SBIUNIT_ASSERT_STREQ(test, test_console_buf, expected,	\
> > > +			     sbi_strlen(expected));		\
> > > +} while (0)
> > > +
> > > +static void puts_test(struct sbiunit_test_case *test)
> > > +{
> > > +	PUTS_TEST(test, "Hello, OpenSBI!", "Hello, OpenSBI!");
> > > +	PUTS_TEST(test, "Hello,\r\nOpenSBI!", "Hello,\nOpenSBI!");
> > > +}
> > > +
> > > +#define PRINTF_TEST(test, expected, format, ...) do {		\
> > > +	clear_test_console_buf();				\
> > > +	test_console_begin(&test_console_dev);			\
> > > +	size_t __res = sbi_printf(format, ##__VA_ARGS__);	\
> > > +	test_console_end();					\
> > > +	SBIUNIT_ASSERT_EQ(test, __res, sbi_strlen(expected));	\
> > > +	SBIUNIT_ASSERT_STREQ(test, test_console_buf, expected,	\
> > > +			     sbi_strlen(expected));		\
> > > +} while (0)
> > > +
> > > +static void printf_test(struct sbiunit_test_case *test)
> > > +{
> > > +	PRINTF_TEST(test, "Hello", "Hello");
> > > +	PRINTF_TEST(test, "3 5 7", "%d %d %d", 3, 5, 7);
> > > +	PRINTF_TEST(test, "Hello", "%s", "Hello");
> > > +	PRINTF_TEST(test, "-1", "%d", -1);
> > > +	PRINTF_TEST(test, "FF", "%X", 255);
> > > +	PRINTF_TEST(test, "ff", "%x", 255);
> > > +	PRINTF_TEST(test, "A", "%c", 'A');
> > > +	PRINTF_TEST(test, "1fe", "%p", (void *)0x1fe);
> > > +	PRINTF_TEST(test, "4294967295", "%u", 4294967295U);
> > > +	PRINTF_TEST(test, "-2147483647", "%ld", -2147483647l);
> > > +	PRINTF_TEST(test, "-9223372036854775807", "%lld", -9223372036854775807LL);
> > > +	PRINTF_TEST(test, "18446744073709551615", "%llu", 18446744073709551615ULL);
> > > +}
> > > +
> > > +static struct sbiunit_test_case console_test_cases[] = {
> > > +	SBIUNIT_TEST_CASE(putc_test),
> > > +	SBIUNIT_TEST_CASE(puts_test),
> > > +	SBIUNIT_TEST_CASE(printf_test),
> > > +	SBIUNIT_END_CASE,
> > > +};
> > > +
> > > +SBIUNIT_TEST_SUITE(console_test_suite, console_test_cases);
> > > -- 
> > > 2.34.1
> > > 
> > 
> > Will there ever be a chance that more than one hart will run these tests
> > simultaneously? If so, we need to take a lock in PUTS_TEST and
> > PRINTF_TEST.
> > 
> 
> Hi Andrew,
> 
> We call 'run_all_tests' function in 'init_coldboot', which runs on the
> coldboot hart only. If I understand it correctly (maybe I'm wrong), there
> could be only one coldboot hart in the system, so there is no chance that we
> will run tests on two harts simultaneously.
> 
> How do you think, should the reason why we don't use locking in the tests be
> documented somewhere? Or maybe we should use the locking anyway, just to
> make it more robust and avoid errors if we decide to run tests on multiple
> cores in the future? (but it is pretty hard for me to imagine why we should
> do that)
>

I'm not sure what's best, since I don't have a good feel for how this test
framework will evolve. I'd vote for adding a lock now, since it won't
hurt.

Thanks,
drew
Ivan Orlov March 4, 2024, 4:46 p.m. UTC | #4 
On 3/4/24 16:33, Andrew Jones wrote:
> 
> I'm not sure what's best, since I don't have a good feel for how this test
> framework will evolve. I'd vote for adding a lock now, since it won't
> hurt.
> 

Alright, I'll add a spinlock in V4. Thanks!
diff mbox series

Patch

diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
index b4c273f..9d065fa 100644
--- a/lib/sbi/objects.mk
+++ b/lib/sbi/objects.mk
@@ -16,6 +16,7 @@  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 ab09a5c..d1229d0 100644
--- a/lib/sbi/sbi_console.c
+++ b/lib/sbi/sbi_console.c
@@ -488,3 +488,7 @@  int sbi_console_init(struct sbi_scratch *scratch)
 
 	return rc;
 }
+
+#ifdef CONFIG_SBIUNIT
+#include "sbi_console_test.c"
+#endif
diff --git a/lib/sbi/sbi_console_test.c b/lib/sbi/sbi_console_test.c
new file mode 100644
index 0000000..734a68c
--- /dev/null
+++ b/lib/sbi/sbi_console_test.c
@@ -0,0 +1,101 @@ 
+/*
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Author: Ivan Orlov <ivan.orlov0322@gmail.com>
+ */
+#include <sbi/sbi_unit_test.h>
+
+#define TEST_CONSOLE_BUF_LEN 1024
+
+static const struct sbi_console_device *old_dev;
+static char test_console_buf[TEST_CONSOLE_BUF_LEN];
+static u32 test_console_buf_pos;
+
+static void test_console_putc(char c)
+{
+	test_console_buf[test_console_buf_pos] = c;
+	test_console_buf_pos = (test_console_buf_pos + 1) % TEST_CONSOLE_BUF_LEN;
+}
+
+static void clear_test_console_buf(void)
+{
+	test_console_buf_pos = 0;
+	test_console_buf[0] = '\0';
+}
+
+static const struct sbi_console_device test_console_dev = {
+	.name = "Test console device",
+	.console_putc = test_console_putc,
+};
+
+/* Mock the console device */
+static inline void test_console_begin(const struct sbi_console_device *device)
+{
+	old_dev = console_dev;
+	console_dev = device;
+}
+
+static inline void test_console_end(void)
+{
+	console_dev = old_dev;
+}
+
+static void putc_test(struct sbiunit_test_case *test)
+{
+	clear_test_console_buf();
+
+	test_console_begin(&test_console_dev);
+	sbi_putc('a');
+	test_console_end();
+	SBIUNIT_ASSERT_EQ(test, test_console_buf[0], 'a');
+}
+
+#define PUTS_TEST(test, expected, str) do {			\
+	clear_test_console_buf();				\
+	test_console_begin(&test_console_dev);			\
+	sbi_puts(str);						\
+	test_console_end();					\
+	SBIUNIT_ASSERT_STREQ(test, test_console_buf, expected,	\
+			     sbi_strlen(expected));		\
+} while (0)
+
+static void puts_test(struct sbiunit_test_case *test)
+{
+	PUTS_TEST(test, "Hello, OpenSBI!", "Hello, OpenSBI!");
+	PUTS_TEST(test, "Hello,\r\nOpenSBI!", "Hello,\nOpenSBI!");
+}
+
+#define PRINTF_TEST(test, expected, format, ...) do {		\
+	clear_test_console_buf();				\
+	test_console_begin(&test_console_dev);			\
+	size_t __res = sbi_printf(format, ##__VA_ARGS__);	\
+	test_console_end();					\
+	SBIUNIT_ASSERT_EQ(test, __res, sbi_strlen(expected));	\
+	SBIUNIT_ASSERT_STREQ(test, test_console_buf, expected,	\
+			     sbi_strlen(expected));		\
+} while (0)
+
+static void printf_test(struct sbiunit_test_case *test)
+{
+	PRINTF_TEST(test, "Hello", "Hello");
+	PRINTF_TEST(test, "3 5 7", "%d %d %d", 3, 5, 7);
+	PRINTF_TEST(test, "Hello", "%s", "Hello");
+	PRINTF_TEST(test, "-1", "%d", -1);
+	PRINTF_TEST(test, "FF", "%X", 255);
+	PRINTF_TEST(test, "ff", "%x", 255);
+	PRINTF_TEST(test, "A", "%c", 'A');
+	PRINTF_TEST(test, "1fe", "%p", (void *)0x1fe);
+	PRINTF_TEST(test, "4294967295", "%u", 4294967295U);
+	PRINTF_TEST(test, "-2147483647", "%ld", -2147483647l);
+	PRINTF_TEST(test, "-9223372036854775807", "%lld", -9223372036854775807LL);
+	PRINTF_TEST(test, "18446744073709551615", "%llu", 18446744073709551615ULL);
+}
+
+static struct sbiunit_test_case console_test_cases[] = {
+	SBIUNIT_TEST_CASE(putc_test),
+	SBIUNIT_TEST_CASE(puts_test),
+	SBIUNIT_TEST_CASE(printf_test),
+	SBIUNIT_END_CASE,
+};
+
+SBIUNIT_TEST_SUITE(console_test_suite, console_test_cases);