Hi Ahmad,
This looks nice, I can't wait to try it out :]
On Mon, Apr 12, 2021 at 09:16:43AM +0200, Ahmad Fatoum wrote:
> Self tests is code written to run within barebox to exercise
> functionality. They offer flexibility to test specific units of barebox
> instead of the program as a whole. Add a very simple infrastructure
> for registering and executing self-tests. THis is based on the Linux
> kselftest modules. We don't utilize modules for this, however, because
> we only have module support on ARM, but we need a generic solution.
>
> Selftests can be enabled individually and even tested without shell
> support to allow tests to happen for size-restricted barebox images
> as well.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> ---
> Kconfig | 1 +
> Makefile | 2 +-
> commands/Makefile | 1 +
> commands/selftest.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
> include/bselftest.h | 70 ++++++++++++++++++++++++++++++++++++
> test/Kconfig | 8 +++++
> test/Makefile | 1 +
> test/self/Kconfig | 33 +++++++++++++++++
> test/self/Makefile | 3 ++
> test/self/core.c | 39 ++++++++++++++++++++
> 10 files changed, 245 insertions(+), 1 deletion(-)
> create mode 100644 commands/selftest.c
> create mode 100644 include/bselftest.h
> create mode 100644 test/Kconfig
> create mode 100644 test/Makefile
> create mode 100644 test/self/Kconfig
> create mode 100644 test/self/Makefile
> create mode 100644 test/self/core.c
>
> diff --git a/Kconfig b/Kconfig
> index 29c32463fb43..7c4cf36881b4 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -15,3 +15,4 @@ source "lib/Kconfig"
> source "crypto/Kconfig"
> source "firmware/Kconfig"
> source "scripts/Kconfig"
> +source "test/Kconfig"
> diff --git a/Makefile b/Makefile
> index f6c0c4817046..0a93915cf347 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -581,7 +581,7 @@ endif
> include $(srctree)/scripts/Makefile.lib
>
> # Objects we will link into barebox / subdirs we need to visit
> -common-y := common/ drivers/ commands/ lib/ crypto/ net/ fs/ firmware/
> +common-y := common/ drivers/ commands/ lib/ crypto/ net/ fs/ firmware/ test/
>
> include $(srctree)/arch/$(SRCARCH)/Makefile
>
> diff --git a/commands/Makefile b/commands/Makefile
> index 447349fd157d..4b45d266fd56 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -130,5 +130,6 @@ obj-$(CONFIG_CMD_SEED) += seed.o
> obj-$(CONFIG_CMD_IP_ROUTE_GET) += ip-route-get.o
> obj-$(CONFIG_CMD_BTHREAD) += bthread.o
> obj-$(CONFIG_CMD_UBSAN) += ubsan.o
> +obj-$(CONFIG_CMD_SELFTEST) += selftest.o
>
> UBSAN_SANITIZE_ubsan.o := y
> diff --git a/commands/selftest.c b/commands/selftest.c
> new file mode 100644
> index 000000000000..de5d100de04a
> --- /dev/null
> +++ b/commands/selftest.c
> @@ -0,0 +1,88 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#define pr_fmt(fmt) "selftest: " fmt
> +
> +#include <common.h>
> +#include <command.h>
> +#include <getopt.h>
> +#include <bselftest.h>
> +#include <complete.h>
> +
> +static int run_selftest(const char *match, bool list)
> +{
> + struct selftest *test;
> + int matches = 0;
> + int err = 0;
> +
> + list_for_each_entry(test, &selftests, list) {
> + if (list) {
> + printf("%s\n", test->name);
> + matches++;
> + continue;
> + }
> +
> + if (match && strcmp(test->name, match))
> + continue;
> +
> + err |= test->func();
> + matches++;
> + }
> +
> + if (!matches) {
> + if (match)
> + printf("No tests matching '%s' found.\n", match);
> + else
> + printf("No matching tests found.\n");
> +
> + return -EINVAL;
What if there is no tests at all: is that an error if no tests were ran?
despit not using a match condition ?
> + }
> +
> + return err;
> +}
> +
snip
> diff --git a/test/self/core.c b/test/self/core.c
> new file mode 100644
> index 000000000000..d8c6e5d271ba
> --- /dev/null
> +++ b/test/self/core.c
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#define pr_fmt(fmt) "bselftest: " fmt
> +
> +#include <common.h>
> +#include <bselftest.h>
> +
> +LIST_HEAD(selftests);
> +
> +static int (*old_main)(void);
> +
> +static int run_selftests(void)
> +{
> + struct selftest *test;
> + int err = 0;
> +
> + pr_notice("Configured tests will run now\n");
> +
> + list_for_each_entry(test, &selftests, list)
> + err |= test->func();
> +
> + if (err)
> + pr_err("Some selftests failed\n");
> +
> + barebox_main = old_main;
> + return barebox_main();
> +}
Why not call run_selftests in start_barebox, right before barebox_main ?
Or maybe add a selftest initcall to be run at the very end of initcalls.
So you don't have to save and restore the barebox_main function.
> +
> +static int init_selftests(void)
> +{
> + if (!IS_ENABLED(CONFIG_SELFTEST_AUTORUN))
> + return 0;
> +
> + old_main = barebox_main;
> + barebox_main = run_selftests;
> +
> + return 0;
> +}
> +postenvironment_initcall(init_selftests);
> --
> 2.29.2
>
>
> _______________________________________________
> barebox mailing list
> barebox@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/barebox
>
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
