[PATCH 00/17] dm: test: Add unit tests for SPL

U-Boot has about 700 tests but only a handful of these target SPL. This is partly because SPL has limited functionality and does not have a command interface to initiate tests.
The current SPL tests are targeted at sandbox_spl and are initiated from pytest.
With SPL tests written in C, we can check the behaviour of of-platdata more easily, since the test can check things directly rather than printing out information for pytest to check.
This series adds support for SPL tests written in C, targeted at the sandbox_spl build.
It includes quite a bit of minor refactoring to get everything working.
Simon Glass (17): dtoc: Extract inner loop from output_node() dtoc: Use a namedtuple for _links dm: core: Expand the comment for DM_GET_DEVICE() dm: core: Avoid void * in the of-platdata structs dm: Avoid using #ifdef for CONFIG_OF_LIVE dm: test: Sort the Makefile dm: test: Update Makefile conditions dm: test: Make use of CONFIG_UNIT_TEST dm: test: Disable some tests that should not run in SPL dm: test: Build tests for SPL dm: test: Update the test runner to support of-platdata dm: test: Add a way to run SPL tests dm: test: Add a very simple of-platadata test Makefile: Generate a symbol file for u-boot-spl pytest: Collect SPL unit tests test: Run SPL unit tests Azure/GitLab/Travis: Add SPL unit tests
.azure-pipelines.yml | 2 +- .gitlab-ci.yml | 2 +- .travis.yml | 2 +- Makefile | 2 +- arch/sandbox/cpu/spl.c | 8 +++ arch/sandbox/cpu/start.c | 9 +++ arch/sandbox/include/asm/state.h | 1 + common/board_r.c | 19 +++--- configs/sandbox_spl_defconfig | 2 +- drivers/core/Makefile | 2 +- drivers/core/root.c | 27 +++----- include/asm-generic/global_data.h | 13 +++- include/dm/of.h | 9 +-- include/dm/platdata.h | 17 ++++- include/dt-structs.h | 8 ++- include/test/test.h | 11 ++++ scripts/Makefile.spl | 8 ++- test/Kconfig | 10 +++ test/Makefile | 24 ++++--- test/dm/Makefile | 13 ++-- test/dm/of_platdata.c | 19 ++++++ test/dm/test-main.c | 45 +++++++------ test/py/conftest.py | 14 ++-- test/py/tests/test_spl.py | 29 +++++++++ test/run | 2 +- tools/dtoc/dtb_platdata.py | 103 +++++++++++++++++------------- 26 files changed, 267 insertions(+), 134 deletions(-) create mode 100644 test/dm/of_platdata.c create mode 100644 test/py/tests/test_spl.py

