[PATCH v2] bdinfo: Correct use of assertions

[PATCH v2] bdinfo: Correct use of assertions

[Simon Glass]

This test was written for the incorrect use of assertions. Update it to
build with the previous approach, where tests fail at the first
assertion.

All assertion functions return 0 on success and non-zero on failure.
They can be nested into functions simply by declaring a function that
returns an int and using ut_assertok() to call it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Expand the commit message to be clearer

 test/cmd/bdinfo.c | 79 +++++++++++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 30 deletions(-)

diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
index 9068df79c4f2..cddf1a46d491 100644
--- a/test/cmd/bdinfo.c
+++ b/test/cmd/bdinfo.c
@@ -27,19 +27,25 @@ DECLARE_GLOBAL_DATA_PTR;
 /* Declare a new bdinfo test */
 #define BDINFO_TEST(_name, _flags)	UNIT_TEST(_name, _flags, bdinfo_test)
 
-static void bdinfo_test_num_l(struct unit_test_state *uts,
-			      const char *name, ulong value)
+static int test_num_l(struct unit_test_state *uts, const char *name,
+		      ulong value)
 {
-	ut_assert_nextline("%-12s= 0x%0*lx", name, 2 * (int)sizeof(value), value);
+	ut_assert_nextline("%-12s= 0x%0*lx", name, 2 * (int)sizeof(value),
+			   value);
+
+	return 0;
 }
 
-static void bdinfo_test_num_ll(struct unit_test_state *uts,
-			       const char *name, unsigned long long value)
+static int test_num_ll(struct unit_test_state *uts, const char *name,
+		       unsigned long long value)
 {
-	ut_assert_nextline("%-12s= 0x%.*llx", name, 2 * (int)sizeof(ulong), value);
+	ut_assert_nextline("%-12s= 0x%.*llx", name, 2 * (int)sizeof(ulong),
+			   value);
+
+	return 0;
 }
 
-static void test_eth(struct unit_test_state *uts)
+static int test_eth(struct unit_test_state *uts)
 {
 	const int idx = eth_get_dev_index();
 	uchar enetaddr[6];
@@ -59,9 +65,11 @@ static void test_eth(struct unit_test_state *uts)
 	else
 		ut_assert_nextline("%-12s= %pM", name, enetaddr);
 	ut_assert_nextline("IP addr     = %s", env_get("ipaddr"));
+
+	return 0;
 }
 
-static void test_video_info(struct unit_test_state *uts)
+static int test_video_info(struct unit_test_state *uts)
 {
 	const struct udevice *dev;
 	struct uclass *uc;
@@ -73,22 +81,25 @@ static void test_video_info(struct unit_test_state *uts)
 			struct video_priv *upriv = dev_get_uclass_priv(dev);
 			struct video_uc_plat *plat = dev_get_uclass_plat(dev);
 
-			bdinfo_test_num_ll(uts, "FB base", (ulong)upriv->fb);
+			ut_assertok(test_num_ll(uts, "FB base",
+						(ulong)upriv->fb));
 			if (upriv->copy_fb) {
-				bdinfo_test_num_ll(uts, "FB copy",
-						   (ulong)upriv->copy_fb);
-				bdinfo_test_num_l(uts, " copy size",
-						  plat->copy_size);
+				ut_assertok(test_num_ll(uts, "FB copy",
+							(ulong)upriv->copy_fb));
+				ut_assertok(test_num_l(uts, " copy size",
+						       plat->copy_size));
 			}
 			ut_assert_nextline("%-12s= %dx%dx%d", "FB size",
 					   upriv->xsize, upriv->ysize,
 					   1 << upriv->bpix);
 		}
 	}
+
+	return 0;
 }
 
-static void lmb_test_dump_region(struct unit_test_state *uts,
-				 struct lmb_region *rgn, char *name)
+static int lmb_test_dump_region(struct unit_test_state *uts,
+				struct lmb_region *rgn, char *name)
 {
 	unsigned long long base, size, end;
 	enum lmb_flags flags;
@@ -105,13 +116,17 @@ static void lmb_test_dump_region(struct unit_test_state *uts,
 		ut_assert_nextline(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes flags: %x",
 				   name, i, base, end, size, flags);
 	}
+
+	return 0;
 }
 
-static void lmb_test_dump_all(struct unit_test_state *uts, struct lmb *lmb)
+static int lmb_test_dump_all(struct unit_test_state *uts, struct lmb *lmb)
 {
 	ut_assert_nextline("lmb_dump_all:");
 	lmb_test_dump_region(uts, &lmb->memory, "memory");
 	lmb_test_dump_region(uts, &lmb->reserved, "reserved");
+
+	return 0;
 }
 
 static int bdinfo_test_move(struct unit_test_state *uts)
