[PATCH v2 3/3] test: Add test for partitions

[Sean Anderson]

On 4/21/21 3:14 AM, Simon Glass wrote:
 > Hi Sean,
 >
 > On Fri, 16 Apr 2021 at 02:36, Sean Anderson <sean.anderson at seco.com> wrote:
 >>
 >>
 >>
 >> On 4/14/21 3:37 PM, Simon Glass wrote:
 >>   > Hi Sean,
 >>   >
 >>   > On Mon, 12 Apr 2021 at 23:53, Sean Anderson <sean.anderson at seco.com> wrote:
 >>   >>
 >>   >> This is technically a library function, but we use MMCs for testing, so
 >>   >> it is easier to do it with DM. At the moment, the only block devices in
 >>   >> sandbox are MMCs (AFAIK) so we just test with those.
 >>   >>
 >>   >> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
 >>   >> ---
 >>   >>
 >>   >> Changes in v2:
 >>   >> - New
 >>   >>
 >>   >>   test/dm/Makefile |  1 +
 >>   >>   test/dm/part.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
 >>   >>   2 files changed, 77 insertions(+)
 >>   >>   create mode 100644 test/dm/part.c
 >>   >>
 >>   >> diff --git a/test/dm/Makefile b/test/dm/Makefile
 >>   >> index f5cc5540e8..7d017f8750 100644
 >>   >> --- a/test/dm/Makefile
 >>   >> +++ b/test/dm/Makefile
 >>   >> @@ -98,5 +98,6 @@ endif
 >>   >>   ifneq ($(CONFIG_EFI_PARTITION),)
 >>   >>   obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o
 >>   >>   endif
 >>   >> +obj-$(CONFIG_EFI_PARTITION) += part.o
 >>   >>   endif
 >>   >>   endif # !SPL
 >>   >> diff --git a/test/dm/part.c b/test/dm/part.c
 >>   >> new file mode 100644
 >>   >> index 0000000000..051e9010b6
 >>   >> --- /dev/null
 >>   >> +++ b/test/dm/part.c
 >>   >> @@ -0,0 +1,76 @@
 >>   >> +// SPDX-License-Identifier: GPL-2.0+
 >>   >> +/*
 >>   >> + * Copyright (C) 2020 Sean Anderson <sean.anderson at seco.com>
 >>   >> + */
 >>   >> +
 >>   >> +#include <common.h>
 >>   >> +#include <dm.h>
 >>   >> +#include <mmc.h>
 >>   >> +#include <part.h>
 >>   >> +#include <part_efi.h>
 >>   >> +#include <dm/test.h>
 >>   >> +#include <test/ut.h>
 >>   >> +
 >>   >> +static int dm_test_part(struct unit_test_state *uts)
 >>   >> +{
 >>   >> +       char str_disk_guid[UUID_STR_LEN + 1];
 >>   >> +       struct blk_desc *mmc_dev_desc;
 >>   >> +       struct disk_partition part_info;
 >>   >> +       struct disk_partition parts[2] = {
 >>   >> +               {
 >>   >> +                       .start = 48, /* GPT data takes up the first 34 blocks or so */
 >>   >> +                       .size = 1,
 >>   >> +                       .name = "test1",
 >>   >> +               },
 >>   >> +               {
 >>   >> +                       .start = 49,
 >>   >> +                       .size = 1,
 >>   >> +                       .name = "test2",
 >>   >> +               },
 >>   >> +       };
 >>   >> +
 >>   >> +       ut_asserteq(1, blk_get_device_by_str("mmc", "1", &mmc_dev_desc));
 >>   >> +       if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
 >>   >> +               gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD);
 >>   >> +               gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD);
 >>   >> +               gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD);
 >>   >> +       }
 >>   >> +       ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts,
 >>   >> +                               ARRAY_SIZE(parts)));
 >>   >> +
 >>   >> +#define test(expected, part_str, whole) \
 >>   >
 >>   > Can this be a function instead of a macro?
 >>
 >> Not one-to-one because ut-asserteq returns on error. This could be
 >> changed to
 >>
 >>          ut_asserteq(-ENODEV, test("", true));
 >>
 >> but I think a macro is the simplest option.
 >
 > Well you are using ut_asserteq() in the macro so I don't see why the
 > macro is better than what you have above, with the code in a function?
 > That is what we normally do.

I suppose. It doesn't really matter too much IMO, but I will change it
for v3.

--Sean

 >
 >
 >>
 >> --Sean
 >>
 >>   >
 >>   >> +       ut_asserteq(expected, \
 >>   >> +                   part_get_info_by_dev_and_name_or_num("mmc", part_str, \
 >>   >> +                                                        &mmc_dev_desc, \
 >>   >> +                                                        &part_info, whole))
 >>   >> +
 >>   >> +       test(-ENODEV, "", true);
 >>   >> +       env_set("bootdevice", "0");
 >>   >> +       test(0, "", true);
 >>   >> +       env_set("bootdevice", "1");
 >>   >> +       test(1, "", false);
 >>   >> +       test(1, "-", false);
 >>   >> +       env_set("bootdevice", "");
 >>   >> +       test(-EPROTONOSUPPORT, "0", false);
 >>   >> +       test(0, "0", true);
 >>   >> +       test(0, ":0", true);
 >>   >> +       test(0, ".0", true);
 >>   >> +       test(0, ".0:0", true);
 >>   >> +       test(-EINVAL, "#test1", true);
 >>   >> +       test(1, "1", false);
 >>   >> +       test(1, "1", true);
 >>   >> +       test(-ENOENT, "1:0", false);
 >>   >> +       test(0, "1:0", true);
 >>   >> +       test(1, "1:1", false);
 >>   >> +       test(2, "1:2", false);
 >>   >> +       test(1, "1.0", false);
 >>   >> +       test(0, "1.0:0", true);
 >>   >> +       test(1, "1.0:1", false);
 >>   >> +       test(2, "1.0:2", false);
 >>   >> +       test(-EINVAL, "1#bogus", false);
 >>   >> +       test(1, "1#test1", false);
 >>   >> +       test(2, "1#test2", false);
 >>   >> +
 >>   >> +       return 0;
 >>   >> +}
 >>   >> +DM_TEST(dm_test_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
 >>   >> --
 >>   >> 2.25.1
 >>   >>
 >
 > Regards,
 > Simon
 >