This function is very long. Put the inner loop in a separate function to enhance readability.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/dtoc/dtb_platdata.py | 89 +++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 40 deletions(-)
diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index 579a6749c40..bd1daa4379a 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -560,6 +560,54 @@ class DtbPlatdata(object): Args: node: node to output """ + def _output_list(node, prop): + """Output the C code for a devicetree property that holds a list + + Args: + node (fdt.Node): Node to output + prop (fdt.Prop): Prop to output + """ + self.buf('{') + vals = [] + # For phandles, output a reference to the platform data + # of the target node. + info = self.get_phandle_argc(prop, node.name) + if info: + # Process the list as pairs of (phandle, id) + pos = 0 + item = 0 + for args in info.args: + phandle_cell = prop.value[pos] + phandle = fdt_util.fdt32_to_cpu(phandle_cell) + target_node = self._fdt.phandle_to_node[phandle] + name = conv_name_to_c(target_node.name) + arg_values = [] + for i in range(args): + arg_values.append(str(fdt_util.fdt32_to_cpu(prop.value[pos + 1 + i]))) + pos += 1 + args + # node member is filled with NULL as the real value + # will be update at run-time during dm_init_and_scan() + # by dm_populate_phandle_data() + vals.append('\t{NULL, {%s}}' % (', '.join(arg_values))) + var_node = '%s%s.%s[%d].node' % \ + (VAL_PREFIX, var_name, member_name, item) + # Save the the link information to be use to define + # dm_populate_phandle_data() + self._links.append({'var_node': var_node, 'dev_name': name}) + item += 1 + for val in vals: + self.buf('\n\t\t%s,' % val) + else: + for val in prop.value: + vals.append(get_value(prop.type, val)) + + # Put 8 values per line to avoid very long lines. + for i in range(0, len(vals), 8): + if i: + self.buf(',\n\t\t') + self.buf(', '.join(vals[i:i + 8])) + self.buf('}') + struct_name, _ = self.get_normalized_compat_name(node) var_name = conv_name_to_c(node.name) self.buf('static struct %s%s %s%s = {\n' % @@ -573,46 +621,7 @@ class DtbPlatdata(object):
# Special handling for lists if isinstance(prop.value, list): - self.buf('{') - vals = [] - # For phandles, output a reference to the platform data - # of the target node. - info = self.get_phandle_argc(prop, node.name) - if info: - # Process the list as pairs of (phandle, id) - pos = 0 - item = 0 - for args in info.args: - phandle_cell = prop.value[pos] - phandle = fdt_util.fdt32_to_cpu(phandle_cell) - target_node = self._fdt.phandle_to_node[phandle] - name = conv_name_to_c(target_node.name) - arg_values = [] - for i in range(args): - arg_values.append(str(fdt_util.fdt32_to_cpu(prop.value[pos + 1 + i]))) - pos += 1 + args - # node member is filled with NULL as the real value - # will be update at run-time during dm_init_and_scan() - # by dm_populate_phandle_data() - vals.append('\t{NULL, {%s}}' % (', '.join(arg_values))) - var_node = '%s%s.%s[%d].node' % \ - (VAL_PREFIX, var_name, member_name, item) - # Save the the link information to be use to define - # dm_populate_phandle_data() - self._links.append({'var_node': var_node, 'dev_name': name}) - item += 1 - for val in vals: - self.buf('\n\t\t%s,' % val) - else: - for val in prop.value: - vals.append(get_value(prop.type, val)) - - # Put 8 values per line to avoid very long lines. - for i in range(0, len(vals), 8): - if i: - self.buf(',\n\t\t') - self.buf(', '.join(vals[i:i + 8])) - self.buf('}') + _output_list(node, prop) else: self.buf(get_value(prop.type, prop.value)) self.buf(',\n')

This function is very long. Put the inner loop in a separate function to enhance readability.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/dtoc/dtb_platdata.py | 89 +++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 40 deletions(-)
Applied to u-boot-dm, thanks!

The use of strings to access a dict is a bit ugly. Use a namedtuple for this instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/dtoc/dtb_platdata.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py index bd1daa4379a..1b52198a943 100644 --- a/tools/dtoc/dtb_platdata.py +++ b/tools/dtoc/dtb_platdata.py @@ -54,6 +54,13 @@ VAL_PREFIX = 'dtv_' # phandles is len(args). This is a list of integers. PhandleInfo = collections.namedtuple('PhandleInfo', ['max_args', 'args'])
+# Holds a single phandle link, allowing a C struct value to be assigned to point +# to a device +# +# var_node: C variable to assign (e.g. 'dtv_mmc.clocks[0].node') +# dev_name: Name of device to assign to (e.g. 'clock') +PhandleLink = collections.namedtuple('PhandleLink', ['var_node', 'dev_name']) +
def conv_name_to_c(name): """Convert a device-tree name to a C identifier @@ -146,7 +153,8 @@ class DtbPlatdata(object): key: Driver alias declared with U_BOOT_DRIVER_ALIAS(driver_alias, driver_name) value: Driver name declared with U_BOOT_DRIVER(driver_name) - _links: List of links to be included in dm_populate_phandle_data() + _links: List of links to be included in dm_populate_phandle_data(), + each a PhandleLink _drivers_additional: List of additional drivers to use during scanning """ def __init__(self, dtb_fname, include_disabled, warning_disabled, @@ -593,7 +601,7 @@ class DtbPlatdata(object): (VAL_PREFIX, var_name, member_name, item) # Save the the link information to be use to define # dm_populate_phandle_data() - self._links.append({'var_node': var_node, 'dev_name': name}) + self._links.append(PhandleLink(var_node, name)) item += 1 for val in vals: self.buf('\n\t\t%s,' % val) @@ -669,9 +677,9 @@ class DtbPlatdata(object): # nodes using DM_GET_DEVICE # dtv_dmc_at_xxx.clocks[0].node = DM_GET_DEVICE(clock_controller_at_xxx) self.buf('void dm_populate_phandle_data(void) {\n') - for l in self._links: + for link in self._links: self.buf('\t%s = DM_GET_DEVICE(%s);\n' % - (l['var_node'], l['dev_name'])) + (link.var_node, link.dev_name)) self.buf('}\n')
self.out(''.join(self.get_buf()))

The use of strings to access a dict is a bit ugly. Use a namedtuple for this instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
tools/dtoc/dtb_platdata.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
Applied to u-boot-dm, thanks!

The current documentation for this is not particularly enlightening. Add a little more detail.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dm/platdata.h | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/include/dm/platdata.h b/include/dm/platdata.h index cab93b071ba..25479b03d22 100644 --- a/include/dm/platdata.h +++ b/include/dm/platdata.h @@ -45,7 +45,22 @@ struct driver_info { #define U_BOOT_DEVICES(__name) \ ll_entry_declare_list(struct driver_info, __name, driver_info)
-/* Get a pointer to a given driver */ +/** + * Get a pointer to a given device info given its name + * + * With the declaration U_BOOT_DEVICE(name), DM_GET_DEVICE(name) will return a + * pointer to the struct driver_info created by that declaration. + * + * if OF_PLATDATA is enabled, from this it is possible to use the @dev member of + * struct driver_info to find the device pointer itself. + * + * TODO(sjg@chromium.org): U_BOOT_DEVICE() tells U-Boot to create a device, so + * the naming seems sensible, but DM_GET_DEVICE() is a bit of misnomer, since it + * finds the driver_info record, not the device. + * + * @__name: Driver name (C identifier, not a string. E.g. gpio7_at_ff7e0000) + * @return struct driver_info * to the driver that created the device + */ #define DM_GET_DEVICE(__name) \ ll_entry_get(struct driver_info, __name, driver_info)

The current documentation for this is not particularly enlightening. Add a little more detail.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dm/platdata.h | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
Applied to u-boot-dm, thanks!

These pointers point to drivers. Update the definition to make this clear.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dt-structs.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/dt-structs.h b/include/dt-structs.h index 924d51fc522..eed8273d18e 100644 --- a/include/dt-structs.h +++ b/include/dt-structs.h @@ -8,18 +8,20 @@
/* These structures may only be used in SPL */ #if CONFIG_IS_ENABLED(OF_PLATDATA) +struct driver_info; + struct phandle_0_arg { - const void *node; + const struct driver_info *node; int arg[0]; };
struct phandle_1_arg { - const void *node; + const struct driver_info *node; int arg[1]; };
struct phandle_2_arg { - const void *node; + const struct driver_info *node; int arg[2]; }; #include <generated/dt-structs-gen.h>

These pointers point to drivers. Update the definition to make this clear.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/dt-structs.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
Applied to u-boot-dm, thanks!

At present this option results in a number of #ifdefs due to the presence or absence of the global_data of_root member.
Add a few macros to global_data.h to work around this. Update the code accordingly.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/board_r.c | 19 +++++++++---------- drivers/core/Makefile | 2 +- drivers/core/root.c | 27 +++++++++------------------ include/asm-generic/global_data.h | 13 ++++++++++++- include/dm/of.h | 9 +-------- test/dm/test-main.c | 16 +++++----------- 6 files changed, 37 insertions(+), 49 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 9b2fec701a5..1f37345f940 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -295,20 +295,21 @@ static int initr_noncached(void) } #endif
-#ifdef CONFIG_OF_LIVE static int initr_of_live(void) { - int ret; + if (CONFIG_IS_ENABLED(OF_LIVE)) { + int ret;
- bootstage_start(BOOTSTAGE_ID_ACCUM_OF_LIVE, "of_live"); - ret = of_live_build(gd->fdt_blob, (struct device_node **)&gd->of_root); - bootstage_accum(BOOTSTAGE_ID_ACCUM_OF_LIVE); - if (ret) - return ret; + bootstage_start(BOOTSTAGE_ID_ACCUM_OF_LIVE, "of_live"); + ret = of_live_build(gd->fdt_blob, + (struct device_node **)gd_of_root_ptr()); + bootstage_accum(BOOTSTAGE_ID_ACCUM_OF_LIVE); + if (ret) + return ret; + }
return 0; } -#endif
#ifdef CONFIG_DM static int initr_dm(void) @@ -701,9 +702,7 @@ static init_fnc_t init_sequence_r[] = { #ifdef CONFIG_SYS_NONCACHED_MEMORY initr_noncached, #endif -#ifdef CONFIG_OF_LIVE initr_of_live, -#endif #ifdef CONFIG_DM initr_dm, #endif diff --git a/drivers/core/Makefile b/drivers/core/Makefile index 10f4bece335..5edd4e41357 100644 --- a/drivers/core/Makefile +++ b/drivers/core/Makefile @@ -11,7 +11,7 @@ obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o obj-$(CONFIG_DM) += dump.o obj-$(CONFIG_$(SPL_TPL_)REGMAP) += regmap.o obj-$(CONFIG_$(SPL_TPL_)SYSCON) += syscon-uclass.o -obj-$(CONFIG_OF_LIVE) += of_access.o of_addr.o +obj-$(CONFIG_$(SPL_)OF_LIVE) += of_access.o of_addr.o ifndef CONFIG_DM_DEV_READ_INLINE obj-$(CONFIG_OF_CONTROL) += read.o endif diff --git a/drivers/core/root.c b/drivers/core/root.c index 0726be6b795..de23161cff8 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -61,7 +61,7 @@ void fix_drivers(void) for (entry = drv; entry != drv + n_ents; entry++) { if (entry->of_match) entry->of_match = (const struct udevice_id *) - ((u32)entry->of_match + gd->reloc_off); + ((ulong)entry->of_match + gd->reloc_off); if (entry->bind) entry->bind += gd->reloc_off; if (entry->probe) @@ -151,11 +151,9 @@ int dm_init(bool of_live) if (ret) return ret; #if CONFIG_IS_ENABLED(OF_CONTROL) -# if CONFIG_IS_ENABLED(OF_LIVE) - if (of_live) - DM_ROOT_NON_CONST->node = np_to_ofnode(gd->of_root); + if (CONFIG_IS_ENABLED(OF_LIVE) && of_live) + DM_ROOT_NON_CONST->node = np_to_ofnode(gd_of_root()); else -#endif DM_ROOT_NON_CONST->node = offset_to_ofnode(0); #endif ret = device_probe(DM_ROOT_NON_CONST); @@ -196,7 +194,7 @@ int dm_scan_platdata(bool pre_reloc_only) return ret; }
-#if CONFIG_IS_ENABLED(OF_LIVE) +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) static int dm_scan_fdt_live(struct udevice *parent, const struct device_node *node_parent, bool pre_reloc_only) @@ -223,9 +221,7 @@ static int dm_scan_fdt_live(struct udevice *parent,
return ret; } -#endif /* CONFIG_IS_ENABLED(OF_LIVE) */
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) /** * dm_scan_fdt_node() - Scan the device tree and bind drivers for a node * @@ -272,24 +268,20 @@ int dm_scan_fdt_dev(struct udevice *dev) if (!dev_of_valid(dev)) return 0;
-#if CONFIG_IS_ENABLED(OF_LIVE) if (of_live_active()) return dm_scan_fdt_live(dev, dev_np(dev), gd->flags & GD_FLG_RELOC ? false : true); - else -#endif + return dm_scan_fdt_node(dev, gd->fdt_blob, dev_of_offset(dev), gd->flags & GD_FLG_RELOC ? false : true); }
int dm_scan_fdt(const void *blob, bool pre_reloc_only) { -#if CONFIG_IS_ENABLED(OF_LIVE) if (of_live_active()) - return dm_scan_fdt_live(gd->dm_root, gd->of_root, + return dm_scan_fdt_live(gd->dm_root, gd_of_root(), pre_reloc_only); - else -#endif + return dm_scan_fdt_node(gd->dm_root, blob, 0, pre_reloc_only); }
@@ -302,10 +294,9 @@ static int dm_scan_fdt_ofnode_path(const void *blob, const char *path, if (!ofnode_valid(node)) return 0;
-#if CONFIG_IS_ENABLED(OF_LIVE) if (of_live_active()) return dm_scan_fdt_live(gd->dm_root, node.np, pre_reloc_only); -#endif + return dm_scan_fdt_node(gd->dm_root, blob, node.of_offset, pre_reloc_only); } @@ -352,7 +343,7 @@ int dm_init_and_scan(bool pre_reloc_only) dm_populate_phandle_data(); #endif
- ret = dm_init(IS_ENABLED(CONFIG_OF_LIVE)); + ret = dm_init(CONFIG_IS_ENABLED(OF_LIVE)); if (ret) { debug("dm_init() failed: %d\n", ret); return ret; diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h index d4a4e2215dc..d6f562d90d4 100644 --- a/include/asm-generic/global_data.h +++ b/include/asm-generic/global_data.h @@ -75,7 +75,7 @@ typedef struct global_data { const void *fdt_blob; /* Our device tree, NULL if none */ void *new_fdt; /* Relocated FDT */ unsigned long fdt_size; /* Space reserved for relocated FDT */ -#ifdef CONFIG_OF_LIVE +#if CONFIG_IS_ENABLED(OF_LIVE) struct device_node *of_root; #endif
@@ -146,6 +146,17 @@ typedef struct global_data { #define gd_board_type() 0 #endif
+/* These macros help avoid #ifdefs in the code */ +#if CONFIG_IS_ENABLED(OF_LIVE) +#define gd_of_root() gd->of_root +#define gd_of_root_ptr() &gd->of_root +#define gd_set_of_root(_root) gd->of_root = (_root) +#else +#define gd_of_root() NULL +#define gd_of_root_ptr() NULL +#define gd_set_of_root(_root) +#endif + /* * Global Data Flags */ diff --git a/include/dm/of.h b/include/dm/of.h index 6bef73b441c..5cb6f44a6c6 100644 --- a/include/dm/of.h +++ b/include/dm/of.h @@ -90,17 +90,10 @@ DECLARE_GLOBAL_DATA_PTR; * * @returns true if livetree is active, false it not */ -#ifdef CONFIG_OF_LIVE static inline bool of_live_active(void) { - return gd->of_root != NULL; + return gd_of_root() != NULL; } -#else -static inline bool of_live_active(void) -{ - return false; -} -#endif
#define OF_BAD_ADDR ((u64)-1)
diff --git a/test/dm/test-main.c b/test/dm/test-main.c index 38b7b1481a7..995988723ae 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-main.c @@ -33,10 +33,8 @@ static int dm_test_init(struct unit_test_state *uts, bool of_live) memset(dm_testdrv_op_count, '\0', sizeof(dm_testdrv_op_count)); state_reset_for_test(state_get_current());
-#ifdef CONFIG_OF_LIVE /* Determine whether to make the live tree available */ - gd->of_root = of_live ? uts->of_root : NULL; -#endif + gd_set_of_root(of_live ? uts->of_root : NULL); ut_assertok(dm_init(of_live)); dms->root = dm_root();
@@ -152,9 +150,7 @@ static int dm_test_main(const char *test_name) printf("Running %d driver model tests\n", n_ents);
found = 0; -#ifdef CONFIG_OF_LIVE - uts->of_root = gd->of_root; -#endif + uts->of_root = gd_of_root(); for (test = tests; test < tests + n_ents; test++) { const char *name = test->name; int runs; @@ -167,7 +163,7 @@ static int dm_test_main(const char *test_name)
/* Run with the live tree if possible */ runs = 0; - if (IS_ENABLED(CONFIG_OF_LIVE)) { + if (CONFIG_IS_ENABLED(OF_LIVE)) { if (!(test->flags & UT_TESTF_FLAT_TREE)) { ut_assertok(dm_do_test(uts, test, true)); runs++; @@ -192,11 +188,9 @@ static int dm_test_main(const char *test_name) printf("Failures: %d\n", uts->fail_count);
/* Put everything back to normal so that sandbox works as expected */ -#ifdef CONFIG_OF_LIVE - gd->of_root = uts->of_root; -#endif + gd_set_of_root(uts->of_root); gd->dm_root = NULL; - ut_assertok(dm_init(IS_ENABLED(CONFIG_OF_LIVE))); + ut_assertok(dm_init(CONFIG_IS_ENABLED(OF_LIVE))); dm_scan_platdata(false); dm_scan_fdt(gd->fdt_blob, false);

Move everything into alphabetical order.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/test/Makefile b/test/Makefile index 7c4039964e1..13ee661f50e 100644 --- a/test/Makefile +++ b/test/Makefile @@ -5,12 +5,12 @@ obj-$(CONFIG_SANDBOX) += bloblist.o obj-$(CONFIG_CMDLINE) += cmd/ obj-$(CONFIG_UNIT_TEST) += cmd_ut.o -obj-$(CONFIG_UNIT_TEST) += ut.o obj-$(CONFIG_SANDBOX) += command_ut.o obj-$(CONFIG_SANDBOX) += compression.o +obj-$(CONFIG_UNIT_TEST) += lib/ +obj-y += log/ obj-$(CONFIG_SANDBOX) += print_ut.o obj-$(CONFIG_SANDBOX) += str_ut.o obj-$(CONFIG_UT_TIME) += time_ut.o obj-$(CONFIG_UT_UNICODE) += unicode_ut.o -obj-y += log/ -obj-$(CONFIG_UNIT_TEST) += lib/ +obj-$(CONFIG_UNIT_TEST) += ut.o

Move everything into alphabetical order.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Applied to u-boot-dm, thanks!

At present most of the tests in test/Makefile are dependent on CONFIG_SANDBOX. But this is not ideal since they rely on commands being available and SPL does not support commands.
Use CONFIG_COMMAND instead. This has the dual purpose of allowing these tests to be used on other boards and allowing SPL to skip them.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/Makefile | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/test/Makefile b/test/Makefile index 13ee661f50e..73bddb4f61f 100644 --- a/test/Makefile +++ b/test/Makefile @@ -2,15 +2,15 @@ # # (C) Copyright 2012 The Chromium Authors
-obj-$(CONFIG_SANDBOX) += bloblist.o -obj-$(CONFIG_CMDLINE) += cmd/ -obj-$(CONFIG_UNIT_TEST) += cmd_ut.o -obj-$(CONFIG_SANDBOX) += command_ut.o -obj-$(CONFIG_SANDBOX) += compression.o +obj-$(CONFIG_$(SPL_)CMDLINE) += bloblist.o +obj-$(CONFIG_$(SPL_)CMDLINE) += cmd/ +obj-$(CONFIG_$(SPL_)CMDLINE) += cmd_ut.o +obj-$(CONFIG_$(SPL_)CMDLINE) += command_ut.o +obj-$(CONFIG_$(SPL_)CMDLINE) += compression.o obj-$(CONFIG_UNIT_TEST) += lib/ obj-y += log/ -obj-$(CONFIG_SANDBOX) += print_ut.o -obj-$(CONFIG_SANDBOX) += str_ut.o +obj-$(CONFIG_$(SPL_)CMDLINE) += print_ut.o +obj-$(CONFIG_$(SPL_)CMDLINE) += str_ut.o obj-$(CONFIG_UT_TIME) += time_ut.o obj-$(CONFIG_UT_UNICODE) += unicode_ut.o obj-$(CONFIG_UNIT_TEST) += ut.o

At present most of the tests in test/Makefile are dependent on CONFIG_SANDBOX. But this is not ideal since they rely on commands being available and SPL does not support commands.
Use CONFIG_COMMAND instead. This has the dual purpose of allowing these tests to be used on other boards and allowing SPL to skip them.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/Makefile | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Applied to u-boot-dm, thanks!

At present we always include test/dm from the main Makefile. We have a CONFIG_UNIT_TEST that should control whether the test/ directory is built, so rely on that instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
Makefile | 2 +- test/Makefile | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 2de0527e2dd..dbe7e7b75c9 100644 --- a/Makefile +++ b/Makefile @@ -806,7 +806,7 @@ libs-$(CONFIG_API) += api/ ifdef CONFIG_POST libs-y += post/ endif -libs-$(CONFIG_UNIT_TEST) += test/ test/dm/ +libs-$(CONFIG_UNIT_TEST) += test/ libs-$(CONFIG_UT_ENV) += test/env/ libs-$(CONFIG_UT_OPTEE) += test/optee/ libs-$(CONFIG_UT_OVERLAY) += test/overlay/ diff --git a/test/Makefile b/test/Makefile index 73bddb4f61f..ed3e882f7a7 100644 --- a/test/Makefile +++ b/test/Makefile @@ -9,8 +9,9 @@ obj-$(CONFIG_$(SPL_)CMDLINE) += command_ut.o obj-$(CONFIG_$(SPL_)CMDLINE) += compression.o obj-$(CONFIG_UNIT_TEST) += lib/ obj-y += log/ +obj-y += dm/ obj-$(CONFIG_$(SPL_)CMDLINE) += print_ut.o obj-$(CONFIG_$(SPL_)CMDLINE) += str_ut.o obj-$(CONFIG_UT_TIME) += time_ut.o obj-$(CONFIG_UT_UNICODE) += unicode_ut.o -obj-$(CONFIG_UNIT_TEST) += ut.o +obj-y += ut.o

At present we always include test/dm from the main Makefile. We have a CONFIG_UNIT_TEST that should control whether the test/ directory is built, so rely on that instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
Makefile | 2 +- test/Makefile | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)
Applied to u-boot-dm, thanks!

Tests are easier to run in U-Boot proper. Running them in SPL does not add test coverage in most cases. Also some tests use features that are not available in SPL.
Update the build rules to disable these tests in SPL. We still need test-main to be able to actually run SPL tests.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/Makefile | 9 ++++++--- test/dm/Makefile | 11 +++++++---- 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/test/Makefile b/test/Makefile index ed3e882f7a7..c8e554206a5 100644 --- a/test/Makefile +++ b/test/Makefile @@ -7,11 +7,14 @@ obj-$(CONFIG_$(SPL_)CMDLINE) += cmd/ obj-$(CONFIG_$(SPL_)CMDLINE) += cmd_ut.o obj-$(CONFIG_$(SPL_)CMDLINE) += command_ut.o obj-$(CONFIG_$(SPL_)CMDLINE) += compression.o -obj-$(CONFIG_UNIT_TEST) += lib/ -obj-y += log/ obj-y += dm/ +obj-y += log/ obj-$(CONFIG_$(SPL_)CMDLINE) += print_ut.o obj-$(CONFIG_$(SPL_)CMDLINE) += str_ut.o obj-$(CONFIG_UT_TIME) += time_ut.o -obj-$(CONFIG_UT_UNICODE) += unicode_ut.o obj-y += ut.o + +ifeq ($(CONFIG_SPL_BUILD),) +obj-$(CONFIG_UNIT_TEST) += lib/ +obj-$(CONFIG_$(SPL_)UT_UNICODE) += unicode_ut.o +endif diff --git a/test/dm/Makefile b/test/dm/Makefile index 70ba1b66953..387a4b81410 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -2,15 +2,16 @@ # # Copyright (c) 2013 Google, Inc
+obj-$(CONFIG_UT_DM) += test-main.o + +# Tests for particular subsystems - when enabling driver model for a new +# subsystem you must add sandbox tests here. +ifeq ($(CONFIG_SPL_BUILD),) obj-$(CONFIG_UT_DM) += bus.o -obj-$(CONFIG_UT_DM) += nop.o obj-$(CONFIG_UT_DM) += test-driver.o obj-$(CONFIG_UT_DM) += test-fdt.o -obj-$(CONFIG_UT_DM) += test-main.o obj-$(CONFIG_UT_DM) += test-uclass.o
-# Tests for particular subsystems - when enabling driver model for a new -# subsystem you must add sandbox tests here. obj-$(CONFIG_UT_DM) += core.o ifneq ($(CONFIG_SANDBOX),) obj-$(CONFIG_ACPIGEN) += acpi.o @@ -35,6 +36,7 @@ obj-$(CONFIG_LED) += led.o obj-$(CONFIG_DM_MAILBOX) += mailbox.o obj-$(CONFIG_DM_MMC) += mmc.o obj-y += fdtdec.o +obj-$(CONFIG_UT_DM) += nop.o obj-y += ofnode.o obj-y += ofread.o obj-$(CONFIG_OSD) += osd.o @@ -82,3 +84,4 @@ obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o obj-$(CONFIG_RESET_SYSCON) += syscon-reset.o obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o endif +endif # !SPL

Tests are easier to run in U-Boot proper. Running them in SPL does not add test coverage in most cases. Also some tests use features that are not available in SPL.
Update the build rules to disable these tests in SPL. We still need test-main to be able to actually run SPL tests.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/Makefile | 9 ++++++--- test/dm/Makefile | 11 +++++++---- 2 files changed, 13 insertions(+), 7 deletions(-)
Applied to u-boot-dm, thanks!

We want to run unit tests in SPL. Add a new Kconfig to control this and enable it for sandbox_spl
Signed-off-by: Simon Glass sjg@chromium.org ---
configs/sandbox_spl_defconfig | 2 +- scripts/Makefile.spl | 1 + test/Kconfig | 10 ++++++++++ 3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 6d8e827aebc..49060cab7a3 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -22,7 +22,6 @@ CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_SIZE=0x4096 CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000 -CONFIG_SILENT_CONSOLE=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_HANDOFF=y CONFIG_SPL_BOARD_INIT=y @@ -220,5 +219,6 @@ CONFIG_TPM=y CONFIG_LZ4=y CONFIG_ERRNO_STR=y CONFIG_UNIT_TEST=y +CONFIG_SPL_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index d528c994ff2..2e3a443035c 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -99,6 +99,7 @@ libs-y += dts/ libs-y += fs/ libs-$(CONFIG_SPL_POST_MEM_SUPPORT) += post/drivers/ libs-$(CONFIG_SPL_NET_SUPPORT) += net/ +libs-$(CONFIG_SPL_UNIT_TEST) += test/
head-y := $(addprefix $(obj)/,$(head-y)) libs-y := $(addprefix $(obj)/,$(libs-y)) diff --git a/test/Kconfig b/test/Kconfig index 28704a25b61..2646e7d825a 100644 --- a/test/Kconfig +++ b/test/Kconfig @@ -6,6 +6,16 @@ menuconfig UNIT_TEST This does not require sandbox to be included, but it is most often used there.
+config SPL_UNIT_TEST + bool "Unit tests in SPL" + # We need to be able to unbind devices for tests to work + select SPL_DM_DEVICE_REMOVE + help + Select this to enable unit tests in SPL. Most test are designed for + running in U-Boot proper, but some are intended for SPL, such as + of-platdata and SPL handover. To run these tests with the sandbox_spl + board, use the -u (unit test) option. + config UT_LIB bool "Unit tests for library functions" depends on UNIT_TEST

At present DM tests assume that a devicetree is available. This is not the case with of-platadata.
Update the code to add this condition.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/dm/test-main.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/test/dm/test-main.c b/test/dm/test-main.c index 995988723ae..5560572c7c6 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-main.c @@ -30,7 +30,8 @@ static int dm_test_init(struct unit_test_state *uts, bool of_live)
memset(dms, '\0', sizeof(*dms)); gd->dm_root = NULL; - memset(dm_testdrv_op_count, '\0', sizeof(dm_testdrv_op_count)); + if (!CONFIG_IS_ENABLED(OF_PLATDATA)) + memset(dm_testdrv_op_count, '\0', sizeof(dm_testdrv_op_count)); state_reset_for_test(state_get_current());
/* Determine whether to make the live tree available */ @@ -91,7 +92,8 @@ static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, ut_assertok(dm_scan_platdata(false)); if (test->flags & UT_TESTF_PROBE_TEST) ut_assertok(do_autoprobe(uts)); - if (test->flags & UT_TESTF_SCAN_FDT) + if (!CONFIG_IS_ENABLED(OF_PLATDATA) && + (test->flags & UT_TESTF_SCAN_FDT)) ut_assertok(dm_extended_scan_fdt(gd->fdt_blob, false));
/* @@ -136,14 +138,16 @@ static int dm_test_main(const char *test_name) uts->priv = &_global_priv_dm_test_state; uts->fail_count = 0;
- /* - * If we have no device tree, or it only has a root node, then these - * tests clearly aren't going to work... - */ - if (!gd->fdt_blob || fdt_next_node(gd->fdt_blob, 0, NULL) < 0) { - puts("Please run with test device tree:\n" - " ./u-boot -d arch/sandbox/dts/test.dtb\n"); - ut_assert(gd->fdt_blob); + if (!CONFIG_IS_ENABLED(OF_PLATDATA)) { + /* + * If we have no device tree, or it only has a root node, then + * these * tests clearly aren't going to work... + */ + if (!gd->fdt_blob || fdt_next_node(gd->fdt_blob, 0, NULL) < 0) { + puts("Please run with test device tree:\n" + " ./u-boot -d arch/sandbox/dts/test.dtb\n"); + ut_assert(gd->fdt_blob); + } }
if (!test_name) @@ -192,7 +196,8 @@ static int dm_test_main(const char *test_name) gd->dm_root = NULL; ut_assertok(dm_init(CONFIG_IS_ENABLED(OF_LIVE))); dm_scan_platdata(false); - dm_scan_fdt(gd->fdt_blob, false); + if (!CONFIG_IS_ENABLED(OF_PLATDATA)) + dm_scan_fdt(gd->fdt_blob, false);
return uts->fail_count ? CMD_RET_FAILURE : 0; }

Add a -u flag for U-Boot SPL which requests that unit tests be run. To make this work, export dm_test_main() and update it to skip test features that are not used with of-platdata.
To run the tests:
$ spl/u-boot-spl -u U-Boot SPL 2020.10-rc5 (Oct 01 2020 - 07:35:39 -0600) Running 0 driver model tests Failures: 0
At present there are no SPL unit tests.
Note that there is one wrinkle with these tests. SPL has limited memory available for allocation. Also malloc_simple does not free memory (free() is a nop) and running tests repeatedly causes driver-model to reinit multiple times and allocate memory. Therefore it is not possible to run more than a few tests at a time. One solution is to increase the amount of malloc space in sandbox_spl. This is not a problem for pytest, since it runs each test individually, so for now this is left as is.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/spl.c | 8 ++++++++ arch/sandbox/cpu/start.c | 9 +++++++++ arch/sandbox/include/asm/state.h | 1 + include/test/test.h | 11 +++++++++++ test/dm/test-main.c | 2 +- 5 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c index 7ab8919eb90..48fd1265afe 100644 --- a/arch/sandbox/cpu/spl.c +++ b/arch/sandbox/cpu/spl.c @@ -12,6 +12,7 @@ #include <spl.h> #include <asm/spl.h> #include <asm/state.h> +#include <test/test.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -67,6 +68,13 @@ void spl_board_init(void) uclass_next_device(&dev)) ; } + + if (state->run_unittests) { + int ret; + + ret = dm_test_main(NULL); + /* continue execution into U-Boot */ + } }
void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image) diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index c6a2bbe4689..f5e104b127b 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -374,6 +374,15 @@ static int sandbox_cmdline_cb_show_of_platdata(struct sandbox_state *state, } SANDBOX_CMDLINE_OPT(show_of_platdata, 0, "Show of-platdata in SPL");
+static int sandbox_cmdline_cb_unittests(struct sandbox_state *state, + const char *arg) +{ + state->run_unittests = true; + + return 0; +} +SANDBOX_CMDLINE_OPT_SHORT(unittests, 'u', 0, "Run unit tests"); + static void setup_ram_buf(struct sandbox_state *state) { /* Zero the RAM buffer if we didn't read it, to keep valgrind happy */ diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index 1bfad305f1a..f828d9d2447 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -92,6 +92,7 @@ struct sandbox_state { int default_log_level; /* Default log level for sandbox */ bool show_of_platdata; /* Show of-platdata in SPL */ bool ram_buf_read; /* true if we read the RAM buffer */ + bool run_unittests; /* Run unit tests */
/* Pointer to information for each SPI bus/cs */ struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS] diff --git a/include/test/test.h b/include/test/test.h index 67c7d69d488..03e29290bf4 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -94,4 +94,15 @@ enum { TEST_DEVRES_SIZE3 = 37, };
+/** + * dm_test_main() - Run driver model tests + * + * Run all the available driver model tests, or a selection + * + * @test_name: Name of single test to run (e.g. "dm_test_fdt_pre_reloc" or just + * "fdt_pre_reloc"), or NULL to run all + * @return 0 if all tests passed, 1 if not + */ +int dm_test_main(const char *test_name); + #endif /* __TEST_TEST_H */ diff --git a/test/dm/test-main.c b/test/dm/test-main.c index 5560572c7c6..9d22df8c4dc 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-main.c @@ -127,7 +127,7 @@ static bool dm_test_run_on_flattree(struct unit_test *test) return !strstr(fname, "video") || strstr(test->name, "video_base"); }
-static int dm_test_main(const char *test_name) +int dm_test_main(const char *test_name) { struct unit_test *tests = ll_entry_start(struct unit_test, dm_test); const int n_ents = ll_entry_count(struct unit_test, dm_test);

At present we have a pytest that covers of-platadata. Add a very simple unit test that just checks that a device can be found. This shows the ability to write these tests in C.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/dm/Makefile | 4 +++- test/dm/of_platdata.c | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 test/dm/of_platdata.c
diff --git a/test/dm/Makefile b/test/dm/Makefile index 387a4b81410..620cdc5f2b4 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -6,7 +6,9 @@ obj-$(CONFIG_UT_DM) += test-main.o
# Tests for particular subsystems - when enabling driver model for a new # subsystem you must add sandbox tests here. -ifeq ($(CONFIG_SPL_BUILD),) +ifeq ($(CONFIG_SPL_BUILD),y) +obj-$(CONFIG_SPL_OF_PLATDATA) += of_platdata.o +else obj-$(CONFIG_UT_DM) += bus.o obj-$(CONFIG_UT_DM) += test-driver.o obj-$(CONFIG_UT_DM) += test-fdt.o diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c new file mode 100644 index 00000000000..7a864eb0fb3 --- /dev/null +++ b/test/dm/of_platdata.c @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: GPL-2.0+ + +#include <common.h> +#include <dm.h> +#include <dm/test.h> +#include <test/test.h> +#include <test/ut.h> + +/* Test that we can find a device using of-platdata */ +static int dm_test_of_platdata_base(struct unit_test_state *uts) +{ + struct udevice *dev; + + ut_assertok(uclass_first_device_err(UCLASS_SERIAL, &dev)); + ut_asserteq_str("sandbox_serial", dev->name); + + return 0; +} +DM_TEST(dm_test_of_platdata_base, UT_TESTF_SCAN_PDATA);

Add a rule to generate u-boot-spl.sym so that pytest can discover the available unit tests.
Signed-off-by: Simon Glass sjg@chromium.org ---
scripts/Makefile.spl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 2e3a443035c..9f1f7445d71 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -214,7 +214,7 @@ spl/boot.bin: $(obj)/$(SPL_BIN)-align.bin FORCE $(call if_changed,mkimage) endif
-INPUTS-y += $(obj)/$(SPL_BIN).bin +INPUTS-y += $(obj)/$(SPL_BIN).bin $(obj)/$(SPL_BIN).sym
ifdef CONFIG_SAMSUNG INPUTS-y += $(obj)/$(BOARD)-spl.bin @@ -408,6 +408,11 @@ MKIMAGEFLAGS_u-boot-spl-mtk.bin = -T mtk_image \ $(obj)/u-boot-spl-mtk.bin: $(obj)/u-boot-spl.bin FORCE $(call if_changed,mkimage)
+quiet_cmd_sym ?= SYM $@ + cmd_sym ?= $(OBJDUMP) -t $< > $@ +$(obj)/$(SPL_BIN).sym: $(obj)/$(SPL_BIN) FORCE + $(call if_changed,sym) + # Rule to link u-boot-spl # May be overridden by arch/$(ARCH)/config.mk quiet_cmd_u-boot-spl ?= LD $@

Add a new test_spl fixture to handle running SPL unit tests.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/py/conftest.py | 14 +++++++++----- test/py/tests/test_spl.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 test/py/tests/test_spl.py
diff --git a/test/py/conftest.py b/test/py/conftest.py index 30920474b36..28fde347c00 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -227,7 +227,7 @@ def pytest_configure(config): console = u_boot_console_exec_attach.ConsoleExecAttach(log, ubconfig)
re_ut_test_list = re.compile(r'_u_boot_list_2_(.*)_test_2_\1_test_(.*)\s*$') -def generate_ut_subtest(metafunc, fixture_name): +def generate_ut_subtest(metafunc, fixture_name, sym_path): """Provide parametrization for a ut_subtest fixture.
Determines the set of unit tests built into a U-Boot binary by parsing the @@ -237,12 +237,13 @@ def generate_ut_subtest(metafunc, fixture_name): Args: metafunc: The pytest test function. fixture_name: The fixture name to test. + sym_path: Relative path to the symbol file with preceding '/' + (e.g. '/u-boot.sym')
Returns: Nothing. """ - - fn = console.config.build_dir + '/u-boot.sym' + fn = console.config.build_dir + sym_path try: with open(fn, 'rt') as f: lines = f.readlines() @@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc): Returns: Nothing. """ - + #print('name', metafunc.fixturenames) for fn in metafunc.fixturenames: if fn == 'ut_subtest': - generate_ut_subtest(metafunc, fn) + generate_ut_subtest(metafunc, fn, '/u-boot.sym') + continue + if fn == 'ut_spl_subtest': + generate_ut_subtest(metafunc, fn, '/spl/u-boot-spl.sym') continue generate_config(metafunc, fn)
diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py new file mode 100644 index 00000000000..58a851e5ec8 --- /dev/null +++ b/test/py/tests/test_spl.py @@ -0,0 +1,29 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright 2020 Google LLC +# Written by Simon Glass sjg@chromium.org + +import os.path +import pytest + +def test_spl(u_boot_console, ut_spl_subtest): + """Execute a "ut" subtest. + + The subtests are collected in function generate_ut_subtest() from linker + generated lists by applying a regular expression to the lines of file + spl/u-boot-spl.sym. The list entries are created using the C macro + UNIT_TEST(). + + Strict naming conventions have to be followed to match the regular + expression. Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in + test suite foo that can be executed via command 'ut foo bar' and is + implemented in C function foo_test_bar(). + + Args: + u_boot_console (ConsoleBase): U-Boot console + ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to + execute command 'ut foo bar' + """ + cons = u_boot_console + cons.restart_uboot_with_flags(['-u', ut_spl_subtest]) + output = cons.get_spawn_output().replace('\r', '') + assert 'Failures: 0' in output