@@ -123,44 +138,48 @@ static int bdinfo_test_move(struct unit_test_state *uts)
 	ut_assertok(console_record_reset_enable());
 	ut_assertok(run_commandf("bdinfo"));
 
-	bdinfo_test_num_l(uts, "boot_params", 0);
+	ut_assertok(test_num_l(uts, "boot_params", 0));
 
 	for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
 		if (bd->bi_dram[i].size) {
-			bdinfo_test_num_l(uts, "DRAM bank",  i);
-			bdinfo_test_num_ll(uts, "-> start", bd->bi_dram[i].start);
-			bdinfo_test_num_ll(uts, "-> size", bd->bi_dram[i].size);
+			ut_assertok(test_num_l(uts, "DRAM bank", i));
+			ut_assertok(test_num_ll(uts, "-> start",
+						bd->bi_dram[i].start));
+			ut_assertok(test_num_ll(uts, "-> size",
+						bd->bi_dram[i].size));
 		}
 	}
 
 	/* CONFIG_SYS_HAS_SRAM testing not supported */
-	bdinfo_test_num_l(uts, "flashstart", 0);
-	bdinfo_test_num_l(uts, "flashsize", 0);
-	bdinfo_test_num_l(uts, "flashoffset", 0);
+	ut_assertok(test_num_l(uts, "flashstart", 0));
+	ut_assertok(test_num_l(uts, "flashsize", 0));
+	ut_assertok(test_num_l(uts, "flashoffset", 0));
 	ut_assert_nextline("baudrate    = %lu bps",
 			   env_get_ulong("baudrate", 10, 1234));
-	bdinfo_test_num_l(uts, "relocaddr", gd->relocaddr);
-	bdinfo_test_num_l(uts, "reloc off", gd->reloc_off);
+	ut_assertok(test_num_l(uts, "relocaddr", gd->relocaddr));
+	ut_assertok(test_num_l(uts, "reloc off", gd->reloc_off));
 	ut_assert_nextline("%-12s= %u-bit", "Build", (uint)sizeof(void *) * 8);
 
 	if (IS_ENABLED(CONFIG_CMD_NET))
-		test_eth(uts);
+		ut_assertok(test_eth(uts));
 
 	/*
 	 * Make sure environment variable "fdtcontroladdr" address
 	 * matches mapped control DT address.
 	 */
 	ut_assert(map_to_sysmem(gd->fdt_blob) == env_get_hex("fdtcontroladdr", 0x1234));
-	bdinfo_test_num_l(uts, "fdt_blob", (ulong)map_to_sysmem(gd->fdt_blob));
-	bdinfo_test_num_l(uts, "new_fdt", (ulong)map_to_sysmem(gd->new_fdt));
-	bdinfo_test_num_l(uts, "fdt_size", (ulong)gd->fdt_size);
+	ut_assertok(test_num_l(uts, "fdt_blob",
+			       (ulong)map_to_sysmem(gd->fdt_blob)));
+	ut_assertok(test_num_l(uts, "new_fdt",
+			       (ulong)map_to_sysmem(gd->new_fdt)));
+	ut_assertok(test_num_l(uts, "fdt_size", (ulong)gd->fdt_size));
 
 	if (IS_ENABLED(CONFIG_VIDEO))
 		test_video_info(uts);
 
 	/* The gd->multi_dtb_fit may not be available, hence, #if below. */
 #if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
-	bdinfo_test_num_l(uts, "multi_dtb_fit", (ulong)gd->multi_dtb_fit);
+	ut_assertok(test_num_l(uts, "multi_dtb_fit", (ulong)gd->multi_dtb_fit));
 #endif
 
 	if (IS_ENABLED(CONFIG_LMB) && gd->fdt_blob) {
-- 
2.41.0.390.g38632f3daf-goog

Re: [PATCH v2] bdinfo: Correct use of assertions

[Marek Vasut]

On 7/12/23 04:46, Simon Glass wrote:
> This test was written for the incorrect use of assertions. Update it to
> build with the previous approach, where tests fail at the first
> assertion.
> 
> All assertion functions return 0 on success and non-zero on failure.
> They can be nested into functions simply by declaring a function that
> returns an int and using ut_assertok() to call it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Marek Vasut <marex@denx.de>

Thanks

Re: [PATCH v2] bdinfo: Correct use of assertions

[Tom Rini]

On Tue, 11 Jul 2023 20:46:30 -0600, Simon Glass wrote:

> This test was written for the incorrect use of assertions. Update it to
> build with the previous approach, where tests fail at the first
> assertion.
> 
> All assertion functions return 0 on success and non-zero on failure.
> They can be nested into functions simply by declaring a function that
> returns an int and using ut_assertok() to call it.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom