| Message ID | 20210412225307.3217317-3-sean.anderson@seco.com |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v2,1/3] cmd: fs: Use part_get_info_by_dev_and_name_or_num to parse partitions | expand |
Hi Sean, On Mon, 12 Apr 2021 at 23:53, Sean Anderson <sean.anderson@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@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@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? > + 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
On 4/14/21 3:37 PM, Simon Glass wrote: > Hi Sean, > > On Mon, 12 Apr 2021 at 23:53, Sean Anderson <sean.anderson@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@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@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. --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 >
Hi Sean, On Fri, 16 Apr 2021 at 02:36, Sean Anderson <sean.anderson@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@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@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@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. > > --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
On 4/21/21 3:14 AM, Simon Glass wrote: > Hi Sean, > > On Fri, 16 Apr 2021 at 02:36, Sean Anderson <sean.anderson@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@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@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@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 >
On Mon, Apr 12, 2021 at 06:53:07PM -0400, Sean Anderson 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@seco.com> Applied to u-boot/master, thanks!
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@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) \ + 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);
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@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