On 10/3/20 9:25 AM, Simon Glass wrote:
Add a new test_spl fixture to handle running SPL unit tests.
diff --git a/test/py/conftest.py b/test/py/conftest.py
@@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc): Returns: Nothing. """
- #print('name', metafunc.fixturenames)
Revert that debug change?
diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py
- cons.restart_uboot_with_flags(['-u', ut_spl_subtest])
How is that change reverted when the test runs, so that subsequent tests are run on the main U-Boot rather than this restarted U-Boot?
It feels like it'd be better to start a separate top-level test run for this purpose, rather than swap out the U-Boot process in the middle of a test run.

Hi Stephen,
On Mon, 5 Oct 2020 at 13:39, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/3/20 9:25 AM, Simon Glass wrote:
Add a new test_spl fixture to handle running SPL unit tests.
diff --git a/test/py/conftest.py b/test/py/conftest.py
@@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc): Returns: Nothing. """
- #print('name', metafunc.fixturenames)
Revert that debug change?
Will do.
diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py
- cons.restart_uboot_with_flags(['-u', ut_spl_subtest])
How is that change reverted when the test runs, so that subsequent tests are run on the main U-Boot rather than this restarted U-Boot?
Well actually at the moment it just continues into U-Boot. It will mostly pass the tests, but probably not all of them.
It feels like it'd be better to start a separate top-level test run for this purpose, rather than swap out the U-Boot process in the middle of a test run.
I was hoping that the fixture stuff would take care of that. How would I do a separate top-level test run?
Regards, Simon

On 10/5/20 1:51 PM, Simon Glass wrote:
Hi Stephen,
On Mon, 5 Oct 2020 at 13:39, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/3/20 9:25 AM, Simon Glass wrote:
Add a new test_spl fixture to handle running SPL unit tests.
diff --git a/test/py/conftest.py b/test/py/conftest.py
@@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc): Returns: Nothing. """
- #print('name', metafunc.fixturenames)
Revert that debug change?
Will do.
diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py
- cons.restart_uboot_with_flags(['-u', ut_spl_subtest])
How is that change reverted when the test runs, so that subsequent tests are run on the main U-Boot rather than this restarted U-Boot?
Well actually at the moment it just continues into U-Boot. It will mostly pass the tests, but probably not all of them.
Looking at existing tests, there are quite a few that do this:
try: test body which might do something nasty to U-Boot finally: u_boot_console.restart_uboot()
... so that might be applicable.
It feels like it'd be better to start a separate top-level test run for this purpose, rather than swap out the U-Boot process in the middle of a test run.
I was hoping that the fixture stuff would take care of that. How would I do a separate top-level test run?
That'd simply be just running test/py/test.py and passing it the relevant U-Boot binary/args rather than the main binary. I assume you'd want to pass relevant -k option to restrict the set of tests run to something relevant, and an appropriate --build-dir option to point at the binary.

Hi Stephen,
On Mon, 5 Oct 2020 at 15:35, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/5/20 1:51 PM, Simon Glass wrote:
Hi Stephen,
On Mon, 5 Oct 2020 at 13:39, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/3/20 9:25 AM, Simon Glass wrote:
Add a new test_spl fixture to handle running SPL unit tests.
diff --git a/test/py/conftest.py b/test/py/conftest.py
@@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc): Returns: Nothing. """
- #print('name', metafunc.fixturenames)
Revert that debug change?
Will do.
diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py
- cons.restart_uboot_with_flags(['-u', ut_spl_subtest])
How is that change reverted when the test runs, so that subsequent tests are run on the main U-Boot rather than this restarted U-Boot?
Well actually at the moment it just continues into U-Boot. It will mostly pass the tests, but probably not all of them.
Looking at existing tests, there are quite a few that do this:
try: test body which might do something nasty to U-Boot finally: u_boot_console.restart_uboot()
... so that might be applicable.
It feels like it'd be better to start a separate top-level test run for this purpose, rather than swap out the U-Boot process in the middle of a test run.
I was hoping that the fixture stuff would take care of that. How would I do a separate top-level test run?
That'd simply be just running test/py/test.py and passing it the relevant U-Boot binary/args rather than the main binary. I assume you'd want to pass relevant -k option to restrict the set of tests run to something relevant, and an appropriate --build-dir option to point at the binary.
I'm fiddling with this a bit. I do already have a separate run of the SPL tests using the mechanism you describe here, in test/run :
# Run tests which require sandbox_spl run_test "sandbox_spl" ./test/py/test.py --bd sandbox_spl --build \ -k 'test_ofplatdata or test_handoff or test_spl'
Also, the test_spl() collection function in this patch is used to read out tests from spl/u-boot-spl and these tests are not present in u-boot since they are only built for SPL (Makefile ensures this).
So yes I can add a try...finally thing, but in fact all that does is make every SPL test be followed by a reboot into U-Boot proper, for no purpose.
Could you please have another look at this?
Regards, Simon

On 10/23/20 1:27 PM, Simon Glass wrote:
Hi Stephen,
On Mon, 5 Oct 2020 at 15:35, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/5/20 1:51 PM, Simon Glass wrote:
Hi Stephen,
On Mon, 5 Oct 2020 at 13:39, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/3/20 9:25 AM, Simon Glass wrote:
Add a new test_spl fixture to handle running SPL unit tests.
diff --git a/test/py/conftest.py b/test/py/conftest.py
@@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc): Returns: Nothing. """
- #print('name', metafunc.fixturenames)
Revert that debug change?
Will do.
diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py
- cons.restart_uboot_with_flags(['-u', ut_spl_subtest])
How is that change reverted when the test runs, so that subsequent tests are run on the main U-Boot rather than this restarted U-Boot?
Well actually at the moment it just continues into U-Boot. It will mostly pass the tests, but probably not all of them.
Looking at existing tests, there are quite a few that do this:
try: test body which might do something nasty to U-Boot finally: u_boot_console.restart_uboot()
... so that might be applicable.
It feels like it'd be better to start a separate top-level test run for this purpose, rather than swap out the U-Boot process in the middle of a test run.
I was hoping that the fixture stuff would take care of that. How would I do a separate top-level test run?
That'd simply be just running test/py/test.py and passing it the relevant U-Boot binary/args rather than the main binary. I assume you'd want to pass relevant -k option to restrict the set of tests run to something relevant, and an appropriate --build-dir option to point at the binary.
I'm fiddling with this a bit. I do already have a separate run of the SPL tests using the mechanism you describe here, in test/run :
# Run tests which require sandbox_spl run_test "sandbox_spl" ./test/py/test.py --bd sandbox_spl --build \ -k 'test_ofplatdata or test_handoff or test_spl'
Also, the test_spl() collection function in this patch is used to read out tests from spl/u-boot-spl and these tests are not present in u-boot since they are only built for SPL (Makefile ensures this).
So yes I can add a try...finally thing, but in fact all that does is make every SPL test be followed by a reboot into U-Boot proper, for no purpose.
Why do you say for no purpose? The rest of the tests expect to test U-Boot proper, or at least whatever U-Boot binary the test system was invoked against. If one test switches to a different U-Boot binary than the user requests to test, and doesn't switch back, that means the user isn't getting what they asked for. Perhaps you can explain your use-case a bit more?
In general, the philosophy for testing multiple U-Boot binaries should be to run different top-level test invocations on each binary, and each top-level test invocation should run all relevant tests against that one binary. If some test absolutely has to switch to a different binary, then it has to switch back to the "real" binary after it runs to avoid the other tests running on a different binary than the user requested. Note that binary switching is only possible for sandbox; when running on real targets, the flashing step is a one-time thing, not something that happens when the target is reset.
Could you please have another look at this?
Regards, Simon

Hi Stephen,
On Mon, 26 Oct 2020 at 09:34, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/23/20 1:27 PM, Simon Glass wrote:
Hi Stephen,
On Mon, 5 Oct 2020 at 15:35, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/5/20 1:51 PM, Simon Glass wrote:
Hi Stephen,
On Mon, 5 Oct 2020 at 13:39, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/3/20 9:25 AM, Simon Glass wrote:
Add a new test_spl fixture to handle running SPL unit tests.
diff --git a/test/py/conftest.py b/test/py/conftest.py
@@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc): Returns: Nothing. """
- #print('name', metafunc.fixturenames)
Revert that debug change?
Will do.
diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py
- cons.restart_uboot_with_flags(['-u', ut_spl_subtest])
How is that change reverted when the test runs, so that subsequent tests are run on the main U-Boot rather than this restarted U-Boot?
Well actually at the moment it just continues into U-Boot. It will mostly pass the tests, but probably not all of them.
Looking at existing tests, there are quite a few that do this:
try: test body which might do something nasty to U-Boot finally: u_boot_console.restart_uboot()
... so that might be applicable.
It feels like it'd be better to start a separate top-level test run for this purpose, rather than swap out the U-Boot process in the middle of a test run.
I was hoping that the fixture stuff would take care of that. How would I do a separate top-level test run?
That'd simply be just running test/py/test.py and passing it the relevant U-Boot binary/args rather than the main binary. I assume you'd want to pass relevant -k option to restrict the set of tests run to something relevant, and an appropriate --build-dir option to point at the binary.
I'm fiddling with this a bit. I do already have a separate run of the SPL tests using the mechanism you describe here, in test/run :
# Run tests which require sandbox_spl run_test "sandbox_spl" ./test/py/test.py --bd sandbox_spl --build \ -k 'test_ofplatdata or test_handoff or test_spl'
Also, the test_spl() collection function in this patch is used to read out tests from spl/u-boot-spl and these tests are not present in u-boot since they are only built for SPL (Makefile ensures this).
So yes I can add a try...finally thing, but in fact all that does is make every SPL test be followed by a reboot into U-Boot proper, for no purpose.
Why do you say for no purpose? The rest of the tests expect to test U-Boot proper, or at least whatever U-Boot binary the test system was invoked against. If one test switches to a different U-Boot binary than the user requests to test, and doesn't switch back, that means the user isn't getting what they asked for. Perhaps you can explain your use-case a bit more?
I sent a v2 which updates it as you suggest.
My point is that the SPL tests are run separately. This works by your pytest harness starting u-boot-spl (which runs the tests) and then u-boot-spl continues to boot into U-Boot proper. It would of course be possible to quit from SPL, but quitting U-Boot of course confuses pytest.
If you look at test/run you can see that I added a separate pytest invocation for the SPL tests. Since sandbox_spl has different options enabled (and does not use test.dts) it can't currently run some of the tests in the suite. There is really no point anyway since those tests already run with the normal and flattree sandbox.
Just to be clear, sandbox and sandbox_spl are somewhat different. By running sandbox_spl with pytest we are precluded from running many of the sandbox tests.
In general, the philosophy for testing multiple U-Boot binaries should be to run different top-level test invocations on each binary, and each top-level test invocation should run all relevant tests against that one binary. If some test absolutely has to switch to a different binary, then it has to switch back to the "real" binary after it runs to avoid the other tests running on a different binary than the user requested. Note that binary switching is only possible for sandbox; when running on real targets, the flashing step is a one-time thing, not something that happens when the target is reset.
Could you please have another look at this?
Yes sandbox can switch binaries but it is not quite as flexible as you suggest here. We are using a single build directory in pytest, so we have to run one of the binaries in that directory. We can run u-boot-spl but then of course it jumps into 'u-boot' after starting up. But the 'u-boot' it jumps to is the sandbox_spl one, not the sandbox oneee
Anyway I added the try...finally clause as you suggested in v2 and sent it out, so I suppose we are covered both ways.
Regards, Simon

Update the 'run' script to include SPL unit tests.
Signed-off-by: Simon Glass sjg@chromium.org ---
test/run | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/run b/test/run index de87e7530b1..735628e7e37 100755 --- a/test/run +++ b/test/run @@ -28,7 +28,7 @@ fi
# Run tests which require sandbox_spl run_test "sandbox_spl" ./test/py/test.py --bd sandbox_spl --build \ - -k 'test_ofplatdata or test_handoff' + -k 'test_ofplatdata or test_handoff or test_spl'
if [ -z "$tools_only" ]; then # Run tests for the flat-device-tree version of sandbox. This is a special

Run SPL unit tests in all test environments.
Signed-off-by: Simon Glass sjg@chromium.org ---
.azure-pipelines.yml | 2 +- .gitlab-ci.yml | 2 +- .travis.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 473ddee3835..a78c8d61300 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -182,7 +182,7 @@ jobs: OVERRIDE: "-O clang-10" sandbox_spl: TEST_PY_BD: "sandbox_spl" - TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff" + TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl" sandbox_flattree: TEST_PY_BD: "sandbox_flattree" evb_ast2500: diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9ac2b336a11..b1e0b3bc9dd 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -198,7 +198,7 @@ sandbox_spl test.py: tags: [ 'all' ] variables: TEST_PY_BD: "sandbox_spl" - TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff" + TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl" <<: *buildman_and_testpy_dfn
evb-ast2500 test.py: diff --git a/.travis.yml b/.travis.yml index fb8f73157d7..cb48ff3023a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -512,7 +512,7 @@ matrix: - name: "test/py sandbox_spl" env: - TEST_PY_BD="sandbox_spl" - TEST_PY_TEST_SPEC="test_ofplatdata or test_handoff" + TEST_PY_TEST_SPEC="test_ofplatdata or test_handoff or test_spl" TOOLCHAIN="i386" TEST_PY_TOOLS="yes" - name: "test/py sandbox_flattree"
participants (2)
-
Simon Glass
-
Stephen Warren