| Message ID | 20181128193636.254378-18-brendanhiggins@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kunit: introduce KUnit, the Linux kernel unit testing framework | expand |
On Wed, Nov 28, 2018 at 1:38 PM Brendan Higgins <brendanhiggins@google.com> wrote: > > Migrate tests without any cleanup, or modifying test logic in anyway to > run under KUnit using the KUnit expectation and assertion API. Nice! You beat me to it. This is probably going to conflict with what is in the DT tree for 4.21. Also, please Cc the DT list for drivers/of/ changes. Looks good to me, but a few mostly formatting comments below. > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > --- > drivers/of/Kconfig | 1 + > drivers/of/unittest.c | 1405 ++++++++++++++++++++++------------------- > 2 files changed, 752 insertions(+), 654 deletions(-) > > diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > index ad3fcad4d75b8..f309399deac20 100644 > --- a/drivers/of/Kconfig > +++ b/drivers/of/Kconfig > @@ -15,6 +15,7 @@ if OF > config OF_UNITTEST > bool "Device Tree runtime unit tests" > depends on !SPARC > + depends on KUNIT Unless KUNIT has depends, better to be a select here. > select IRQ_DOMAIN > select OF_EARLY_FLATTREE > select OF_RESOLVE > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c > index 41b49716ac75f..a5ef44730ffdb 100644 > --- a/drivers/of/unittest.c > +++ b/drivers/of/unittest.c > @@ -26,186 +26,187 @@ > > #include <linux/bitops.h> > > +#include <kunit/test.h> > + > #include "of_private.h" > > -static struct unittest_results { > - int passed; > - int failed; > -} unittest_results; > - > -#define unittest(result, fmt, ...) ({ \ > - bool failed = !(result); \ > - if (failed) { \ > - unittest_results.failed++; \ > - pr_err("FAIL %s():%i " fmt, __func__, __LINE__, ##__VA_ARGS__); \ > - } else { \ > - unittest_results.passed++; \ > - pr_debug("pass %s():%i\n", __func__, __LINE__); \ > - } \ > - failed; \ > -}) > - > -static void __init of_unittest_find_node_by_name(void) > +static void of_unittest_find_node_by_name(struct kunit *test) Why do we have to drop __init everywhere? The tests run later? > { > struct device_node *np; > const char *options, *name; > > np = of_find_node_by_path("/testcase-data"); > name = kasprintf(GFP_KERNEL, "%pOF", np); > - unittest(np && !strcmp("/testcase-data", name), > - "find /testcase-data failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > + KUNIT_EXPECT_STREQ_MSG(test, "/testcase-data", name, > + "find /testcase-data failed\n"); > of_node_put(np); > kfree(name); > > /* Test if trailing '/' works */ > - np = of_find_node_by_path("/testcase-data/"); > - unittest(!np, "trailing '/' on /testcase-data/ should fail\n"); > + KUNIT_EXPECT_EQ_MSG(test, of_find_node_by_path("/testcase-data/"), NULL, > + "trailing '/' on /testcase-data/ should fail\n"); > > np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > name = kasprintf(GFP_KERNEL, "%pOF", np); > - unittest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", name), > - "find /testcase-data/phandle-tests/consumer-a failed\n"); > + KUNIT_EXPECT_STREQ_MSG(test, > + "/testcase-data/phandle-tests/consumer-a", name, > + "find /testcase-data/phandle-tests/consumer-a failed\n"); > of_node_put(np); > kfree(name); > > np = of_find_node_by_path("testcase-alias"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > name = kasprintf(GFP_KERNEL, "%pOF", np); > - unittest(np && !strcmp("/testcase-data", name), > - "find testcase-alias failed\n"); > + KUNIT_EXPECT_STREQ_MSG(test, "/testcase-data", name, > + "find testcase-alias failed\n"); > of_node_put(np); > kfree(name); > > /* Test if trailing '/' works on aliases */ > - np = of_find_node_by_path("testcase-alias/"); > - unittest(!np, "trailing '/' on testcase-alias/ should fail\n"); > + KUNIT_EXPECT_EQ_MSG(test, of_find_node_by_path("testcase-alias/"), NULL, > + "trailing '/' on testcase-alias/ should fail\n"); > > np = of_find_node_by_path("testcase-alias/phandle-tests/consumer-a"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > name = kasprintf(GFP_KERNEL, "%pOF", np); > - unittest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", name), > - "find testcase-alias/phandle-tests/consumer-a failed\n"); > + KUNIT_EXPECT_STREQ_MSG(test, > + "/testcase-data/phandle-tests/consumer-a", name, > + "find testcase-alias/phandle-tests/consumer-a failed\n"); > of_node_put(np); > kfree(name); > > - np = of_find_node_by_path("/testcase-data/missing-path"); > - unittest(!np, "non-existent path returned node %pOF\n", np); > + KUNIT_EXPECT_EQ_MSG(test, > + of_find_node_by_path("/testcase-data/missing-path"), > + NULL, > + "non-existent path returned node %pOF\n", np); 1 tab indent would help with less vertical code (in general, not this one so much). > of_node_put(np); > > - np = of_find_node_by_path("missing-alias"); > - unittest(!np, "non-existent alias returned node %pOF\n", np); > + KUNIT_EXPECT_EQ_MSG(test, of_find_node_by_path("missing-alias"), NULL, > + "non-existent alias returned node %pOF\n", np); > of_node_put(np); > > - np = of_find_node_by_path("testcase-alias/missing-path"); > - unittest(!np, "non-existent alias with relative path returned node %pOF\n", np); > + KUNIT_EXPECT_EQ_MSG(test, > + of_find_node_by_path("testcase-alias/missing-path"), > + NULL, > + "non-existent alias with relative path returned node %pOF\n", > + np); > of_node_put(np); > > np = of_find_node_opts_by_path("/testcase-data:testoption", &options); > - unittest(np && !strcmp("testoption", options), > - "option path test failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > + KUNIT_EXPECT_STREQ_MSG(test, "testoption", options, > + "option path test failed\n"); > of_node_put(np); > > np = of_find_node_opts_by_path("/testcase-data:test/option", &options); > - unittest(np && !strcmp("test/option", options), > - "option path test, subcase #1 failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > + KUNIT_EXPECT_STREQ_MSG(test, "test/option", options, > + "option path test, subcase #1 failed\n"); > of_node_put(np); > > np = of_find_node_opts_by_path("/testcase-data/testcase-device1:test/option", &options); > - unittest(np && !strcmp("test/option", options), > - "option path test, subcase #2 failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > + KUNIT_EXPECT_STREQ_MSG(test, "test/option", options, > + "option path test, subcase #2 failed\n"); > of_node_put(np); > > np = of_find_node_opts_by_path("/testcase-data:testoption", NULL); > - unittest(np, "NULL option path test failed\n"); > + KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG(test, np, > + "NULL option path test failed\n"); > of_node_put(np); > > np = of_find_node_opts_by_path("testcase-alias:testaliasoption", > &options); > - unittest(np && !strcmp("testaliasoption", options), > - "option alias path test failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > + KUNIT_EXPECT_STREQ_MSG(test, "testaliasoption", options, > + "option alias path test failed\n"); > of_node_put(np); > > np = of_find_node_opts_by_path("testcase-alias:test/alias/option", > &options); > - unittest(np && !strcmp("test/alias/option", options), > - "option alias path test, subcase #1 failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > + KUNIT_EXPECT_STREQ_MSG(test, "test/alias/option", options, > + "option alias path test, subcase #1 failed\n"); > of_node_put(np); > > np = of_find_node_opts_by_path("testcase-alias:testaliasoption", NULL); > - unittest(np, "NULL option alias path test failed\n"); > + KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG(test, np, > + "NULL option alias path test failed\n"); > of_node_put(np); > > options = "testoption"; > np = of_find_node_opts_by_path("testcase-alias", &options); > - unittest(np && !options, "option clearing test failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > + KUNIT_EXPECT_EQ_MSG(test, options, NULL, > + "option clearing test failed\n"); > of_node_put(np); > > options = "testoption"; > np = of_find_node_opts_by_path("/", &options); > - unittest(np && !options, "option clearing root node test failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > + KUNIT_EXPECT_EQ_MSG(test, options, NULL, > + "option clearing root node test failed\n"); > of_node_put(np); > } > > -static void __init of_unittest_dynamic(void) > +static void of_unittest_dynamic(struct kunit *test) > { > struct device_node *np; > struct property *prop; > > np = of_find_node_by_path("/testcase-data"); > - if (!np) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > /* Array of 4 properties for the purpose of testing */ > prop = kcalloc(4, sizeof(*prop), GFP_KERNEL); > - if (!prop) { > - unittest(0, "kzalloc() failed\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, prop); > > /* Add a new property - should pass*/ > prop->name = "new-property"; > prop->value = "new-property-data"; > prop->length = strlen(prop->value) + 1; > - unittest(of_add_property(np, prop) == 0, "Adding a new property failed\n"); > + KUNIT_EXPECT_EQ_MSG(test, of_add_property(np, prop), 0, > + "Adding a new property failed\n"); > > /* Try to add an existing property - should fail */ > prop++; > prop->name = "new-property"; > prop->value = "new-property-data-should-fail"; > prop->length = strlen(prop->value) + 1; > - unittest(of_add_property(np, prop) != 0, > - "Adding an existing property should have failed\n"); > + KUNIT_EXPECT_NE_MSG(test, of_add_property(np, prop), 0, > + "Adding an existing property should have failed\n"); > > /* Try to modify an existing property - should pass */ > prop->value = "modify-property-data-should-pass"; > prop->length = strlen(prop->value) + 1; > - unittest(of_update_property(np, prop) == 0, > - "Updating an existing property should have passed\n"); > + KUNIT_EXPECT_EQ_MSG(test, of_update_property(np, prop), 0, > + "Updating an existing property should have passed\n"); > > /* Try to modify non-existent property - should pass*/ > prop++; > prop->name = "modify-property"; > prop->value = "modify-missing-property-data-should-pass"; > prop->length = strlen(prop->value) + 1; > - unittest(of_update_property(np, prop) == 0, > - "Updating a missing property should have passed\n"); > + KUNIT_EXPECT_EQ_MSG(test, of_update_property(np, prop), 0, > + "Updating a missing property should have passed\n"); > > /* Remove property - should pass */ > - unittest(of_remove_property(np, prop) == 0, > - "Removing a property should have passed\n"); > + KUNIT_EXPECT_EQ_MSG(test, of_remove_property(np, prop), 0, > + "Removing a property should have passed\n"); > > /* Adding very large property - should pass */ > prop++; > prop->name = "large-property-PAGE_SIZEx8"; > prop->length = PAGE_SIZE * 8; > prop->value = kzalloc(prop->length, GFP_KERNEL); > - unittest(prop->value != NULL, "Unable to allocate large buffer\n"); > - if (prop->value) > - unittest(of_add_property(np, prop) == 0, > - "Adding a large property should have passed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, prop->value); > + KUNIT_EXPECT_EQ_MSG(test, of_add_property(np, prop), 0, > + "Adding a large property should have passed\n"); > } > > -static int __init of_unittest_check_node_linkage(struct device_node *np) > +static int of_unittest_check_node_linkage(struct device_node *np) > { > struct device_node *child; > int count = 0, rc; > @@ -230,27 +231,29 @@ static int __init of_unittest_check_node_linkage(struct device_node *np) > return rc; > } > > -static void __init of_unittest_check_tree_linkage(void) > +static void of_unittest_check_tree_linkage(struct kunit *test) > { > struct device_node *np; > int allnode_count = 0, child_count; > > - if (!of_root) > - return; > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, of_root); > > for_each_of_allnodes(np) > allnode_count++; > child_count = of_unittest_check_node_linkage(of_root); > > - unittest(child_count > 0, "Device node data structure is corrupted\n"); > - unittest(child_count == allnode_count, > - "allnodes list size (%i) doesn't match sibling lists size (%i)\n", > - allnode_count, child_count); > + KUNIT_EXPECT_GT_MSG(test, child_count, 0, > + "Device node data structure is corrupted\n"); > + KUNIT_EXPECT_EQ_MSG(test, child_count, allnode_count, > + "allnodes list size (%i) doesn't match sibling lists size (%i)\n", > + allnode_count, child_count); > pr_debug("allnodes list size (%i); sibling lists size (%i)\n", allnode_count, child_count); > } > > -static void __init of_unittest_printf_one(struct device_node *np, const char *fmt, > - const char *expected) > +static void of_unittest_printf_one(struct kunit *test, > + struct device_node *np, > + const char *fmt, > + const char *expected) > { > unsigned char *buf; > int buf_size; > @@ -266,9 +269,12 @@ static void __init of_unittest_printf_one(struct device_node *np, const char *fm > size = snprintf(buf, buf_size - 2, fmt, np); > > /* use strcmp() instead of strncmp() here to be absolutely sure strings match */ > - unittest((strcmp(buf, expected) == 0) && (buf[size+1] == 0xff), > - "sprintf failed; fmt='%s' expected='%s' rslt='%s'\n", > - fmt, expected, buf); > + KUNIT_EXPECT_STREQ_MSG(test, buf, expected, > + "sprintf failed; fmt='%s' expected='%s' rslt='%s'\n", > + fmt, expected, buf); > + KUNIT_EXPECT_EQ_MSG(test, buf[size+1], 0xff, > + "sprintf failed; fmt='%s' expected='%s' rslt='%s'\n", > + fmt, expected, buf); > > /* Make sure length limits work */ > size++; > @@ -276,40 +282,43 @@ static void __init of_unittest_printf_one(struct device_node *np, const char *fm > /* Clear the buffer, and make sure it works correctly still */ > memset(buf, 0xff, buf_size); > snprintf(buf, size+1, fmt, np); > - unittest(strncmp(buf, expected, size) == 0 && (buf[size+1] == 0xff), > - "snprintf failed; size=%i fmt='%s' expected='%s' rslt='%s'\n", > - size, fmt, expected, buf); > + KUNIT_EXPECT_STREQ_MSG(test, buf, expected, > + "snprintf failed; size=%i fmt='%s' expected='%s' rslt='%s'\n", > + size, fmt, expected, buf); > + KUNIT_EXPECT_EQ_MSG(test, buf[size+1], 0xff, > + "snprintf failed; size=%i fmt='%s' expected='%s' rslt='%s'\n", > + size, fmt, expected, buf); > } > kfree(buf); > } > > -static void __init of_unittest_printf(void) > +static void of_unittest_printf(struct kunit *test) > { > struct device_node *np; > const char *full_name = "/testcase-data/platform-tests/test-device@1/dev@100"; > char phandle_str[16] = ""; > > np = of_find_node_by_path(full_name); > - if (!np) { > - unittest(np, "testcase data missing\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > num_to_str(phandle_str, sizeof(phandle_str), np->phandle, 0); > > - of_unittest_printf_one(np, "%pOF", full_name); > - of_unittest_printf_one(np, "%pOFf", full_name); > - of_unittest_printf_one(np, "%pOFp", phandle_str); > - of_unittest_printf_one(np, "%pOFP", "dev@100"); > - of_unittest_printf_one(np, "ABC %pOFP ABC", "ABC dev@100 ABC"); > - of_unittest_printf_one(np, "%10pOFP", " dev@100"); > - of_unittest_printf_one(np, "%-10pOFP", "dev@100 "); > - of_unittest_printf_one(of_root, "%pOFP", "/"); > - of_unittest_printf_one(np, "%pOFF", "----"); > - of_unittest_printf_one(np, "%pOFPF", "dev@100:----"); > - of_unittest_printf_one(np, "%pOFPFPc", "dev@100:----:dev@100:test-sub-device"); > - of_unittest_printf_one(np, "%pOFc", "test-sub-device"); > - of_unittest_printf_one(np, "%pOFC", > + of_unittest_printf_one(test, np, "%pOF", full_name); > + of_unittest_printf_one(test, np, "%pOFf", full_name); > + of_unittest_printf_one(test, np, "%pOFp", phandle_str); > + of_unittest_printf_one(test, np, "%pOFP", "dev@100"); > + of_unittest_printf_one(test, np, "ABC %pOFP ABC", "ABC dev@100 ABC"); > + of_unittest_printf_one(test, np, "%10pOFP", " dev@100"); > + of_unittest_printf_one(test, np, "%-10pOFP", "dev@100 "); > + of_unittest_printf_one(test, of_root, "%pOFP", "/"); > + of_unittest_printf_one(test, np, "%pOFF", "----"); > + of_unittest_printf_one(test, np, "%pOFPF", "dev@100:----"); > + of_unittest_printf_one(test, > + np, > + "%pOFPFPc", > + "dev@100:----:dev@100:test-sub-device"); > + of_unittest_printf_one(test, np, "%pOFc", "test-sub-device"); > + of_unittest_printf_one(test, np, "%pOFC", > "\"test-sub-device\",\"test-compat2\",\"test-compat3\""); > } > > @@ -319,7 +328,7 @@ struct node_hash { > }; > > static DEFINE_HASHTABLE(phandle_ht, 8); > -static void __init of_unittest_check_phandles(void) > +static void of_unittest_check_phandles(struct kunit *test) > { > struct device_node *np; > struct node_hash *nh; > @@ -331,24 +340,25 @@ static void __init of_unittest_check_phandles(void) > continue; > > hash_for_each_possible(phandle_ht, nh, node, np->phandle) { > + KUNIT_EXPECT_NE_MSG(test, nh->np->phandle, np->phandle, > + "Duplicate phandle! %i used by %pOF and %pOF\n", > + np->phandle, nh->np, np); > if (nh->np->phandle == np->phandle) { > - pr_info("Duplicate phandle! %i used by %pOF and %pOF\n", > - np->phandle, nh->np, np); > dup_count++; > break; > } > } > > nh = kzalloc(sizeof(*nh), GFP_KERNEL); > - if (WARN_ON(!nh)) > - return; > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, nh); > > nh->np = np; > hash_add(phandle_ht, &nh->node, np->phandle); > phandle_count++; > } > - unittest(dup_count == 0, "Found %i duplicates in %i phandles\n", > - dup_count, phandle_count); > + KUNIT_EXPECT_EQ_MSG(test, dup_count, 0, > + "Found %i duplicates in %i phandles\n", > + dup_count, phandle_count); > > /* Clean up */ > hash_for_each_safe(phandle_ht, i, tmp, nh, node) { > @@ -357,20 +367,22 @@ static void __init of_unittest_check_phandles(void) > } > } > > -static void __init of_unittest_parse_phandle_with_args(void) > +static void of_unittest_parse_phandle_with_args(struct kunit *test) > { > struct device_node *np; > struct of_phandle_args args; > - int i, rc; > + int i, rc = 0; > > np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); > - if (!np) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > - rc = of_count_phandle_with_args(np, "phandle-list", "#phandle-cells"); > - unittest(rc == 7, "of_count_phandle_with_args() returned %i, expected 7\n", rc); > + KUNIT_EXPECT_EQ_MSG(test, > + of_count_phandle_with_args(np, > + "phandle-list", > + "#phandle-cells"), > + 7, > + "of_count_phandle_with_args() returned %i, expected 7\n", > + rc); > > for (i = 0; i < 8; i++) { > bool passed = true; > @@ -423,81 +435,90 @@ static void __init of_unittest_parse_phandle_with_args(void) > passed = false; > } > > - unittest(passed, "index %i - data error on node %pOF rc=%i\n", > - i, args.np, rc); > + KUNIT_EXPECT_TRUE_MSG(test, passed, > + "index %i - data error on node %pOF rc=%i\n", > + i, args.np, rc); > } > > /* Check for missing list property */ > - rc = of_parse_phandle_with_args(np, "phandle-list-missing", > - "#phandle-cells", 0, &args); > - unittest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc); > - rc = of_count_phandle_with_args(np, "phandle-list-missing", > - "#phandle-cells"); > - unittest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc); > + KUNIT_EXPECT_EQ(test, > + of_parse_phandle_with_args(np, > + "phandle-list-missing", > + "#phandle-cells", > + 0, &args), > + -ENOENT); > + KUNIT_EXPECT_EQ(test, > + of_count_phandle_with_args(np, > + "phandle-list-missing", > + "#phandle-cells"), > + -ENOENT); > > /* Check for missing cells property */ > - rc = of_parse_phandle_with_args(np, "phandle-list", > - "#phandle-cells-missing", 0, &args); > - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); > - rc = of_count_phandle_with_args(np, "phandle-list", > - "#phandle-cells-missing"); > - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); > + KUNIT_EXPECT_EQ(test, > + of_parse_phandle_with_args(np, > + "phandle-list", > + "#phandle-cells-missing", > + 0, &args), > + -EINVAL); > + KUNIT_EXPECT_EQ(test, > + of_count_phandle_with_args(np, > + "phandle-list", > + "#phandle-cells-missing"), > + -EINVAL); > > /* Check for bad phandle in list */ > - rc = of_parse_phandle_with_args(np, "phandle-list-bad-phandle", > - "#phandle-cells", 0, &args); > - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); > - rc = of_count_phandle_with_args(np, "phandle-list-bad-phandle", > - "#phandle-cells"); > - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); > + KUNIT_EXPECT_EQ(test, > + of_parse_phandle_with_args(np, > + "phandle-list-bad-phandle", > + "#phandle-cells", > + 0, &args), > + -EINVAL); > + KUNIT_EXPECT_EQ(test, > + of_count_phandle_with_args(np, > + "phandle-list-bad-phandle", > + "#phandle-cells"), > + -EINVAL); > > /* Check for incorrectly formed argument list */ > - rc = of_parse_phandle_with_args(np, "phandle-list-bad-args", > - "#phandle-cells", 1, &args); > - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); > - rc = of_count_phandle_with_args(np, "phandle-list-bad-args", > - "#phandle-cells"); > - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); > + KUNIT_EXPECT_EQ(test, > + of_parse_phandle_with_args(np, > + "phandle-list-bad-args", > + "#phandle-cells", > + 1, &args), > + -EINVAL); > + KUNIT_EXPECT_EQ(test, > + of_count_phandle_with_args(np, > + "phandle-list-bad-args", > + "#phandle-cells"), > + -EINVAL); > } > > -static void __init of_unittest_parse_phandle_with_args_map(void) > +static void of_unittest_parse_phandle_with_args_map(struct kunit *test) > { > struct device_node *np, *p0, *p1, *p2, *p3; > struct of_phandle_args args; > int i, rc; > > np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-b"); > - if (!np) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > p0 = of_find_node_by_path("/testcase-data/phandle-tests/provider0"); > - if (!p0) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p0); > > p1 = of_find_node_by_path("/testcase-data/phandle-tests/provider1"); > - if (!p1) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p1); > > p2 = of_find_node_by_path("/testcase-data/phandle-tests/provider2"); > - if (!p2) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p2); > > p3 = of_find_node_by_path("/testcase-data/phandle-tests/provider3"); > - if (!p3) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p3); > > - rc = of_count_phandle_with_args(np, "phandle-list", "#phandle-cells"); > - unittest(rc == 7, "of_count_phandle_with_args() returned %i, expected 7\n", rc); > + KUNIT_EXPECT_EQ(test, > + of_count_phandle_with_args(np, > + "phandle-list", > + "#phandle-cells"), > + 7); > > for (i = 0; i < 8; i++) { > bool passed = true; > @@ -554,117 +575,214 @@ static void __init of_unittest_parse_phandle_with_args_map(void) > passed = false; > } > > - unittest(passed, "index %i - data error on node %s rc=%i\n", > - i, args.np->full_name, rc); > + KUNIT_EXPECT_TRUE_MSG(test, passed, > + "index %i - data error on node %s rc=%i\n", > + i, args.np->full_name, rc); > } > > /* Check for missing list property */ > - rc = of_parse_phandle_with_args_map(np, "phandle-list-missing", > - "phandle", 0, &args); > - unittest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc); > + KUNIT_EXPECT_EQ(test, > + of_parse_phandle_with_args_map(np, > + "phandle-list-missing", > + "phandle", > + 0, &args), > + -ENOENT); > > /* Check for missing cells,map,mask property */ > - rc = of_parse_phandle_with_args_map(np, "phandle-list", > - "phandle-missing", 0, &args); > - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); > + KUNIT_EXPECT_EQ(test, > + of_parse_phandle_with_args_map(np, > + "phandle-list", > + "phandle-missing", > + 0, &args), > + -EINVAL); > > /* Check for bad phandle in list */ > - rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-phandle", > - "phandle", 0, &args); > - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); > + KUNIT_EXPECT_EQ(test, > + of_parse_phandle_with_args_map(np, > + "phandle-list-bad-phandle", > + "phandle", > + 0, &args), > + -EINVAL); > > /* Check for incorrectly formed argument list */ > - rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-args", > - "phandle", 1, &args); > - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); > + KUNIT_EXPECT_EQ(test, > + of_parse_phandle_with_args_map(np, > + "phandle-list-bad-args", > + "phandle", > + 1, &args), > + -EINVAL); > } > > -static void __init of_unittest_property_string(void) > +static void of_unittest_property_string(struct kunit *test) > { > const char *strings[4]; > struct device_node *np; > int rc; > > np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); > - if (!np) { > - pr_err("No testcase data in device tree\n"); > - return; > - } > - > - rc = of_property_match_string(np, "phandle-list-names", "first"); > - unittest(rc == 0, "first expected:0 got:%i\n", rc); > - rc = of_property_match_string(np, "phandle-list-names", "second"); > - unittest(rc == 1, "second expected:1 got:%i\n", rc); > - rc = of_property_match_string(np, "phandle-list-names", "third"); > - unittest(rc == 2, "third expected:2 got:%i\n", rc); > - rc = of_property_match_string(np, "phandle-list-names", "fourth"); > - unittest(rc == -ENODATA, "unmatched string; rc=%i\n", rc); > - rc = of_property_match_string(np, "missing-property", "blah"); > - unittest(rc == -EINVAL, "missing property; rc=%i\n", rc); > - rc = of_property_match_string(np, "empty-property", "blah"); > - unittest(rc == -ENODATA, "empty property; rc=%i\n", rc); > - rc = of_property_match_string(np, "unterminated-string", "blah"); > - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > + > + KUNIT_EXPECT_EQ(test, > + of_property_match_string(np, > + "phandle-list-names", > + "first"), > + 0); > + KUNIT_EXPECT_EQ(test, > + of_property_match_string(np, > + "phandle-list-names", > + "second"), > + 1); Fewer lines on these would be better even if we go over 80 chars. > + KUNIT_EXPECT_EQ(test, > + of_property_match_string(np, > + "phandle-list-names", > + "third"), > + 2); > + KUNIT_EXPECT_EQ_MSG(test, > + of_property_match_string(np, > + "phandle-list-names", > + "fourth"), > + -ENODATA, > + "unmatched string"); > + KUNIT_EXPECT_EQ_MSG(test, > + of_property_match_string(np, > + "missing-property", > + "blah"), > + -EINVAL, > + "missing property"); > + KUNIT_EXPECT_EQ_MSG(test, > + of_property_match_string(np, > + "empty-property", > + "blah"), > + -ENODATA, > + "empty property"); > + KUNIT_EXPECT_EQ_MSG(test, > + of_property_match_string(np, > + "unterminated-string", > + "blah"), > + -EILSEQ, > + "unterminated string"); > > /* of_property_count_strings() tests */ > - rc = of_property_count_strings(np, "string-property"); > - unittest(rc == 1, "Incorrect string count; rc=%i\n", rc); > - rc = of_property_count_strings(np, "phandle-list-names"); > - unittest(rc == 3, "Incorrect string count; rc=%i\n", rc); > - rc = of_property_count_strings(np, "unterminated-string"); > - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); > - rc = of_property_count_strings(np, "unterminated-string-list"); > - unittest(rc == -EILSEQ, "unterminated string array; rc=%i\n", rc); > + KUNIT_EXPECT_EQ(test, > + of_property_count_strings(np, "string-property"), > + 1); > + KUNIT_EXPECT_EQ(test, > + of_property_count_strings(np, "phandle-list-names"), > + 3); > + KUNIT_EXPECT_EQ_MSG(test, > + of_property_count_strings(np, > + "unterminated-string"), > + -EILSEQ, > + "unterminated string"); > + KUNIT_EXPECT_EQ_MSG(test, > + of_property_count_strings( > + np, > + "unterminated-string-list"), > + -EILSEQ, > + "unterminated string array"); > > /* of_property_read_string_index() tests */ > rc = of_property_read_string_index(np, "string-property", 0, strings); > - unittest(rc == 0 && !strcmp(strings[0], "foobar"), "of_property_read_string_index() failure; rc=%i\n", rc); > + KUNIT_ASSERT_EQ(test, rc, 0); > + KUNIT_EXPECT_STREQ(test, strings[0], "foobar"); > strings[0] = NULL; > rc = of_property_read_string_index(np, "string-property", 1, strings); > - unittest(rc == -ENODATA && strings[0] == NULL, "of_property_read_string_index() failure; rc=%i\n", rc); > - rc = of_property_read_string_index(np, "phandle-list-names", 0, strings); > - unittest(rc == 0 && !strcmp(strings[0], "first"), "of_property_read_string_index() failure; rc=%i\n", rc); > - rc = of_property_read_string_index(np, "phandle-list-names", 1, strings); > - unittest(rc == 0 && !strcmp(strings[0], "second"), "of_property_read_string_index() failure; rc=%i\n", rc); > - rc = of_property_read_string_index(np, "phandle-list-names", 2, strings); > - unittest(rc == 0 && !strcmp(strings[0], "third"), "of_property_read_string_index() failure; rc=%i\n", rc); > + KUNIT_EXPECT_EQ(test, rc, -ENODATA); > + KUNIT_EXPECT_EQ(test, strings[0], NULL); > + rc = of_property_read_string_index(np, > + "phandle-list-names", > + 0, > + strings); > + KUNIT_ASSERT_EQ(test, rc, 0); > + KUNIT_EXPECT_STREQ(test, strings[0], "first"); > + rc = of_property_read_string_index(np, > + "phandle-list-names", > + 1, > + strings); > + KUNIT_ASSERT_EQ(test, rc, 0); > + KUNIT_EXPECT_STREQ(test, strings[0], "second"); > + rc = of_property_read_string_index(np, > + "phandle-list-names", > + 2, > + strings); > + KUNIT_ASSERT_EQ(test, rc, 0); > + KUNIT_EXPECT_STREQ(test, strings[0], "third"); > strings[0] = NULL; > - rc = of_property_read_string_index(np, "phandle-list-names", 3, strings); > - unittest(rc == -ENODATA && strings[0] == NULL, "of_property_read_string_index() failure; rc=%i\n", rc); > + rc = of_property_read_string_index(np, > + "phandle-list-names", 3, strings); > + KUNIT_EXPECT_EQ(test, rc, -ENODATA); > + KUNIT_EXPECT_EQ(test, strings[0], NULL); > strings[0] = NULL; > - rc = of_property_read_string_index(np, "unterminated-string", 0, strings); > - unittest(rc == -EILSEQ && strings[0] == NULL, "of_property_read_string_index() failure; rc=%i\n", rc); > - rc = of_property_read_string_index(np, "unterminated-string-list", 0, strings); > - unittest(rc == 0 && !strcmp(strings[0], "first"), "of_property_read_string_index() failure; rc=%i\n", rc); > + rc = of_property_read_string_index(np, > + "unterminated-string", > + 0, > + strings); > + KUNIT_EXPECT_EQ(test, rc, -EILSEQ); > + KUNIT_EXPECT_EQ(test, strings[0], NULL); > + rc = of_property_read_string_index(np, > + "unterminated-string-list", > + 0, > + strings); > + KUNIT_ASSERT_EQ(test, rc, 0); > + KUNIT_EXPECT_STREQ(test, strings[0], "first"); > strings[0] = NULL; > - rc = of_property_read_string_index(np, "unterminated-string-list", 2, strings); /* should fail */ > - unittest(rc == -EILSEQ && strings[0] == NULL, "of_property_read_string_index() failure; rc=%i\n", rc); > - strings[1] = NULL; > + rc = of_property_read_string_index(np, > + "unterminated-string-list", > + 2, > + strings); /* should fail */ > + KUNIT_EXPECT_EQ(test, rc, -EILSEQ); > + KUNIT_EXPECT_EQ(test, strings[0], NULL); > > /* of_property_read_string_array() tests */ > - rc = of_property_read_string_array(np, "string-property", strings, 4); > - unittest(rc == 1, "Incorrect string count; rc=%i\n", rc); > - rc = of_property_read_string_array(np, "phandle-list-names", strings, 4); > - unittest(rc == 3, "Incorrect string count; rc=%i\n", rc); > - rc = of_property_read_string_array(np, "unterminated-string", strings, 4); > - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); > + strings[1] = NULL; > + KUNIT_EXPECT_EQ(test, > + of_property_read_string_array(np, > + "string-property", > + strings, 4), > + 1); > + KUNIT_EXPECT_EQ(test, > + of_property_read_string_array(np, > + "phandle-list-names", > + strings, 4), > + 3); > + KUNIT_EXPECT_EQ_MSG(test, > + of_property_read_string_array(np, > + "unterminated-string", > + strings, 4), > + -EILSEQ, > + "unterminated string"); > /* -- An incorrectly formed string should cause a failure */ > - rc = of_property_read_string_array(np, "unterminated-string-list", strings, 4); > - unittest(rc == -EILSEQ, "unterminated string array; rc=%i\n", rc); > + KUNIT_EXPECT_EQ_MSG(test, > + of_property_read_string_array( > + np, > + "unterminated-string-list", > + strings, > + 4), > + -EILSEQ, > + "unterminated string array"); > /* -- parsing the correctly formed strings should still work: */ > strings[2] = NULL; > - rc = of_property_read_string_array(np, "unterminated-string-list", strings, 2); > - unittest(rc == 2 && strings[2] == NULL, "of_property_read_string_array() failure; rc=%i\n", rc); > + rc = of_property_read_string_array(np, > + "unterminated-string-list", > + strings, > + 2); > + KUNIT_EXPECT_EQ(test, rc, 2); > + KUNIT_EXPECT_EQ(test, strings[2], NULL); > strings[1] = NULL; > - rc = of_property_read_string_array(np, "phandle-list-names", strings, 1); > - unittest(rc == 1 && strings[1] == NULL, "Overwrote end of string array; rc=%i, str='%s'\n", rc, strings[1]); > + rc = of_property_read_string_array(np, > + "phandle-list-names", > + strings, > + 1); > + KUNIT_ASSERT_EQ(test, rc, 1); > + KUNIT_EXPECT_EQ_MSG(test, strings[1], NULL, > + "Overwrote end of string array"); > } > > #define propcmp(p1, p2) (((p1)->length == (p2)->length) && \ > (p1)->value && (p2)->value && \ > !memcmp((p1)->value, (p2)->value, (p1)->length) && \ > !strcmp((p1)->name, (p2)->name)) > -static void __init of_unittest_property_copy(void) > +static void of_unittest_property_copy(struct kunit *test) > { > #ifdef CONFIG_OF_DYNAMIC > struct property p1 = { .name = "p1", .length = 0, .value = "" }; > @@ -672,20 +790,24 @@ static void __init of_unittest_property_copy(void) > struct property *new; > > new = __of_prop_dup(&p1, GFP_KERNEL); > - unittest(new && propcmp(&p1, new), "empty property didn't copy correctly\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new); > + KUNIT_EXPECT_TRUE_MSG(test, propcmp(&p1, new), > + "empty property didn't copy correctly"); > kfree(new->value); > kfree(new->name); > kfree(new); > > new = __of_prop_dup(&p2, GFP_KERNEL); > - unittest(new && propcmp(&p2, new), "non-empty property didn't copy correctly\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new); > + KUNIT_EXPECT_TRUE_MSG(test, propcmp(&p2, new), > + "non-empty property didn't copy correctly"); > kfree(new->value); > kfree(new->name); > kfree(new); > #endif > } > > -static void __init of_unittest_changeset(void) > +static void of_unittest_changeset(struct kunit *test) > { > #ifdef CONFIG_OF_DYNAMIC > struct property *ppadd, padd = { .name = "prop-add", .length = 1, .value = "" }; > @@ -698,32 +820,32 @@ static void __init of_unittest_changeset(void) > struct of_changeset chgset; > > n1 = __of_node_dup(NULL, "n1"); > - unittest(n1, "testcase setup failure\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, n1); > > n2 = __of_node_dup(NULL, "n2"); > - unittest(n2, "testcase setup failure\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, n2); > > n21 = __of_node_dup(NULL, "n21"); > - unittest(n21, "testcase setup failure %p\n", n21); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, n21); > > nchangeset = of_find_node_by_path("/testcase-data/changeset"); > nremove = of_get_child_by_name(nchangeset, "node-remove"); > - unittest(nremove, "testcase setup failure\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, nremove); > > ppadd = __of_prop_dup(&padd, GFP_KERNEL); > - unittest(ppadd, "testcase setup failure\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppadd); > > ppname_n1 = __of_prop_dup(&pname_n1, GFP_KERNEL); > - unittest(ppname_n1, "testcase setup failure\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppname_n1); > > ppname_n2 = __of_prop_dup(&pname_n2, GFP_KERNEL); > - unittest(ppname_n2, "testcase setup failure\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppname_n2); > > ppname_n21 = __of_prop_dup(&pname_n21, GFP_KERNEL); > - unittest(ppname_n21, "testcase setup failure\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppname_n21); > > ppupdate = __of_prop_dup(&pupdate, GFP_KERNEL); > - unittest(ppupdate, "testcase setup failure\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppupdate); > > parent = nchangeset; > n1->parent = parent; > @@ -731,54 +853,82 @@ static void __init of_unittest_changeset(void) > n21->parent = n2; > > ppremove = of_find_property(parent, "prop-remove", NULL); > - unittest(ppremove, "failed to find removal prop"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppremove); > > of_changeset_init(&chgset); > > - unittest(!of_changeset_attach_node(&chgset, n1), "fail attach n1\n"); > - unittest(!of_changeset_add_property(&chgset, n1, ppname_n1), "fail add prop name\n"); > - > - unittest(!of_changeset_attach_node(&chgset, n2), "fail attach n2\n"); > - unittest(!of_changeset_add_property(&chgset, n2, ppname_n2), "fail add prop name\n"); > - > - unittest(!of_changeset_detach_node(&chgset, nremove), "fail remove node\n"); > - unittest(!of_changeset_add_property(&chgset, n21, ppname_n21), "fail add prop name\n"); > - > - unittest(!of_changeset_attach_node(&chgset, n21), "fail attach n21\n"); > - > - unittest(!of_changeset_add_property(&chgset, parent, ppadd), "fail add prop prop-add\n"); > - unittest(!of_changeset_update_property(&chgset, parent, ppupdate), "fail update prop\n"); > - unittest(!of_changeset_remove_property(&chgset, parent, ppremove), "fail remove prop\n"); > - > - unittest(!of_changeset_apply(&chgset), "apply failed\n"); > + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_attach_node(&chgset, n1), > + "fail attach n1\n"); > + KUNIT_EXPECT_FALSE_MSG(test, > + of_changeset_add_property(&chgset, > + n1, > + ppname_n1), > + "fail add prop name\n"); > + > + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_attach_node(&chgset, n2), > + "fail attach n2\n"); > + KUNIT_EXPECT_FALSE_MSG(test, > + of_changeset_add_property(&chgset, > + n2, > + ppname_n2), > + "fail add prop name\n"); > + > + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_detach_node(&chgset, nremove), > + "fail remove node\n"); > + KUNIT_EXPECT_FALSE_MSG(test, > + of_changeset_add_property(&chgset, > + n21, > + ppname_n21), > + "fail add prop name\n"); > + > + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_attach_node(&chgset, n21), > + "fail attach n21\n"); > + > + KUNIT_EXPECT_FALSE_MSG(test, > + of_changeset_add_property(&chgset, > + parent, > + ppadd), > + "fail add prop prop-add\n"); > + KUNIT_EXPECT_FALSE_MSG(test, > + of_changeset_update_property(&chgset, > + parent, > + ppupdate), > + "fail update prop\n"); > + KUNIT_EXPECT_FALSE_MSG(test, > + of_changeset_remove_property(&chgset, > + parent, > + ppremove), > + "fail remove prop\n"); > + > + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_apply(&chgset), > + "apply failed\n"); > > of_node_put(nchangeset); > > /* Make sure node names are constructed correctly */ > - unittest((np = of_find_node_by_path("/testcase-data/changeset/n2/n21")), > - "'%pOF' not added\n", n21); > + KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG( > + test, > + np = of_find_node_by_path( > + "/testcase-data/changeset/n2/n21"), > + "'%pOF' not added\n", n21); > of_node_put(np); > > - unittest(!of_changeset_revert(&chgset), "revert failed\n"); > + KUNIT_EXPECT_FALSE(test, of_changeset_revert(&chgset)); > > of_changeset_destroy(&chgset); > #endif > } > > -static void __init of_unittest_parse_interrupts(void) > +static void of_unittest_parse_interrupts(struct kunit *test) > { > struct device_node *np; > struct of_phandle_args args; > int i, rc; > > - if (of_irq_workarounds & OF_IMAP_OLDWORLD_MAC) > - return; > + KUNIT_ASSERT_FALSE(test, of_irq_workarounds & OF_IMAP_OLDWORLD_MAC); > > np = of_find_node_by_path("/testcase-data/interrupts/interrupts0"); > - if (!np) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > for (i = 0; i < 4; i++) { > bool passed = true; > @@ -790,16 +940,14 @@ static void __init of_unittest_parse_interrupts(void) > passed &= (args.args_count == 1); > passed &= (args.args[0] == (i + 1)); > > - unittest(passed, "index %i - data error on node %pOF rc=%i\n", > - i, args.np, rc); > + KUNIT_EXPECT_TRUE_MSG(test, passed, > + "index %i - data error on node %pOF rc=%i\n", > + i, args.np, rc); > } > of_node_put(np); > > np = of_find_node_by_path("/testcase-data/interrupts/interrupts1"); > - if (!np) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > for (i = 0; i < 4; i++) { > bool passed = true; > @@ -836,26 +984,23 @@ static void __init of_unittest_parse_interrupts(void) > default: > passed = false; > } > - unittest(passed, "index %i - data error on node %pOF rc=%i\n", > - i, args.np, rc); > + KUNIT_EXPECT_TRUE_MSG(test, passed, > + "index %i - data error on node %pOF rc=%i\n", > + i, args.np, rc); > } > of_node_put(np); > } > > -static void __init of_unittest_parse_interrupts_extended(void) > +static void of_unittest_parse_interrupts_extended(struct kunit *test) > { > struct device_node *np; > struct of_phandle_args args; > int i, rc; > > - if (of_irq_workarounds & OF_IMAP_OLDWORLD_MAC) > - return; > + KUNIT_ASSERT_FALSE(test, of_irq_workarounds & OF_IMAP_OLDWORLD_MAC); > > np = of_find_node_by_path("/testcase-data/interrupts/interrupts-extended0"); > - if (!np) { > - pr_err("missing testcase data\n"); > - return; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > for (i = 0; i < 7; i++) { > bool passed = true; > @@ -909,8 +1054,9 @@ static void __init of_unittest_parse_interrupts_extended(void) > passed = false; > } > > - unittest(passed, "index %i - data error on node %pOF rc=%i\n", > - i, args.np, rc); > + KUNIT_EXPECT_TRUE_MSG(test, passed, > + "index %i - data error on node %pOF rc=%i\n", > + i, args.np, rc); > } > of_node_put(np); > } > @@ -950,7 +1096,7 @@ static struct { > { .path = "/testcase-data/match-node/name9", .data = "K", }, > }; > > -static void __init of_unittest_match_node(void) > +static void of_unittest_match_node(struct kunit *test) > { > struct device_node *np; > const struct of_device_id *match; > @@ -958,26 +1104,19 @@ static void __init of_unittest_match_node(void) > > for (i = 0; i < ARRAY_SIZE(match_node_tests); i++) { > np = of_find_node_by_path(match_node_tests[i].path); > - if (!np) { > - unittest(0, "missing testcase node %s\n", > - match_node_tests[i].path); > - continue; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > match = of_match_node(match_node_table, np); > - if (!match) { > - unittest(0, "%s didn't match anything\n", > - match_node_tests[i].path); > - continue; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, np, > + "%s didn't match anything", > + match_node_tests[i].path); > > - if (strcmp(match->data, match_node_tests[i].data) != 0) { > - unittest(0, "%s got wrong match. expected %s, got %s\n", > - match_node_tests[i].path, match_node_tests[i].data, > - (const char *)match->data); > - continue; > - } > - unittest(1, "passed"); > + KUNIT_EXPECT_STREQ_MSG(test, > + match->data, match_node_tests[i].data, > + "%s got wrong match. expected %s, got %s\n", > + match_node_tests[i].path, > + match_node_tests[i].data, > + (const char *)match->data); > } > } > > @@ -989,9 +1128,9 @@ static struct resource test_bus_res = { > static const struct platform_device_info test_bus_info = { > .name = "unittest-bus", > }; > -static void __init of_unittest_platform_populate(void) > +static void of_unittest_platform_populate(struct kunit *test) > { > - int irq, rc; > + int irq; > struct device_node *np, *child, *grandchild; > struct platform_device *pdev, *test_bus; > const struct of_device_id match[] = { > @@ -1005,32 +1144,27 @@ static void __init of_unittest_platform_populate(void) > /* Test that a missing irq domain returns -EPROBE_DEFER */ > np = of_find_node_by_path("/testcase-data/testcase-device1"); > pdev = of_find_device_by_node(np); > - unittest(pdev, "device 1 creation failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev); > > if (!(of_irq_workarounds & OF_IMAP_OLDWORLD_MAC)) { > irq = platform_get_irq(pdev, 0); > - unittest(irq == -EPROBE_DEFER, > - "device deferred probe failed - %d\n", irq); > + KUNIT_ASSERT_EQ(test, irq, -EPROBE_DEFER); > > /* Test that a parsing failure does not return -EPROBE_DEFER */ > np = of_find_node_by_path("/testcase-data/testcase-device2"); > pdev = of_find_device_by_node(np); > - unittest(pdev, "device 2 creation failed\n"); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev); > irq = platform_get_irq(pdev, 0); > - unittest(irq < 0 && irq != -EPROBE_DEFER, > - "device parsing error failed - %d\n", irq); > + KUNIT_ASSERT_TRUE_MSG(test, irq < 0 && irq != -EPROBE_DEFER, > + "device parsing error failed - %d\n", > + irq); > } > > np = of_find_node_by_path("/testcase-data/platform-tests"); > - unittest(np, "No testcase data in device tree\n"); > - if (!np) > - return; > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > test_bus = platform_device_register_full(&test_bus_info); > - rc = PTR_ERR_OR_ZERO(test_bus); > - unittest(!rc, "testbus registration failed; rc=%i\n", rc); > - if (rc) > - return; > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_bus); > test_bus->dev.of_node = np; > > /* > @@ -1045,17 +1179,21 @@ static void __init of_unittest_platform_populate(void) > of_platform_populate(np, match, NULL, &test_bus->dev); > for_each_child_of_node(np, child) { > for_each_child_of_node(child, grandchild) > - unittest(of_find_device_by_node(grandchild), > - "Could not create device for node '%s'\n", > - grandchild->name); > + KUNIT_EXPECT_TRUE_MSG(test, > + of_find_device_by_node( > + grandchild), > + "Could not create device for node '%s'\n", > + grandchild->name); > } > > of_platform_depopulate(&test_bus->dev); > for_each_child_of_node(np, child) { > for_each_child_of_node(child, grandchild) > - unittest(!of_find_device_by_node(grandchild), > - "device didn't get destroyed '%s'\n", > - grandchild->name); > + KUNIT_EXPECT_FALSE_MSG(test, > + of_find_device_by_node( > + grandchild), > + "device didn't get destroyed '%s'\n", > + grandchild->name); > } > > platform_device_unregister(test_bus); > @@ -1129,7 +1267,7 @@ static int attach_node_and_children(struct device_node *np) > * unittest_data_add - Reads, copies data from > * linked tree and attaches it to the live tree > */ > -static int __init unittest_data_add(void) > +static int unittest_data_add(void) > { > void *unittest_data; > struct device_node *unittest_data_node, *np; > @@ -1200,7 +1338,7 @@ static int __init unittest_data_add(void) > } > > #ifdef CONFIG_OF_OVERLAY > -static int __init overlay_data_apply(const char *overlay_name, int *overlay_id); > +static int overlay_data_apply(const char *overlay_name, int *overlay_id); > > static int unittest_probe(struct platform_device *pdev) > { > @@ -1429,173 +1567,157 @@ static void of_unittest_destroy_tracked_overlays(void) > } while (defers > 0); > } > > -static int __init of_unittest_apply_overlay(int overlay_nr, int unittest_nr, > - int *overlay_id) > +static int of_unittest_apply_overlay(struct kunit *test, > + int overlay_nr, > + int unittest_nr, > + int *overlay_id) > { > const char *overlay_name; > > overlay_name = overlay_name_from_nr(overlay_nr); > > - if (!overlay_data_apply(overlay_name, overlay_id)) { > - unittest(0, "could not apply overlay \"%s\"\n", > - overlay_name); > - return -EFAULT; > - } > + KUNIT_ASSERT_TRUE_MSG(test, > + overlay_data_apply(overlay_name, overlay_id), > + "could not apply overlay \"%s\"\n", > + overlay_name); > of_unittest_track_overlay(*overlay_id); > > return 0; > } > > /* apply an overlay while checking before and after states */ > -static int __init of_unittest_apply_overlay_check(int overlay_nr, > +static int of_unittest_apply_overlay_check(struct kunit *test, int overlay_nr, > int unittest_nr, int before, int after, > enum overlay_type ovtype) > { > - int ret, ovcs_id; > + int ovcs_id; > > /* unittest device must not be in before state */ > - if (of_unittest_device_exists(unittest_nr, ovtype) != before) { > - unittest(0, "%s with device @\"%s\" %s\n", > - overlay_name_from_nr(overlay_nr), > - unittest_path(unittest_nr, ovtype), > - !before ? "enabled" : "disabled"); > - return -EINVAL; > - } > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_device_exists(unittest_nr, ovtype), > + before, > + "%s with device @\"%s\" %s\n", > + overlay_name_from_nr(overlay_nr), > + unittest_path(unittest_nr, ovtype), > + !before ? "enabled" : "disabled"); > > ovcs_id = 0; > - ret = of_unittest_apply_overlay(overlay_nr, unittest_nr, &ovcs_id); > - if (ret != 0) { > - /* of_unittest_apply_overlay already called unittest() */ > - return ret; > - } > + KUNIT_EXPECT_EQ(test, > + of_unittest_apply_overlay(test, > + overlay_nr, > + unittest_nr, > + &ovcs_id), > + 0); > > /* unittest device must be to set to after state */ > - if (of_unittest_device_exists(unittest_nr, ovtype) != after) { > - unittest(0, "%s failed to create @\"%s\" %s\n", > - overlay_name_from_nr(overlay_nr), > - unittest_path(unittest_nr, ovtype), > - !after ? "enabled" : "disabled"); > - return -EINVAL; > - } > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_device_exists(unittest_nr, ovtype), > + after, > + "%s failed to create @\"%s\" %s\n", > + overlay_name_from_nr(overlay_nr), > + unittest_path(unittest_nr, ovtype), > + !after ? "enabled" : "disabled"); > > return 0; > } > > /* apply an overlay and then revert it while checking before, after states */ > -static int __init of_unittest_apply_revert_overlay_check(int overlay_nr, > +static int of_unittest_apply_revert_overlay_check( > + struct kunit *test, int overlay_nr, > int unittest_nr, int before, int after, > enum overlay_type ovtype) > { > - int ret, ovcs_id; > + int ovcs_id; > > /* unittest device must be in before state */ > - if (of_unittest_device_exists(unittest_nr, ovtype) != before) { > - unittest(0, "%s with device @\"%s\" %s\n", > - overlay_name_from_nr(overlay_nr), > - unittest_path(unittest_nr, ovtype), > - !before ? "enabled" : "disabled"); > - return -EINVAL; > - } > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_device_exists(unittest_nr, ovtype), > + before, > + "%s with device @\"%s\" %s\n", > + overlay_name_from_nr(overlay_nr), > + unittest_path(unittest_nr, ovtype), > + !before ? "enabled" : "disabled"); > > /* apply the overlay */ > ovcs_id = 0; > - ret = of_unittest_apply_overlay(overlay_nr, unittest_nr, &ovcs_id); > - if (ret != 0) { > - /* of_unittest_apply_overlay already called unittest() */ > - return ret; > - } > + KUNIT_ASSERT_EQ(test, > + of_unittest_apply_overlay(test, > + overlay_nr, > + unittest_nr, > + &ovcs_id), > + 0); > > /* unittest device must be in after state */ > - if (of_unittest_device_exists(unittest_nr, ovtype) != after) { > - unittest(0, "%s failed to create @\"%s\" %s\n", > - overlay_name_from_nr(overlay_nr), > - unittest_path(unittest_nr, ovtype), > - !after ? "enabled" : "disabled"); > - return -EINVAL; > - } > - > - ret = of_overlay_remove(&ovcs_id); > - if (ret != 0) { > - unittest(0, "%s failed to be destroyed @\"%s\"\n", > - overlay_name_from_nr(overlay_nr), > - unittest_path(unittest_nr, ovtype)); > - return ret; > - } > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_device_exists(unittest_nr, ovtype), > + after, > + "%s failed to create @\"%s\" %s\n", > + overlay_name_from_nr(overlay_nr), > + unittest_path(unittest_nr, ovtype), > + !after ? "enabled" : "disabled"); > + > + KUNIT_ASSERT_EQ_MSG(test, of_overlay_remove(&ovcs_id), 0, > + "%s failed to be destroyed @\"%s\"\n", > + overlay_name_from_nr(overlay_nr), > + unittest_path(unittest_nr, ovtype)); > > /* unittest device must be again in before state */ > - if (of_unittest_device_exists(unittest_nr, PDEV_OVERLAY) != before) { > - unittest(0, "%s with device @\"%s\" %s\n", > - overlay_name_from_nr(overlay_nr), > - unittest_path(unittest_nr, ovtype), > - !before ? "enabled" : "disabled"); > - return -EINVAL; > - } > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_device_exists(unittest_nr, > + PDEV_OVERLAY), > + before, > + "%s with device @\"%s\" %s\n", > + overlay_name_from_nr(overlay_nr), > + unittest_path(unittest_nr, ovtype), > + !before ? "enabled" : "disabled"); > > return 0; > } > > /* test activation of device */ > -static void __init of_unittest_overlay_0(void) > +static void of_unittest_overlay_0(struct kunit *test) > { > /* device should enable */ > - if (of_unittest_apply_overlay_check(0, 0, 0, 1, PDEV_OVERLAY)) > - return; > - > - unittest(1, "overlay test %d passed\n", 0); > + of_unittest_apply_overlay_check(test, 0, 0, 0, 1, PDEV_OVERLAY); > } > > /* test deactivation of device */ > -static void __init of_unittest_overlay_1(void) > +static void of_unittest_overlay_1(struct kunit *test) > { > /* device should disable */ > - if (of_unittest_apply_overlay_check(1, 1, 1, 0, PDEV_OVERLAY)) > - return; > - > - unittest(1, "overlay test %d passed\n", 1); > + of_unittest_apply_overlay_check(test, 1, 1, 1, 0, PDEV_OVERLAY); > } > > /* test activation of device */ > -static void __init of_unittest_overlay_2(void) > +static void of_unittest_overlay_2(struct kunit *test) > { > /* device should enable */ > - if (of_unittest_apply_overlay_check(2, 2, 0, 1, PDEV_OVERLAY)) > - return; > - > - unittest(1, "overlay test %d passed\n", 2); > + of_unittest_apply_overlay_check(test, 2, 2, 0, 1, PDEV_OVERLAY); > } > > /* test deactivation of device */ > -static void __init of_unittest_overlay_3(void) > +static void of_unittest_overlay_3(struct kunit *test) > { > /* device should disable */ > - if (of_unittest_apply_overlay_check(3, 3, 1, 0, PDEV_OVERLAY)) > - return; > - > - unittest(1, "overlay test %d passed\n", 3); > + of_unittest_apply_overlay_check(test, 3, 3, 1, 0, PDEV_OVERLAY); > } > > /* test activation of a full device node */ > -static void __init of_unittest_overlay_4(void) > +static void of_unittest_overlay_4(struct kunit *test) > { > /* device should disable */ > - if (of_unittest_apply_overlay_check(4, 4, 0, 1, PDEV_OVERLAY)) > - return; > - > - unittest(1, "overlay test %d passed\n", 4); > + of_unittest_apply_overlay_check(test, 4, 4, 0, 1, PDEV_OVERLAY); > } > > /* test overlay apply/revert sequence */ > -static void __init of_unittest_overlay_5(void) > +static void of_unittest_overlay_5(struct kunit *test) > { > /* device should disable */ > - if (of_unittest_apply_revert_overlay_check(5, 5, 0, 1, PDEV_OVERLAY)) > - return; > - > - unittest(1, "overlay test %d passed\n", 5); > + of_unittest_apply_revert_overlay_check(test, 5, 5, 0, 1, PDEV_OVERLAY); > } > > /* test overlay application in sequence */ > -static void __init of_unittest_overlay_6(void) > +static void of_unittest_overlay_6(struct kunit *test) > { > int i, ov_id[2], ovcs_id; > int overlay_nr = 6, unittest_nr = 6; > @@ -1604,74 +1726,69 @@ static void __init of_unittest_overlay_6(void) > > /* unittest device must be in before state */ > for (i = 0; i < 2; i++) { > - if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY) > - != before) { > - unittest(0, "%s with device @\"%s\" %s\n", > - overlay_name_from_nr(overlay_nr + i), > - unittest_path(unittest_nr + i, > - PDEV_OVERLAY), > - !before ? "enabled" : "disabled"); > - return; > - } > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_device_exists(unittest_nr + i, > + PDEV_OVERLAY), > + before, > + "%s with device @\"%s\" %s\n", > + overlay_name_from_nr(overlay_nr + i), > + unittest_path(unittest_nr + i, > + PDEV_OVERLAY), > + !before ? "enabled" : "disabled"); > } > > /* apply the overlays */ > for (i = 0; i < 2; i++) { > - > overlay_name = overlay_name_from_nr(overlay_nr + i); > > - if (!overlay_data_apply(overlay_name, &ovcs_id)) { > - unittest(0, "could not apply overlay \"%s\"\n", > - overlay_name); > - return; > - } > + KUNIT_ASSERT_TRUE_MSG(test, > + overlay_data_apply(overlay_name, > + &ovcs_id), > + "could not apply overlay \"%s\"\n", > + overlay_name); > ov_id[i] = ovcs_id; > of_unittest_track_overlay(ov_id[i]); > } > > for (i = 0; i < 2; i++) { > /* unittest device must be in after state */ > - if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY) > - != after) { > - unittest(0, "overlay @\"%s\" failed @\"%s\" %s\n", > - overlay_name_from_nr(overlay_nr + i), > - unittest_path(unittest_nr + i, > - PDEV_OVERLAY), > - !after ? "enabled" : "disabled"); > - return; > - } > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_device_exists(unittest_nr + i, > + PDEV_OVERLAY), > + after, > + "overlay @\"%s\" failed @\"%s\" %s\n", > + overlay_name_from_nr(overlay_nr + i), > + unittest_path(unittest_nr + i, > + PDEV_OVERLAY), > + !after ? "enabled" : "disabled"); > } > > for (i = 1; i >= 0; i--) { > ovcs_id = ov_id[i]; > - if (of_overlay_remove(&ovcs_id)) { > - unittest(0, "%s failed destroy @\"%s\"\n", > - overlay_name_from_nr(overlay_nr + i), > - unittest_path(unittest_nr + i, > - PDEV_OVERLAY)); > - return; > - } > + KUNIT_ASSERT_FALSE_MSG(test, of_overlay_remove(&ovcs_id), > + "%s failed destroy @\"%s\"\n", > + overlay_name_from_nr(overlay_nr + i), > + unittest_path(unittest_nr + i, > + PDEV_OVERLAY)); > of_unittest_untrack_overlay(ov_id[i]); > } > > for (i = 0; i < 2; i++) { > /* unittest device must be again in before state */ > - if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY) > - != before) { > - unittest(0, "%s with device @\"%s\" %s\n", > - overlay_name_from_nr(overlay_nr + i), > - unittest_path(unittest_nr + i, > - PDEV_OVERLAY), > - !before ? "enabled" : "disabled"); > - return; > - } > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_device_exists(unittest_nr + i, > + PDEV_OVERLAY), > + before, > + "%s with device @\"%s\" %s\n", > + overlay_name_from_nr(overlay_nr + i), > + unittest_path(unittest_nr + i, > + PDEV_OVERLAY), > + !before ? "enabled" : "disabled"); > } > - > - unittest(1, "overlay test %d passed\n", 6); > } > > /* test overlay application in sequence */ > -static void __init of_unittest_overlay_8(void) > +static void of_unittest_overlay_8(struct kunit *test) > { > int i, ov_id[2], ovcs_id; > int overlay_nr = 8, unittest_nr = 8; > @@ -1681,76 +1798,73 @@ static void __init of_unittest_overlay_8(void) > > /* apply the overlays */ > for (i = 0; i < 2; i++) { > - > overlay_name = overlay_name_from_nr(overlay_nr + i); > > - if (!overlay_data_apply(overlay_name, &ovcs_id)) { > - unittest(0, "could not apply overlay \"%s\"\n", > - overlay_name); > - return; > - } > + KUNIT_ASSERT_TRUE_MSG(test, > + overlay_data_apply(overlay_name, > + &ovcs_id), > + "could not apply overlay \"%s\"\n", > + overlay_name); > ov_id[i] = ovcs_id; > of_unittest_track_overlay(ov_id[i]); > } > > /* now try to remove first overlay (it should fail) */ > ovcs_id = ov_id[0]; > - if (!of_overlay_remove(&ovcs_id)) { > - unittest(0, "%s was destroyed @\"%s\"\n", > - overlay_name_from_nr(overlay_nr + 0), > - unittest_path(unittest_nr, > - PDEV_OVERLAY)); > - return; > - } > + KUNIT_ASSERT_TRUE_MSG(test, of_overlay_remove(&ovcs_id), > + "%s was destroyed @\"%s\"\n", > + overlay_name_from_nr(overlay_nr + 0), > + unittest_path(unittest_nr, > + PDEV_OVERLAY)); > > /* removing them in order should work */ > for (i = 1; i >= 0; i--) { > ovcs_id = ov_id[i]; > - if (of_overlay_remove(&ovcs_id)) { > - unittest(0, "%s not destroyed @\"%s\"\n", > - overlay_name_from_nr(overlay_nr + i), > - unittest_path(unittest_nr, > - PDEV_OVERLAY)); > - return; > - } > + KUNIT_ASSERT_FALSE_MSG(test, of_overlay_remove(&ovcs_id), > + "%s not destroyed @\"%s\"\n", > + overlay_name_from_nr(overlay_nr + i), > + unittest_path(unittest_nr, > + PDEV_OVERLAY)); > of_unittest_untrack_overlay(ov_id[i]); > } > - > - unittest(1, "overlay test %d passed\n", 8); > } > > /* test insertion of a bus with parent devices */ > -static void __init of_unittest_overlay_10(void) > +static void of_unittest_overlay_10(struct kunit *test) > { > - int ret; > char *child_path; > > /* device should disable */ > - ret = of_unittest_apply_overlay_check(10, 10, 0, 1, PDEV_OVERLAY); > - if (unittest(ret == 0, > - "overlay test %d failed; overlay application\n", 10)) > - return; > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_apply_overlay_check(test, > + 10, > + 10, > + 0, > + 1, > + PDEV_OVERLAY), I prefer putting multiple args on a line and having fewer lines. > + 0, > + "overlay test %d failed; overlay application\n", > + 10); > > child_path = kasprintf(GFP_KERNEL, "%s/test-unittest101", > unittest_path(10, PDEV_OVERLAY)); > - if (unittest(child_path, "overlay test %d failed; kasprintf\n", 10)) > - return; > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, child_path); > > - ret = of_path_device_type_exists(child_path, PDEV_OVERLAY); > + KUNIT_EXPECT_TRUE_MSG(test, > + of_path_device_type_exists(child_path, > + PDEV_OVERLAY), > + "overlay test %d failed; no child device\n", 10); > kfree(child_path); > - > - unittest(ret, "overlay test %d failed; no child device\n", 10); > } > > /* test insertion of a bus with parent devices (and revert) */ > -static void __init of_unittest_overlay_11(void) > +static void of_unittest_overlay_11(struct kunit *test) > { > - int ret; > - > /* device should disable */ > - ret = of_unittest_apply_revert_overlay_check(11, 11, 0, 1, > - PDEV_OVERLAY); > - unittest(ret == 0, "overlay test %d failed; overlay apply\n", 11); > + KUNIT_EXPECT_FALSE(test, > + of_unittest_apply_revert_overlay_check(test, > + 11, 11, 0, 1, > + PDEV_OVERLAY)); > } > > #if IS_BUILTIN(CONFIG_I2C) && IS_ENABLED(CONFIG_OF_OVERLAY) > @@ -1972,25 +2086,23 @@ static struct i2c_driver unittest_i2c_mux_driver = { > > #endif > > -static int of_unittest_overlay_i2c_init(void) > +static int of_unittest_overlay_i2c_init(struct kunit *test) > { > - int ret; > + KUNIT_ASSERT_EQ_MSG(test, > + i2c_add_driver(&unittest_i2c_dev_driver), > + 0, > + "could not register unittest i2c device driver\n"); > > - ret = i2c_add_driver(&unittest_i2c_dev_driver); > - if (unittest(ret == 0, > - "could not register unittest i2c device driver\n")) > - return ret; > - > - ret = platform_driver_register(&unittest_i2c_bus_driver); > - if (unittest(ret == 0, > - "could not register unittest i2c bus driver\n")) > - return ret; > + KUNIT_ASSERT_EQ_MSG(test, > + platform_driver_register(&unittest_i2c_bus_driver), > + 0, > + "could not register unittest i2c bus driver\n"); > > #if IS_BUILTIN(CONFIG_I2C_MUX) > - ret = i2c_add_driver(&unittest_i2c_mux_driver); > - if (unittest(ret == 0, > - "could not register unittest i2c mux driver\n")) > - return ret; > + KUNIT_ASSERT_EQ_MSG(test, > + i2c_add_driver(&unittest_i2c_mux_driver), > + 0, > + "could not register unittest i2c mux driver\n"); > #endif > > return 0; > @@ -2005,101 +2117,89 @@ static void of_unittest_overlay_i2c_cleanup(void) > i2c_del_driver(&unittest_i2c_dev_driver); > } > > -static void __init of_unittest_overlay_i2c_12(void) > +static void of_unittest_overlay_i2c_12(struct kunit *test) > { > /* device should enable */ > - if (of_unittest_apply_overlay_check(12, 12, 0, 1, I2C_OVERLAY)) > - return; > - > - unittest(1, "overlay test %d passed\n", 12); > + of_unittest_apply_overlay_check(test, 12, 12, 0, 1, I2C_OVERLAY); > } > > /* test deactivation of device */ > -static void __init of_unittest_overlay_i2c_13(void) > +static void of_unittest_overlay_i2c_13(struct kunit *test) > { > /* device should disable */ > - if (of_unittest_apply_overlay_check(13, 13, 1, 0, I2C_OVERLAY)) > - return; > - > - unittest(1, "overlay test %d passed\n", 13); > + of_unittest_apply_overlay_check(test, 13, 13, 1, 0, I2C_OVERLAY); > } > > /* just check for i2c mux existence */ > -static void of_unittest_overlay_i2c_14(void) > +static void of_unittest_overlay_i2c_14(struct kunit *test) > { > + KUNIT_SUCCEED(test); > } > > -static void __init of_unittest_overlay_i2c_15(void) > +static void of_unittest_overlay_i2c_15(struct kunit *test) > { > /* device should enable */ > - if (of_unittest_apply_overlay_check(15, 15, 0, 1, I2C_OVERLAY)) > - return; > - > - unittest(1, "overlay test %d passed\n", 15); > + of_unittest_apply_overlay_check(test, 15, 15, 0, 1, I2C_OVERLAY); > } > > #else > > -static inline void of_unittest_overlay_i2c_14(void) { } > -static inline void of_unittest_overlay_i2c_15(void) { } > +static inline void of_unittest_overlay_i2c_14(struct kunit *test) { } > +static inline void of_unittest_overlay_i2c_15(struct kunit *test) { } > > #endif > > -static void __init of_unittest_overlay(void) > +static void of_unittest_overlay(struct kunit *test) > { > struct device_node *bus_np = NULL; > > - if (platform_driver_register(&unittest_driver)) { > - unittest(0, "could not register unittest driver\n"); > - goto out; > - } > + KUNIT_ASSERT_FALSE_MSG(test, > + platform_driver_register(&unittest_driver), > + "could not register unittest driver\n"); > > bus_np = of_find_node_by_path(bus_path); > - if (bus_np == NULL) { > - unittest(0, "could not find bus_path \"%s\"\n", bus_path); > - goto out; > - } > + KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, bus_np, > + "could not find bus_path \"%s\"\n", > + bus_path); > > - if (of_platform_default_populate(bus_np, NULL, NULL)) { > - unittest(0, "could not populate bus @ \"%s\"\n", bus_path); > - goto out; > - } > + KUNIT_ASSERT_FALSE_MSG(test, > + of_platform_default_populate(bus_np, NULL, NULL), > + "could not populate bus @ \"%s\"\n", bus_path); > > - if (!of_unittest_device_exists(100, PDEV_OVERLAY)) { > - unittest(0, "could not find unittest0 @ \"%s\"\n", > - unittest_path(100, PDEV_OVERLAY)); > - goto out; > - } > + KUNIT_ASSERT_TRUE_MSG(test, > + of_unittest_device_exists(100, PDEV_OVERLAY), > + "could not find unittest0 @ \"%s\"\n", > + unittest_path(100, PDEV_OVERLAY)); > > - if (of_unittest_device_exists(101, PDEV_OVERLAY)) { > - unittest(0, "unittest1 @ \"%s\" should not exist\n", > - unittest_path(101, PDEV_OVERLAY)); > - goto out; > - } > - > - unittest(1, "basic infrastructure of overlays passed"); > + KUNIT_ASSERT_FALSE_MSG(test, > + of_unittest_device_exists(101, PDEV_OVERLAY), > + "unittest1 @ \"%s\" should not exist\n", > + unittest_path(101, PDEV_OVERLAY)); > > /* tests in sequence */ > - of_unittest_overlay_0(); > - of_unittest_overlay_1(); > - of_unittest_overlay_2(); > - of_unittest_overlay_3(); > - of_unittest_overlay_4(); > - of_unittest_overlay_5(); > - of_unittest_overlay_6(); > - of_unittest_overlay_8(); > - > - of_unittest_overlay_10(); > - of_unittest_overlay_11(); > + of_unittest_overlay_0(test); > + of_unittest_overlay_1(test); > + of_unittest_overlay_2(test); > + of_unittest_overlay_3(test); > + of_unittest_overlay_4(test); > + of_unittest_overlay_5(test); > + of_unittest_overlay_6(test); > + of_unittest_overlay_8(test); > + > + of_unittest_overlay_10(test); > + of_unittest_overlay_11(test); > > #if IS_BUILTIN(CONFIG_I2C) > - if (unittest(of_unittest_overlay_i2c_init() == 0, "i2c init failed\n")) > + KUNIT_ASSERT_EQ_MSG(test, > + of_unittest_overlay_i2c_init(test), > + 0, > + "i2c init failed\n"); > goto out; > > - of_unittest_overlay_i2c_12(); > - of_unittest_overlay_i2c_13(); > - of_unittest_overlay_i2c_14(); > - of_unittest_overlay_i2c_15(); > + of_unittest_overlay_i2c_12(test); > + of_unittest_overlay_i2c_13(test); > + of_unittest_overlay_i2c_14(test); > + of_unittest_overlay_i2c_15(test); > > of_unittest_overlay_i2c_cleanup(); > #endif > @@ -2111,7 +2211,7 @@ static void __init of_unittest_overlay(void) > } > > #else > -static inline void __init of_unittest_overlay(void) { } > +static inline void of_unittest_overlay(struct kunit *test) { } > #endif > > #ifdef CONFIG_OF_OVERLAY > @@ -2254,7 +2354,7 @@ void __init unittest_unflatten_overlay_base(void) > * > * Return 0 on unexpected error. > */ > -static int __init overlay_data_apply(const char *overlay_name, int *overlay_id) > +static int overlay_data_apply(const char *overlay_name, int *overlay_id) > { > struct overlay_info *info; > int found = 0; > @@ -2301,19 +2401,17 @@ static int __init overlay_data_apply(const char *overlay_name, int *overlay_id) > * The first part of the function is _not_ normal overlay usage; it is > * finishing splicing the base overlay device tree into the live tree. > */ > -static __init void of_unittest_overlay_high_level(void) > +static void of_unittest_overlay_high_level(struct kunit *test) > { > struct device_node *last_sibling; > struct device_node *np; > struct device_node *of_symbols; > - struct device_node *overlay_base_symbols; > + struct device_node *overlay_base_symbols = 0; > struct device_node **pprev; > struct property *prop; > > - if (!overlay_base_root) { > - unittest(0, "overlay_base_root not initialized\n"); > - return; > - } > + KUNIT_ASSERT_TRUE_MSG(test, overlay_base_root, > + "overlay_base_root not initialized\n"); > > /* > * Could not fixup phandles in unittest_unflatten_overlay_base() > @@ -2358,11 +2456,10 @@ static __init void of_unittest_overlay_high_level(void) > } > > for (np = overlay_base_root->child; np; np = np->sibling) { > - if (of_get_child_by_name(of_root, np->name)) { > - unittest(0, "illegal node name in overlay_base %s", > - np->name); > - return; > - } > + KUNIT_ASSERT_FALSE_MSG(test, > + of_get_child_by_name(of_root, np->name), > + "illegal node name in overlay_base %s", > + np->name); > } > > /* > @@ -2395,21 +2492,24 @@ static __init void of_unittest_overlay_high_level(void) > > new_prop = __of_prop_dup(prop, GFP_KERNEL); > if (!new_prop) { > - unittest(0, "__of_prop_dup() of '%s' from overlay_base node __symbols__", > - prop->name); > + KUNIT_FAIL(test, > + "__of_prop_dup() of '%s' from overlay_base node __symbols__", > + prop->name); > goto err_unlock; > } > if (__of_add_property(of_symbols, new_prop)) { > /* "name" auto-generated by unflatten */ > if (!strcmp(new_prop->name, "name")) > continue; > - unittest(0, "duplicate property '%s' in overlay_base node __symbols__", > - prop->name); > + KUNIT_FAIL(test, > + "duplicate property '%s' in overlay_base node __symbols__", > + prop->name); > goto err_unlock; > } > if (__of_add_property_sysfs(of_symbols, new_prop)) { > - unittest(0, "unable to add property '%s' in overlay_base node __symbols__ to sysfs", > - prop->name); > + KUNIT_FAIL(test, > + "unable to add property '%s' in overlay_base node __symbols__ to sysfs", > + prop->name); > goto err_unlock; > } > } > @@ -2420,14 +2520,16 @@ static __init void of_unittest_overlay_high_level(void) > > /* now do the normal overlay usage test */ > > - unittest(overlay_data_apply("overlay", NULL), > - "Adding overlay 'overlay' failed\n"); > + KUNIT_EXPECT_TRUE_MSG(test, overlay_data_apply("overlay", NULL), > + "Adding overlay 'overlay' failed\n"); > > - unittest(overlay_data_apply("overlay_bad_phandle", NULL), > - "Adding overlay 'overlay_bad_phandle' failed\n"); > + KUNIT_EXPECT_TRUE_MSG(test, > + overlay_data_apply("overlay_bad_phandle", NULL), > + "Adding overlay 'overlay_bad_phandle' failed\n"); > > - unittest(overlay_data_apply("overlay_bad_symbol", NULL), > - "Adding overlay 'overlay_bad_symbol' failed\n"); > + KUNIT_EXPECT_TRUE_MSG(test, > + overlay_data_apply("overlay_bad_symbol", NULL), > + "Adding overlay 'overlay_bad_symbol' failed\n"); > > return; > > @@ -2437,54 +2539,49 @@ static __init void of_unittest_overlay_high_level(void) > > #else > > -static inline __init void of_unittest_overlay_high_level(void) {} > +static inline void of_unittest_overlay_high_level(struct kunit *test) {} > > #endif > > -static int __init of_unittest(void) > +static int of_test_init(struct kunit *test) > { > - struct device_node *np; > - int res; > - > /* adding data for unittest */ > - res = unittest_data_add(); > - if (res) > - return res; > + KUNIT_ASSERT_EQ(test, 0, unittest_data_add()); > + > if (!of_aliases) > of_aliases = of_find_node_by_path("/aliases"); > > - np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); > - if (!np) { > - pr_info("No testcase data in device tree; not running tests\n"); > - return 0; > - } > - of_node_put(np); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, of_find_node_by_path( > + "/testcase-data/phandle-tests/consumer-a")); > > - pr_info("start of unittest - you will see error messages\n"); > - of_unittest_check_tree_linkage(); > - of_unittest_check_phandles(); > - of_unittest_find_node_by_name(); > - of_unittest_dynamic(); > - of_unittest_parse_phandle_with_args(); > - of_unittest_parse_phandle_with_args_map(); > - of_unittest_printf(); > - of_unittest_property_string(); > - of_unittest_property_copy(); > - of_unittest_changeset(); > - of_unittest_parse_interrupts(); > - of_unittest_parse_interrupts_extended(); > - of_unittest_match_node(); > - of_unittest_platform_populate(); > - of_unittest_overlay(); > + return 0; > +} > > +static struct kunit_case of_test_cases[] = { > + KUNIT_CASE(of_unittest_check_tree_linkage), > + KUNIT_CASE(of_unittest_check_phandles), > + KUNIT_CASE(of_unittest_find_node_by_name), > + KUNIT_CASE(of_unittest_dynamic), > + KUNIT_CASE(of_unittest_parse_phandle_with_args), > + KUNIT_CASE(of_unittest_parse_phandle_with_args_map), > + KUNIT_CASE(of_unittest_printf), > + KUNIT_CASE(of_unittest_property_string), > + KUNIT_CASE(of_unittest_property_copy), > + KUNIT_CASE(of_unittest_changeset), > + KUNIT_CASE(of_unittest_parse_interrupts), > + KUNIT_CASE(of_unittest_parse_interrupts_extended), > + KUNIT_CASE(of_unittest_match_node), > + KUNIT_CASE(of_unittest_platform_populate), > + KUNIT_CASE(of_unittest_overlay), > /* Double check linkage after removing testcase data */ > - of_unittest_check_tree_linkage(); > - > - of_unittest_overlay_high_level(); > - > - pr_info("end of unittest - %i passed, %i failed\n", > - unittest_results.passed, unittest_results.failed); > + KUNIT_CASE(of_unittest_check_tree_linkage), > + KUNIT_CASE(of_unittest_overlay_high_level), > + {}, > +}; > > - return 0; > -} > -late_initcall(of_unittest); > +static struct kunit_module of_test_module = { > + .name = "of-test", > + .init = of_test_init, > + .test_cases = of_test_cases, > +}; > +module_test(of_test_module); > -- > 2.20.0.rc0.387.gc7a69e6b6c-goog >
On 11/28/18 12:56 PM, Rob Herring wrote: >> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig >> index ad3fcad4d75b8..f309399deac20 100644 >> --- a/drivers/of/Kconfig >> +++ b/drivers/of/Kconfig >> @@ -15,6 +15,7 @@ if OF >> config OF_UNITTEST >> bool "Device Tree runtime unit tests" >> depends on !SPARC >> + depends on KUNIT > Unless KUNIT has depends, better to be a select here. That's just style or taste. I would prefer to use depends instead of select, but that's also just my preference.
On Wed, Nov 28, 2018 at 12:56 PM Rob Herring <robh@kernel.org> wrote: > > On Wed, Nov 28, 2018 at 1:38 PM Brendan Higgins > <brendanhiggins@google.com> wrote: > > > > Migrate tests without any cleanup, or modifying test logic in anyway to > > run under KUnit using the KUnit expectation and assertion API. > > Nice! You beat me to it. This is probably going to conflict with what > is in the DT tree for 4.21. Also, please Cc the DT list for > drivers/of/ changes. Oh, I thought you were asking me to do it :-) In any case, I am happy to. Oh yeah, sorry about not CC'ing the list. Cheers
On Thu, Nov 29, 2018 at 4:40 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > On 11/28/18 12:56 PM, Rob Herring wrote: > >> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > >> index ad3fcad4d75b8..f309399deac20 100644 > >> --- a/drivers/of/Kconfig > >> +++ b/drivers/of/Kconfig > >> @@ -15,6 +15,7 @@ if OF > >> config OF_UNITTEST > >> bool "Device Tree runtime unit tests" > >> depends on !SPARC > >> + depends on KUNIT > > Unless KUNIT has depends, better to be a select here. > > That's just style or taste. I would prefer to use depends > instead of select, but that's also just my preference. I prefer depends too, but Rob is the maintainer here.
On 11/28/18 11:36 AM, Brendan Higgins wrote: > Migrate tests without any cleanup, or modifying test logic in anyway to > run under KUnit using the KUnit expectation and assertion API. > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > --- > drivers/of/Kconfig | 1 + > drivers/of/unittest.c | 1405 ++++++++++++++++++++++------------------- > 2 files changed, 752 insertions(+), 654 deletions(-) < snip > I am travelling and will not have an opportunity to properly review this patch, patch 18, or patch 19 until next week. -Frank
On Mon, Dec 3, 2018 at 6:14 PM Brendan Higgins <brendanhiggins@google.com> wrote: > > On Thu, Nov 29, 2018 at 4:40 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > > > On 11/28/18 12:56 PM, Rob Herring wrote: > > >> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > > >> index ad3fcad4d75b8..f309399deac20 100644 > > >> --- a/drivers/of/Kconfig > > >> +++ b/drivers/of/Kconfig > > >> @@ -15,6 +15,7 @@ if OF > > >> config OF_UNITTEST > > >> bool "Device Tree runtime unit tests" > > >> depends on !SPARC > > >> + depends on KUNIT > > > Unless KUNIT has depends, better to be a select here. > > > > That's just style or taste. I would prefer to use depends > > instead of select, but that's also just my preference. > > I prefer depends too, but Rob is the maintainer here. Well, we should be consistent, not the follow the whims of each maintainer. Rob
On Tue, Dec 4, 2018 at 5:41 AM Rob Herring <robh@kernel.org> wrote: > > On Mon, Dec 3, 2018 at 6:14 PM Brendan Higgins > <brendanhiggins@google.com> wrote: > > > > On Thu, Nov 29, 2018 at 4:40 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > > > > > On 11/28/18 12:56 PM, Rob Herring wrote: > > > >> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > > > >> index ad3fcad4d75b8..f309399deac20 100644 > > > >> --- a/drivers/of/Kconfig > > > >> +++ b/drivers/of/Kconfig > > > >> @@ -15,6 +15,7 @@ if OF > > > >> config OF_UNITTEST > > > >> bool "Device Tree runtime unit tests" > > > >> depends on !SPARC > > > >> + depends on KUNIT > > > > Unless KUNIT has depends, better to be a select here. > > > > > > That's just style or taste. I would prefer to use depends > > > instead of select, but that's also just my preference. > > > > I prefer depends too, but Rob is the maintainer here. > > Well, we should be consistent, not the follow the whims of each maintainer. Sorry, I don't think that came out the way I meant it. I don't really think we are consistent on this point across the kernel, and I don't feel very strongly about the point, so I was just looking to follow the path of least resistance. (I also just assumed Rob would keep us consistent within drivers/of/.) I figure if we are running unit tests from the test runner script or from an automated system, you won't be hunting for dependencies for a single test every time you want to run a test, so select doesn't make it easier to configure in most imagined use cases. KUNIT hypothetically should not depend on anything, so select should be safe to use. On the other hand, if we end up being wrong on this point and KUnit gains widespread adoption, I would prefer not to be in a position where I have to change a bunch of configs all over the kernel because this example got copied and pasted.
On Wed, Dec 5, 2018 at 5:43 PM Brendan Higgins <brendanhiggins@google.com> wrote: > > On Tue, Dec 4, 2018 at 5:41 AM Rob Herring <robh@kernel.org> wrote: > > > > On Mon, Dec 3, 2018 at 6:14 PM Brendan Higgins > > <brendanhiggins@google.com> wrote: > > > > > > On Thu, Nov 29, 2018 at 4:40 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > > > > > > > On 11/28/18 12:56 PM, Rob Herring wrote: > > > > >> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig > > > > >> index ad3fcad4d75b8..f309399deac20 100644 > > > > >> --- a/drivers/of/Kconfig > > > > >> +++ b/drivers/of/Kconfig > > > > >> @@ -15,6 +15,7 @@ if OF > > > > >> config OF_UNITTEST > > > > >> bool "Device Tree runtime unit tests" > > > > >> depends on !SPARC > > > > >> + depends on KUNIT > > > > > Unless KUNIT has depends, better to be a select here. > > > > > > > > That's just style or taste. I would prefer to use depends > > > > instead of select, but that's also just my preference. > > > > > > I prefer depends too, but Rob is the maintainer here. > > > > Well, we should be consistent, not the follow the whims of each maintainer. > > Sorry, I don't think that came out the way I meant it. I don't really > think we are consistent on this point across the kernel, and I don't > feel very strongly about the point, so I was just looking to follow > the path of least resistance. (I also just assumed Rob would keep us > consistent within drivers/of/.) I meant across unittests, we should be consistent. All unittests do either "depends on KUNIT" or "select KUNIT". The question I would ask is does KUNIT need to be user visible or is useful to enable without any unittests enabled? With depends, a user has 2 options to go enable vs. 1 with select. But if you want a global kill switch to turn off all unittests, then depends works better. > I figure if we are running unit tests from the test runner script or > from an automated system, you won't be hunting for dependencies for a > single test every time you want to run a test, so select doesn't make > it easier to configure in most imagined use cases. > > KUNIT hypothetically should not depend on anything, so select should > be safe to use. > > On the other hand, if we end up being wrong on this point and KUnit > gains widespread adoption, I would prefer not to be in a position > where I have to change a bunch of configs all over the kernel because > this example got copied and pasted. You'll be so happy that 100s of tests have been created using kunit, it won't be a big deal. :) In any case, I wouldn't spend more time on this. Rob
On Wed, Nov 28, 2018 at 12:56 PM Rob Herring <robh@kernel.org> wrote: > > On Wed, Nov 28, 2018 at 1:38 PM Brendan Higgins > <brendanhiggins@google.com> wrote: > > > > Migrate tests without any cleanup, or modifying test logic in anyway to > > run under KUnit using the KUnit expectation and assertion API. > > Nice! You beat me to it. This is probably going to conflict with what > is in the DT tree for 4.21. Also, please Cc the DT list for > drivers/of/ changes. > > Looks good to me, but a few mostly formatting comments below. I just realized that we never talked about your other comments, and I still have some questions. (Sorry, it was the last thing I looked at while getting v4 ready.) No worries if you don't get to it before I send v4 out, I just didn't want you to think I was ignoring you. > > > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > > --- > > drivers/of/Kconfig | 1 + > > drivers/of/unittest.c | 1405 ++++++++++++++++++++++------------------- > > 2 files changed, 752 insertions(+), 654 deletions(-) > > <snip> > > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c > > index 41b49716ac75f..a5ef44730ffdb 100644 > > --- a/drivers/of/unittest.c > > +++ b/drivers/of/unittest.c <snip> > > - > > -static void __init of_unittest_find_node_by_name(void) > > +static void of_unittest_find_node_by_name(struct kunit *test) > > Why do we have to drop __init everywhere? The tests run later? From the standpoint of a unit test __init doesn't really make any sense, right? I know that right now we are running as part of a kernel, but the goal should be that a unit test is not part of a kernel and we just include what we need. Even so, that's the future. For now, I did not put the KUnit infrastructure in the .init section because I didn't think it belonged there. In practice, KUnit only knows how to run during the init phase of the kernel, but I don't think it should be restricted there. You should be able to run tests whenever you want because you should be able to test anything right? I figured any restriction on that is misleading and will potentially get in the way at worst, and unnecessary at best especially since people shouldn't build a production kernel with all kinds of unit tests inside. > > > { > > struct device_node *np; > > const char *options, *name; > > <snip> > > > > > > - np = of_find_node_by_path("/testcase-data/missing-path"); > > - unittest(!np, "non-existent path returned node %pOF\n", np); > > + KUNIT_EXPECT_EQ_MSG(test, > > + of_find_node_by_path("/testcase-data/missing-path"), > > + NULL, > > + "non-existent path returned node %pOF\n", np); > > 1 tab indent would help with less vertical code (in general, not this > one so much). Will do. > > > of_node_put(np); > > > > - np = of_find_node_by_path("missing-alias"); > > - unittest(!np, "non-existent alias returned node %pOF\n", np); > > + KUNIT_EXPECT_EQ_MSG(test, of_find_node_by_path("missing-alias"), NULL, > > + "non-existent alias returned node %pOF\n", np); > > of_node_put(np); > > > > - np = of_find_node_by_path("testcase-alias/missing-path"); > > - unittest(!np, "non-existent alias with relative path returned node %pOF\n", np); > > + KUNIT_EXPECT_EQ_MSG(test, > > + of_find_node_by_path("testcase-alias/missing-path"), > > + NULL, > > + "non-existent alias with relative path returned node %pOF\n", > > + np); > > of_node_put(np); > > <snip> > > > > -static void __init of_unittest_property_string(void) > > +static void of_unittest_property_string(struct kunit *test) > > { > > const char *strings[4]; > > struct device_node *np; > > int rc; > > > > np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); > > - if (!np) { > > - pr_err("No testcase data in device tree\n"); > > - return; > > - } > > - > > - rc = of_property_match_string(np, "phandle-list-names", "first"); > > - unittest(rc == 0, "first expected:0 got:%i\n", rc); > > - rc = of_property_match_string(np, "phandle-list-names", "second"); > > - unittest(rc == 1, "second expected:1 got:%i\n", rc); > > - rc = of_property_match_string(np, "phandle-list-names", "third"); > > - unittest(rc == 2, "third expected:2 got:%i\n", rc); > > - rc = of_property_match_string(np, "phandle-list-names", "fourth"); > > - unittest(rc == -ENODATA, "unmatched string; rc=%i\n", rc); > > - rc = of_property_match_string(np, "missing-property", "blah"); > > - unittest(rc == -EINVAL, "missing property; rc=%i\n", rc); > > - rc = of_property_match_string(np, "empty-property", "blah"); > > - unittest(rc == -ENODATA, "empty property; rc=%i\n", rc); > > - rc = of_property_match_string(np, "unterminated-string", "blah"); > > - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); > > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > + > > + KUNIT_EXPECT_EQ(test, > > + of_property_match_string(np, > > + "phandle-list-names", > > + "first"), > > + 0); > > + KUNIT_EXPECT_EQ(test, > > + of_property_match_string(np, > > + "phandle-list-names", > > + "second"), > > + 1); > > Fewer lines on these would be better even if we go over 80 chars. On the of_property_match_string(...), I have no opinion. I will do whatever you like best. Nevertheless, as far as the KUNIT_EXPECT_*(...), I do have an opinion: I am trying to establish a good, readable convention. Given an expect statement structured as ``` KUNIT_EXPECT_*( test, expect_arg_0, ..., expect_arg_n, fmt_str, fmt_arg_0, ..., fmt_arg_n) ``` where `test` is the `struct kunit` context argument, `expect_arg_{0, ..., n}` are the arguments the expectations is being made about (so in the above example, `of_property_match_string(...)` and `1`), and `fmt_*` is the optional format string that comes at the end of some expectations. The pattern I had been trying to promote is the following: 1) If everything fits on 1 line, do that. 2) If you must make a line split, prefer to keep `test` on its own line, `expect_arg_{0, ..., n}` should be kept together, if possible, and the format string should follow the conventions already most commonly used with format strings. 3) If you must split up `expect_arg_{0, ..., n}` each argument should get its own line and should not share a line with either `test` or any `fmt_*`. The reason I care about this so much is because expectations should be extremely easy to read; they are the most important part of a unit test because they tell you what the test is verifying. I am not married to the formatting I proposed above, but I want something that will be extremely easy to identify the arguments that the expectation is on. Maybe that means that I need to add some syntactic fluff to make it clearer, I don't know, but this is definitely something we need to get right, especially in the earliest examples. > > > + KUNIT_EXPECT_EQ(test, > > + of_property_match_string(np, > > + "phandle-list-names", > > + "third"), > > + 2); > > + KUNIT_EXPECT_EQ_MSG(test, > > + of_property_match_string(np, > > + "phandle-list-names", > > + "fourth"), > > + -ENODATA, > > + "unmatched string"); > > + KUNIT_EXPECT_EQ_MSG(test, > > + of_property_match_string(np, > > + "missing-property", > > + "blah"), > > + -EINVAL, > > + "missing property"); > > + KUNIT_EXPECT_EQ_MSG(test, > > + of_property_match_string(np, > > + "empty-property", > > + "blah"), > > + -ENODATA, > > + "empty property"); > > + KUNIT_EXPECT_EQ_MSG(test, > > + of_property_match_string(np, > > + "unterminated-string", > > + "blah"), > > + -EILSEQ, > > + "unterminated string"); <snip> > > /* test insertion of a bus with parent devices */ > > -static void __init of_unittest_overlay_10(void) > > +static void of_unittest_overlay_10(struct kunit *test) > > { > > - int ret; > > char *child_path; > > > > /* device should disable */ > > - ret = of_unittest_apply_overlay_check(10, 10, 0, 1, PDEV_OVERLAY); > > - if (unittest(ret == 0, > > - "overlay test %d failed; overlay application\n", 10)) > > - return; > > + KUNIT_ASSERT_EQ_MSG(test, > > + of_unittest_apply_overlay_check(test, > > + 10, > > + 10, > > + 0, > > + 1, > > + PDEV_OVERLAY), > > I prefer putting multiple args on a line and having fewer lines. Looking at this now, I tend to agree, but I don't think I saw a consistent way to break them up for these functions. I figured there should be some type of pattern. > > > + 0, > > + "overlay test %d failed; overlay application\n", > > + 10); > > > > child_path = kasprintf(GFP_KERNEL, "%s/test-unittest101", > > unittest_path(10, PDEV_OVERLAY)); > > - if (unittest(child_path, "overlay test %d failed; kasprintf\n", 10)) > > - return; > > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, child_path); > > > > - ret = of_path_device_type_exists(child_path, PDEV_OVERLAY); > > + KUNIT_EXPECT_TRUE_MSG(test, > > + of_path_device_type_exists(child_path, > > + PDEV_OVERLAY), > > + "overlay test %d failed; no child device\n", 10); > > kfree(child_path); > > - > > - unittest(ret, "overlay test %d failed; no child device\n", 10); > > } <snip>
On Tue, Feb 12, 2019 at 7:44 PM Brendan Higgins <brendanhiggins@google.com> wrote: > > On Wed, Nov 28, 2018 at 12:56 PM Rob Herring <robh@kernel.org> wrote: > > > > On Wed, Nov 28, 2018 at 1:38 PM Brendan Higgins > > <brendanhiggins@google.com> wrote: > > > > > > Migrate tests without any cleanup, or modifying test logic in anyway to > > > run under KUnit using the KUnit expectation and assertion API. > > > > Nice! You beat me to it. This is probably going to conflict with what > > is in the DT tree for 4.21. Also, please Cc the DT list for > > drivers/of/ changes. > > > > Looks good to me, but a few mostly formatting comments below. > > I just realized that we never talked about your other comments, and I > still have some questions. (Sorry, it was the last thing I looked at > while getting v4 ready.) No worries if you don't get to it before I > send v4 out, I just didn't want you to think I was ignoring you. > > > > > > > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > > > --- > > > drivers/of/Kconfig | 1 + > > > drivers/of/unittest.c | 1405 ++++++++++++++++++++++------------------- > > > 2 files changed, 752 insertions(+), 654 deletions(-) > > > > <snip> > > > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c > > > index 41b49716ac75f..a5ef44730ffdb 100644 > > > --- a/drivers/of/unittest.c > > > +++ b/drivers/of/unittest.c > <snip> > > > - > > > -static void __init of_unittest_find_node_by_name(void) > > > +static void of_unittest_find_node_by_name(struct kunit *test) > > > > Why do we have to drop __init everywhere? The tests run later? > > From the standpoint of a unit test __init doesn't really make any > sense, right? I know that right now we are running as part of a > kernel, but the goal should be that a unit test is not part of a > kernel and we just include what we need. Well, the test only runs during boot and better to free the space when done with it. There was some desire to make it a kernel module and then we'd also need to get rid of __init too. > Even so, that's the future. For now, I did not put the KUnit > infrastructure in the .init section because I didn't think it belonged > there. In practice, KUnit only knows how to run during the init phase > of the kernel, but I don't think it should be restricted there. You > should be able to run tests whenever you want because you should be > able to test anything right? I figured any restriction on that is > misleading and will potentially get in the way at worst, and > unnecessary at best especially since people shouldn't build a > production kernel with all kinds of unit tests inside. More folks will run things if they can be enabled on production kernels. If size is the only issue, modules mitigate that. However, there's probably APIs to test which we don't want to export to modules. I think in general, we change things in the kernel when needed, not for something in the future. Changing __init is simple enough to do later. OTOH, things get copied and maybe this we don't want copied, so we can remove it if you want to. > <snip> > > > > > > -static void __init of_unittest_property_string(void) > > > +static void of_unittest_property_string(struct kunit *test) > > > { > > > const char *strings[4]; > > > struct device_node *np; > > > int rc; > > > > > > np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); > > > - if (!np) { > > > - pr_err("No testcase data in device tree\n"); > > > - return; > > > - } > > > - > > > - rc = of_property_match_string(np, "phandle-list-names", "first"); > > > - unittest(rc == 0, "first expected:0 got:%i\n", rc); > > > - rc = of_property_match_string(np, "phandle-list-names", "second"); > > > - unittest(rc == 1, "second expected:1 got:%i\n", rc); > > > - rc = of_property_match_string(np, "phandle-list-names", "third"); > > > - unittest(rc == 2, "third expected:2 got:%i\n", rc); > > > - rc = of_property_match_string(np, "phandle-list-names", "fourth"); > > > - unittest(rc == -ENODATA, "unmatched string; rc=%i\n", rc); > > > - rc = of_property_match_string(np, "missing-property", "blah"); > > > - unittest(rc == -EINVAL, "missing property; rc=%i\n", rc); > > > - rc = of_property_match_string(np, "empty-property", "blah"); > > > - unittest(rc == -ENODATA, "empty property; rc=%i\n", rc); > > > - rc = of_property_match_string(np, "unterminated-string", "blah"); > > > - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); > > > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > > + > > > + KUNIT_EXPECT_EQ(test, > > > + of_property_match_string(np, > > > + "phandle-list-names", > > > + "first"), > > > + 0); > > > + KUNIT_EXPECT_EQ(test, > > > + of_property_match_string(np, > > > + "phandle-list-names", > > > + "second"), > > > + 1); > > > > Fewer lines on these would be better even if we go over 80 chars. > > On the of_property_match_string(...), I have no opinion. I will do > whatever you like best. > > Nevertheless, as far as the KUNIT_EXPECT_*(...), I do have an opinion: I am > trying to establish a good, readable convention. Given an expect statement > structured as > ``` > KUNIT_EXPECT_*( > test, > expect_arg_0, ..., expect_arg_n, > fmt_str, fmt_arg_0, ..., fmt_arg_n) > ``` > where `test` is the `struct kunit` context argument, `expect_arg_{0, ..., n}` > are the arguments the expectations is being made about (so in the above example, > `of_property_match_string(...)` and `1`), and `fmt_*` is the optional format > string that comes at the end of some expectations. > > The pattern I had been trying to promote is the following: > > 1) If everything fits on 1 line, do that. > 2) If you must make a line split, prefer to keep `test` on its own line, > `expect_arg_{0, ..., n}` should be kept together, if possible, and the format > string should follow the conventions already most commonly used with format > strings. > 3) If you must split up `expect_arg_{0, ..., n}` each argument should get its > own line and should not share a line with either `test` or any `fmt_*`. You'd better write a checkpatch.pl check or else good luck enforcing that. :) > The reason I care about this so much is because expectations should be > extremely easy to read; they are the most important part of a unit > test because they tell you what the test is verifying. I am not > married to the formatting I proposed above, but I want something that > will be extremely easy to identify the arguments that the expectation > is on. Maybe that means that I need to add some syntactic fluff to > make it clearer, I don't know, but this is definitely something we > need to get right, especially in the earliest examples. Makes sense. I think putting the test (of_property_match_string) on one line furthers the readability. Rob
On Thu, Feb 14, 2019 at 12:10 PM Rob Herring <robh@kernel.org> wrote: > > On Tue, Feb 12, 2019 at 7:44 PM Brendan Higgins > <brendanhiggins@google.com> wrote: > > > > On Wed, Nov 28, 2018 at 12:56 PM Rob Herring <robh@kernel.org> wrote: > > > > > > On Wed, Nov 28, 2018 at 1:38 PM Brendan Higgins > > > <brendanhiggins@google.com> wrote: > > > > > > > > Migrate tests without any cleanup, or modifying test logic in anyway to > > > > run under KUnit using the KUnit expectation and assertion API. > > > > > > Nice! You beat me to it. This is probably going to conflict with what > > > is in the DT tree for 4.21. Also, please Cc the DT list for > > > drivers/of/ changes. > > > > > > Looks good to me, but a few mostly formatting comments below. > > > > I just realized that we never talked about your other comments, and I > > still have some questions. (Sorry, it was the last thing I looked at > > while getting v4 ready.) No worries if you don't get to it before I > > send v4 out, I just didn't want you to think I was ignoring you. > > > > > > > > > > > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > > > > --- > > > > drivers/of/Kconfig | 1 + > > > > drivers/of/unittest.c | 1405 ++++++++++++++++++++++------------------- > > > > 2 files changed, 752 insertions(+), 654 deletions(-) > > > > > > <snip> > > > > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c > > > > index 41b49716ac75f..a5ef44730ffdb 100644 > > > > --- a/drivers/of/unittest.c > > > > +++ b/drivers/of/unittest.c > > <snip> > > > > - > > > > -static void __init of_unittest_find_node_by_name(void) > > > > +static void of_unittest_find_node_by_name(struct kunit *test) > > > > > > Why do we have to drop __init everywhere? The tests run later? > > > > From the standpoint of a unit test __init doesn't really make any > > sense, right? I know that right now we are running as part of a > > kernel, but the goal should be that a unit test is not part of a > > kernel and we just include what we need. > > Well, the test only runs during boot and better to free the space when > done with it. There was some desire to make it a kernel module and > then we'd also need to get rid of __init too. > > > Even so, that's the future. For now, I did not put the KUnit > > infrastructure in the .init section because I didn't think it belonged > > there. In practice, KUnit only knows how to run during the init phase > > of the kernel, but I don't think it should be restricted there. You > > should be able to run tests whenever you want because you should be > > able to test anything right? I figured any restriction on that is > > misleading and will potentially get in the way at worst, and > > unnecessary at best especially since people shouldn't build a > > production kernel with all kinds of unit tests inside. > > More folks will run things if they can be enabled on production > kernels. If size is the only issue, modules mitigate that. However, > there's probably APIs to test which we don't want to export to > modules. > > I think in general, we change things in the kernel when needed, not > for something in the future. Changing __init is simple enough to do > later. > > OTOH, things get copied and maybe this we don't want copied, so we can > remove it if you want to. Mmmm...I just realized that the patch I sent you the other day makes this patch unhappy because unflatten_device_tree is in the .init section. So I will need to fix that. I still think that the correct course of action is to make KUnit non init. Luis pointed out in another thread that to be 100% sure that everything will be properly initialized, KUnit must be able to run after all initialization takes place. > > > <snip> > > > > > > > > -static void __init of_unittest_property_string(void) > > > > +static void of_unittest_property_string(struct kunit *test) > > > > { > > > > const char *strings[4]; > > > > struct device_node *np; > > > > int rc; > > > > > > > > np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); > > > > - if (!np) { > > > > - pr_err("No testcase data in device tree\n"); > > > > - return; > > > > - } > > > > - > > > > - rc = of_property_match_string(np, "phandle-list-names", "first"); > > > > - unittest(rc == 0, "first expected:0 got:%i\n", rc); > > > > - rc = of_property_match_string(np, "phandle-list-names", "second"); > > > > - unittest(rc == 1, "second expected:1 got:%i\n", rc); > > > > - rc = of_property_match_string(np, "phandle-list-names", "third"); > > > > - unittest(rc == 2, "third expected:2 got:%i\n", rc); > > > > - rc = of_property_match_string(np, "phandle-list-names", "fourth"); > > > > - unittest(rc == -ENODATA, "unmatched string; rc=%i\n", rc); > > > > - rc = of_property_match_string(np, "missing-property", "blah"); > > > > - unittest(rc == -EINVAL, "missing property; rc=%i\n", rc); > > > > - rc = of_property_match_string(np, "empty-property", "blah"); > > > > - unittest(rc == -ENODATA, "empty property; rc=%i\n", rc); > > > > - rc = of_property_match_string(np, "unterminated-string", "blah"); > > > > - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); > > > > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > > > + > > > > + KUNIT_EXPECT_EQ(test, > > > > + of_property_match_string(np, > > > > + "phandle-list-names", > > > > + "first"), > > > > + 0); > > > > + KUNIT_EXPECT_EQ(test, > > > > + of_property_match_string(np, > > > > + "phandle-list-names", > > > > + "second"), > > > > + 1); > > > > > > Fewer lines on these would be better even if we go over 80 chars. > > > > On the of_property_match_string(...), I have no opinion. I will do > > whatever you like best. > > > > Nevertheless, as far as the KUNIT_EXPECT_*(...), I do have an opinion: I am > > trying to establish a good, readable convention. Given an expect statement > > structured as > > ``` > > KUNIT_EXPECT_*( > > test, > > expect_arg_0, ..., expect_arg_n, > > fmt_str, fmt_arg_0, ..., fmt_arg_n) > > ``` > > where `test` is the `struct kunit` context argument, `expect_arg_{0, ..., n}` > > are the arguments the expectations is being made about (so in the above example, > > `of_property_match_string(...)` and `1`), and `fmt_*` is the optional format > > string that comes at the end of some expectations. > > > > The pattern I had been trying to promote is the following: > > > > 1) If everything fits on 1 line, do that. > > 2) If you must make a line split, prefer to keep `test` on its own line, > > `expect_arg_{0, ..., n}` should be kept together, if possible, and the format > > string should follow the conventions already most commonly used with format > > strings. > > 3) If you must split up `expect_arg_{0, ..., n}` each argument should get its > > own line and should not share a line with either `test` or any `fmt_*`. > > You'd better write a checkpatch.pl check or else good luck enforcing that. :) Absolutely. Well I already had to touch checkpatch.pl for something else, so at least I know roughly what I am getting myself into. > > > The reason I care about this so much is because expectations should be > > extremely easy to read; they are the most important part of a unit > > test because they tell you what the test is verifying. I am not > > married to the formatting I proposed above, but I want something that > > will be extremely easy to identify the arguments that the expectation > > is on. Maybe that means that I need to add some syntactic fluff to > > make it clearer, I don't know, but this is definitely something we > > need to get right, especially in the earliest examples. > > Makes sense. I think putting the test (of_property_match_string) on > one line furthers the readability. Fair enough, I tried to apply your comments the best that I could on v4, but I think I will probably need to make another pass (especially given the init thing). Anyway, let's continue the discussion on v4. Cheers
On 2/12/19 5:44 PM, Brendan Higgins wrote: > On Wed, Nov 28, 2018 at 12:56 PM Rob Herring <robh@kernel.org> wrote: >> >> On Wed, Nov 28, 2018 at 1:38 PM Brendan Higgins >> <brendanhiggins@google.com> wrote: >>> >>> Migrate tests without any cleanup, or modifying test logic in anyway to >>> run under KUnit using the KUnit expectation and assertion API. >> >> Nice! You beat me to it. This is probably going to conflict with what >> is in the DT tree for 4.21. Also, please Cc the DT list for >> drivers/of/ changes. >> >> Looks good to me, but a few mostly formatting comments below. > > I just realized that we never talked about your other comments, and I > still have some questions. (Sorry, it was the last thing I looked at > while getting v4 ready.) No worries if you don't get to it before I > send v4 out, I just didn't want you to think I was ignoring you. > >> >>> >>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com> >>> --- >>> drivers/of/Kconfig | 1 + >>> drivers/of/unittest.c | 1405 ++++++++++++++++++++++------------------- >>> 2 files changed, 752 insertions(+), 654 deletions(-) >>> > <snip> >>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c >>> index 41b49716ac75f..a5ef44730ffdb 100644 >>> --- a/drivers/of/unittest.c >>> +++ b/drivers/of/unittest.c > <snip> >>> - >>> -static void __init of_unittest_find_node_by_name(void) >>> +static void of_unittest_find_node_by_name(struct kunit *test) >> >> Why do we have to drop __init everywhere? The tests run later? > >>From the standpoint of a unit test __init doesn't really make any > sense, right? I know that right now we are running as part of a > kernel, but the goal should be that a unit test is not part of a > kernel and we just include what we need. > > Even so, that's the future. For now, I did not put the KUnit > infrastructure in the .init section because I didn't think it belonged > there. In practice, KUnit only knows how to run during the init phase > of the kernel, but I don't think it should be restricted there. You > should be able to run tests whenever you want because you should be > able to test anything right? I figured any restriction on that is > misleading and will potentially get in the way at worst, and > unnecessary at best especially since people shouldn't build a > production kernel with all kinds of unit tests inside. > >> >>> { >>> struct device_node *np; >>> const char *options, *name; >>> > <snip> >>> >>> >>> - np = of_find_node_by_path("/testcase-data/missing-path"); >>> - unittest(!np, "non-existent path returned node %pOF\n", np); >>> + KUNIT_EXPECT_EQ_MSG(test, >>> + of_find_node_by_path("/testcase-data/missing-path"), >>> + NULL, >>> + "non-existent path returned node %pOF\n", np); >> >> 1 tab indent would help with less vertical code (in general, not this >> one so much). > > Will do. > >> >>> of_node_put(np); >>> >>> - np = of_find_node_by_path("missing-alias"); >>> - unittest(!np, "non-existent alias returned node %pOF\n", np); >>> + KUNIT_EXPECT_EQ_MSG(test, of_find_node_by_path("missing-alias"), NULL, >>> + "non-existent alias returned node %pOF\n", np); >>> of_node_put(np); >>> >>> - np = of_find_node_by_path("testcase-alias/missing-path"); >>> - unittest(!np, "non-existent alias with relative path returned node %pOF\n", np); >>> + KUNIT_EXPECT_EQ_MSG(test, >>> + of_find_node_by_path("testcase-alias/missing-path"), >>> + NULL, >>> + "non-existent alias with relative path returned node %pOF\n", >>> + np); >>> of_node_put(np); >>> > <snip> >>> >>> -static void __init of_unittest_property_string(void) >>> +static void of_unittest_property_string(struct kunit *test) >>> { >>> const char *strings[4]; >>> struct device_node *np; >>> int rc; >>> >>> np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); >>> - if (!np) { >>> - pr_err("No testcase data in device tree\n"); >>> - return; >>> - } >>> - >>> - rc = of_property_match_string(np, "phandle-list-names", "first"); >>> - unittest(rc == 0, "first expected:0 got:%i\n", rc); >>> - rc = of_property_match_string(np, "phandle-list-names", "second"); >>> - unittest(rc == 1, "second expected:1 got:%i\n", rc); >>> - rc = of_property_match_string(np, "phandle-list-names", "third"); >>> - unittest(rc == 2, "third expected:2 got:%i\n", rc); >>> - rc = of_property_match_string(np, "phandle-list-names", "fourth"); >>> - unittest(rc == -ENODATA, "unmatched string; rc=%i\n", rc); >>> - rc = of_property_match_string(np, "missing-property", "blah"); >>> - unittest(rc == -EINVAL, "missing property; rc=%i\n", rc); >>> - rc = of_property_match_string(np, "empty-property", "blah"); >>> - unittest(rc == -ENODATA, "empty property; rc=%i\n", rc); >>> - rc = of_property_match_string(np, "unterminated-string", "blah"); >>> - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); >>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); >>> + >>> + KUNIT_EXPECT_EQ(test, >>> + of_property_match_string(np, >>> + "phandle-list-names", >>> + "first"), >>> + 0); >>> + KUNIT_EXPECT_EQ(test, >>> + of_property_match_string(np, >>> + "phandle-list-names", >>> + "second"), >>> + 1); >> >> Fewer lines on these would be better even if we go over 80 chars. Agreed. unittest.c already is a greater than 80 char file in general, and is a file that benefits from that. > On the of_property_match_string(...), I have no opinion. I will do > whatever you like best. > > Nevertheless, as far as the KUNIT_EXPECT_*(...), I do have an opinion: I am > trying to establish a good, readable convention. Given an expect statement > structured as > ``` > KUNIT_EXPECT_*( > test, > expect_arg_0, ..., expect_arg_n, > fmt_str, fmt_arg_0, ..., fmt_arg_n) > ``` > where `test` is the `struct kunit` context argument, `expect_arg_{0, ..., n}` > are the arguments the expectations is being made about (so in the above example, > `of_property_match_string(...)` and `1`), and `fmt_*` is the optional format > string that comes at the end of some expectations. > > The pattern I had been trying to promote is the following: > > 1) If everything fits on 1 line, do that. > 2) If you must make a line split, prefer to keep `test` on its own line, > `expect_arg_{0, ..., n}` should be kept together, if possible, and the format > string should follow the conventions already most commonly used with format > strings. > 3) If you must split up `expect_arg_{0, ..., n}` each argument should get its > own line and should not share a line with either `test` or any `fmt_*`. > > The reason I care about this so much is because expectations should be > extremely easy to read; they are the most important part of a unit > test because they tell you what the test is verifying. I am not > married to the formatting I proposed above, but I want something that > will be extremely easy to identify the arguments that the expectation > is on. Maybe that means that I need to add some syntactic fluff to > make it clearer, I don't know, but this is definitely something we > need to get right, especially in the earliest examples. I will probably raise the ire of the kernel formatting rule makers by offering what I think is a _much_ more readable format __for this specific case__. In other words for drivers/of/unittest.c. If you can not make your mail window _very_ wide, or if this email has been line wrapped, this example will not be clear. Two possible formats: ### ----- version 1, as created by the patch series static void of_unittest_property_string(struct kunit *test) { const char *strings[4]; struct device_node *np; int rc; np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); KUNIT_EXPECT_EQ( test, of_property_match_string(np, "phandle-list-names", "first"), 0); KUNIT_EXPECT_EQ( test, of_property_match_string(np, "phandle-list-names", "second"), 1); KUNIT_EXPECT_EQ( test, of_property_match_string(np, "phandle-list-names", "third"), 2); KUNIT_EXPECT_EQ_MSG( test, of_property_match_string(np, "phandle-list-names", "fourth"), -ENODATA, "unmatched string"); KUNIT_EXPECT_EQ_MSG( test, of_property_match_string(np, "missing-property", "blah"), -EINVAL, "missing property"); KUNIT_EXPECT_EQ_MSG( test, of_property_match_string(np, "empty-property", "blah"), -ENODATA, "empty property"); KUNIT_EXPECT_EQ_MSG( test, of_property_match_string(np, "unterminated-string", "blah"), -EILSEQ, "unterminated string"); /* of_property_count_strings() tests */ KUNIT_EXPECT_EQ(test, of_property_count_strings(np, "string-property"), 1); KUNIT_EXPECT_EQ(test, of_property_count_strings(np, "phandle-list-names"), 3); KUNIT_EXPECT_EQ_MSG( test, of_property_count_strings(np, "unterminated-string"), -EILSEQ, "unterminated string"); KUNIT_EXPECT_EQ_MSG( test, of_property_count_strings(np, "unterminated-string-list"), -EILSEQ, "unterminated string array"); ### ----- version 2, modified to use really long lines static void of_unittest_property_string(struct kunit *test) { const char *strings[4]; struct device_node *np; int rc; np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); KUNIT_EXPECT_EQ( test, of_property_match_string(np, "phandle-list-names", "first"), 0); KUNIT_EXPECT_EQ( test, of_property_match_string(np, "phandle-list-names", "second"), 1); KUNIT_EXPECT_EQ( test, of_property_match_string(np, "phandle-list-names", "third"), 2); KUNIT_EXPECT_EQ_MSG(test, of_property_match_string(np, "phandle-list-names", "fourth"), -ENODATA, "unmatched string"); KUNIT_EXPECT_EQ_MSG(test, of_property_match_string(np, "missing-property", "blah"), -EINVAL, "missing property"); KUNIT_EXPECT_EQ_MSG(test, of_property_match_string(np, "empty-property", "blah"), -ENODATA, "empty property"); KUNIT_EXPECT_EQ_MSG(test, of_property_match_string(np, "unterminated-string", "blah"), -EILSEQ, "unterminated string"); /* of_property_count_strings() tests */ KUNIT_EXPECT_EQ( test, of_property_count_strings(np, "string-property"), 1); KUNIT_EXPECT_EQ( test, of_property_count_strings(np, "phandle-list-names"), 3); KUNIT_EXPECT_EQ_MSG(test, of_property_count_strings(np, "unterminated-string"), -EILSEQ, "unterminated string"); KUNIT_EXPECT_EQ_MSG(test, of_property_count_strings(np, "unterminated-string-list"), -EILSEQ, "unterminated string array"); ------------------------ ------------------------------------------------------------- -------------------------------------- ^ ^ ^ | | | | | | mostly boilerplate what is being tested expected result, error message (can vary in relop and _MSG or not) In my opinion, the second version is much more readable. It is easy to see the differences in the boilerplate. It is easy to see what is being tested, and how the arguments of the tested function vary for each test. It is easy to see the expected result and error message. The entire block fits into a single short window (though much wider). - Frank >> >>> + KUNIT_EXPECT_EQ(test, >>> + of_property_match_string(np, >>> + "phandle-list-names", >>> + "third"), >>> + 2); >>> + KUNIT_EXPECT_EQ_MSG(test, >>> + of_property_match_string(np, >>> + "phandle-list-names", >>> + "fourth"), >>> + -ENODATA, >>> + "unmatched string"); >>> + KUNIT_EXPECT_EQ_MSG(test, >>> + of_property_match_string(np, >>> + "missing-property", >>> + "blah"), >>> + -EINVAL, >>> + "missing property"); >>> + KUNIT_EXPECT_EQ_MSG(test, >>> + of_property_match_string(np, >>> + "empty-property", >>> + "blah"), >>> + -ENODATA, >>> + "empty property"); >>> + KUNIT_EXPECT_EQ_MSG(test, >>> + of_property_match_string(np, >>> + "unterminated-string", >>> + "blah"), >>> + -EILSEQ, >>> + "unterminated string"); > <snip> >>> /* test insertion of a bus with parent devices */ >>> -static void __init of_unittest_overlay_10(void) >>> +static void of_unittest_overlay_10(struct kunit *test) >>> { >>> - int ret; >>> char *child_path; >>> >>> /* device should disable */ >>> - ret = of_unittest_apply_overlay_check(10, 10, 0, 1, PDEV_OVERLAY); >>> - if (unittest(ret == 0, >>> - "overlay test %d failed; overlay application\n", 10)) >>> - return; >>> + KUNIT_ASSERT_EQ_MSG(test, >>> + of_unittest_apply_overlay_check(test, >>> + 10, >>> + 10, >>> + 0, >>> + 1, >>> + PDEV_OVERLAY), >> >> I prefer putting multiple args on a line and having fewer lines. > > Looking at this now, I tend to agree, but I don't think I saw a > consistent way to break them up for these functions. I figured there > should be some type of pattern. > >> >>> + 0, >>> + "overlay test %d failed; overlay application\n", >>> + 10); >>> >>> child_path = kasprintf(GFP_KERNEL, "%s/test-unittest101", >>> unittest_path(10, PDEV_OVERLAY)); >>> - if (unittest(child_path, "overlay test %d failed; kasprintf\n", 10)) >>> - return; >>> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, child_path); >>> >>> - ret = of_path_device_type_exists(child_path, PDEV_OVERLAY); >>> + KUNIT_EXPECT_TRUE_MSG(test, >>> + of_path_device_type_exists(child_path, >>> + PDEV_OVERLAY), >>> + "overlay test %d failed; no child device\n", 10); >>> kfree(child_path); >>> - >>> - unittest(ret, "overlay test %d failed; no child device\n", 10); >>> } > <snip> >
On Mon, Feb 18, 2019 at 2:56 PM Frank Rowand <frowand.list@gmail.com> wrote: > > On 2/12/19 5:44 PM, Brendan Higgins wrote: > > On Wed, Nov 28, 2018 at 12:56 PM Rob Herring <robh@kernel.org> wrote: > >> > >> On Wed, Nov 28, 2018 at 1:38 PM Brendan Higgins > >> <brendanhiggins@google.com> wrote: <snip> > >>> --- > >>> drivers/of/Kconfig | 1 + > >>> drivers/of/unittest.c | 1405 ++++++++++++++++++++++------------------- > >>> 2 files changed, 752 insertions(+), 654 deletions(-) > >>> > > <snip> > >>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c > >>> index 41b49716ac75f..a5ef44730ffdb 100644 > >>> --- a/drivers/of/unittest.c > >>> +++ b/drivers/of/unittest.c <snip> > >>> + > >>> + KUNIT_EXPECT_EQ(test, > >>> + of_property_match_string(np, > >>> + "phandle-list-names", > >>> + "first"), > >>> + 0); > >>> + KUNIT_EXPECT_EQ(test, > >>> + of_property_match_string(np, > >>> + "phandle-list-names", > >>> + "second"), > >>> + 1); > >> > >> Fewer lines on these would be better even if we go over 80 chars. > > Agreed. unittest.c already is a greater than 80 char file in general, and > is a file that benefits from that. > Noted. > > > On the of_property_match_string(...), I have no opinion. I will do > > whatever you like best. > > > > Nevertheless, as far as the KUNIT_EXPECT_*(...), I do have an opinion: I am > > trying to establish a good, readable convention. Given an expect statement > > structured as > > ``` > > KUNIT_EXPECT_*( > > test, > > expect_arg_0, ..., expect_arg_n, > > fmt_str, fmt_arg_0, ..., fmt_arg_n) > > ``` > > where `test` is the `struct kunit` context argument, `expect_arg_{0, ..., n}` > > are the arguments the expectations is being made about (so in the above example, > > `of_property_match_string(...)` and `1`), and `fmt_*` is the optional format > > string that comes at the end of some expectations. > > > > The pattern I had been trying to promote is the following: > > > > 1) If everything fits on 1 line, do that. > > 2) If you must make a line split, prefer to keep `test` on its own line, > > `expect_arg_{0, ..., n}` should be kept together, if possible, and the format > > string should follow the conventions already most commonly used with format > > strings. > > 3) If you must split up `expect_arg_{0, ..., n}` each argument should get its > > own line and should not share a line with either `test` or any `fmt_*`. > > > > The reason I care about this so much is because expectations should be > > extremely easy to read; they are the most important part of a unit > > test because they tell you what the test is verifying. I am not > > married to the formatting I proposed above, but I want something that > > will be extremely easy to identify the arguments that the expectation > > is on. Maybe that means that I need to add some syntactic fluff to > > make it clearer, I don't know, but this is definitely something we > > need to get right, especially in the earliest examples. > > I will probably raise the ire of the kernel formatting rule makers by offering > what I think is a _much_ more readable format __for this specific case__. > In other words for drivers/of/unittest.c. > > If you can not make your mail window _very_ wide, or if this email has been > line wrapped, this example will not be clear. > > Two possible formats: > > > ### ----- version 1, as created by the patch series > > static void of_unittest_property_string(struct kunit *test) > { > const char *strings[4]; > struct device_node *np; > int rc; > > np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > KUNIT_EXPECT_EQ( > test, > of_property_match_string(np, "phandle-list-names", "first"), > 0); > KUNIT_EXPECT_EQ( > test, > of_property_match_string(np, "phandle-list-names", "second"), > 1); > KUNIT_EXPECT_EQ( > test, > of_property_match_string(np, "phandle-list-names", "third"), > 2); > KUNIT_EXPECT_EQ_MSG( > test, > of_property_match_string(np, "phandle-list-names", "fourth"), > -ENODATA, > "unmatched string"); > KUNIT_EXPECT_EQ_MSG( > test, > of_property_match_string(np, "missing-property", "blah"), > -EINVAL, > "missing property"); > KUNIT_EXPECT_EQ_MSG( > test, > of_property_match_string(np, "empty-property", "blah"), > -ENODATA, > "empty property"); > KUNIT_EXPECT_EQ_MSG( > test, > of_property_match_string(np, "unterminated-string", "blah"), > -EILSEQ, > "unterminated string"); > > /* of_property_count_strings() tests */ > KUNIT_EXPECT_EQ(test, > of_property_count_strings(np, "string-property"), 1); > KUNIT_EXPECT_EQ(test, > of_property_count_strings(np, "phandle-list-names"), 3); > KUNIT_EXPECT_EQ_MSG( > test, > of_property_count_strings(np, "unterminated-string"), -EILSEQ, > "unterminated string"); > KUNIT_EXPECT_EQ_MSG( > test, > of_property_count_strings(np, "unterminated-string-list"), > -EILSEQ, > "unterminated string array"); > > > > > ### ----- version 2, modified to use really long lines > > static void of_unittest_property_string(struct kunit *test) > { > const char *strings[4]; > struct device_node *np; > int rc; > > np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); > > KUNIT_EXPECT_EQ( test, of_property_match_string(np, "phandle-list-names", "first"), 0); > KUNIT_EXPECT_EQ( test, of_property_match_string(np, "phandle-list-names", "second"), 1); > KUNIT_EXPECT_EQ( test, of_property_match_string(np, "phandle-list-names", "third"), 2); > KUNIT_EXPECT_EQ_MSG(test, of_property_match_string(np, "phandle-list-names", "fourth"), -ENODATA, "unmatched string"); > KUNIT_EXPECT_EQ_MSG(test, of_property_match_string(np, "missing-property", "blah"), -EINVAL, "missing property"); > KUNIT_EXPECT_EQ_MSG(test, of_property_match_string(np, "empty-property", "blah"), -ENODATA, "empty property"); > KUNIT_EXPECT_EQ_MSG(test, of_property_match_string(np, "unterminated-string", "blah"), -EILSEQ, "unterminated string"); > > /* of_property_count_strings() tests */ > KUNIT_EXPECT_EQ( test, of_property_count_strings(np, "string-property"), 1); > KUNIT_EXPECT_EQ( test, of_property_count_strings(np, "phandle-list-names"), 3); > KUNIT_EXPECT_EQ_MSG(test, of_property_count_strings(np, "unterminated-string"), -EILSEQ, "unterminated string"); > KUNIT_EXPECT_EQ_MSG(test, of_property_count_strings(np, "unterminated-string-list"), -EILSEQ, "unterminated string array"); > > > ------------------------ ------------------------------------------------------------- -------------------------------------- > ^ ^ ^ > | | | > | | | > mostly boilerplate what is being tested expected result, error message > (can vary in relop > and _MSG or not) > > In my opinion, the second version is much more readable. It is easy to see the > differences in the boilerplate. It is easy to see what is being tested, and how > the arguments of the tested function vary for each test. It is easy to see the > expected result and error message. The entire block fits into a single short > window (though much wider). I have no opinion on the over 80 char thing, so as long as everyone else is okay with it, I have no complaints. Cheers
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index ad3fcad4d75b8..f309399deac20 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -15,6 +15,7 @@ if OF config OF_UNITTEST bool "Device Tree runtime unit tests" depends on !SPARC + depends on KUNIT select IRQ_DOMAIN select OF_EARLY_FLATTREE select OF_RESOLVE diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 41b49716ac75f..a5ef44730ffdb 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -26,186 +26,187 @@ #include <linux/bitops.h> +#include <kunit/test.h> + #include "of_private.h" -static struct unittest_results { - int passed; - int failed; -} unittest_results; - -#define unittest(result, fmt, ...) ({ \ - bool failed = !(result); \ - if (failed) { \ - unittest_results.failed++; \ - pr_err("FAIL %s():%i " fmt, __func__, __LINE__, ##__VA_ARGS__); \ - } else { \ - unittest_results.passed++; \ - pr_debug("pass %s():%i\n", __func__, __LINE__); \ - } \ - failed; \ -}) - -static void __init of_unittest_find_node_by_name(void) +static void of_unittest_find_node_by_name(struct kunit *test) { struct device_node *np; const char *options, *name; np = of_find_node_by_path("/testcase-data"); name = kasprintf(GFP_KERNEL, "%pOF", np); - unittest(np && !strcmp("/testcase-data", name), - "find /testcase-data failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); + KUNIT_EXPECT_STREQ_MSG(test, "/testcase-data", name, + "find /testcase-data failed\n"); of_node_put(np); kfree(name); /* Test if trailing '/' works */ - np = of_find_node_by_path("/testcase-data/"); - unittest(!np, "trailing '/' on /testcase-data/ should fail\n"); + KUNIT_EXPECT_EQ_MSG(test, of_find_node_by_path("/testcase-data/"), NULL, + "trailing '/' on /testcase-data/ should fail\n"); np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); name = kasprintf(GFP_KERNEL, "%pOF", np); - unittest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", name), - "find /testcase-data/phandle-tests/consumer-a failed\n"); + KUNIT_EXPECT_STREQ_MSG(test, + "/testcase-data/phandle-tests/consumer-a", name, + "find /testcase-data/phandle-tests/consumer-a failed\n"); of_node_put(np); kfree(name); np = of_find_node_by_path("testcase-alias"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); name = kasprintf(GFP_KERNEL, "%pOF", np); - unittest(np && !strcmp("/testcase-data", name), - "find testcase-alias failed\n"); + KUNIT_EXPECT_STREQ_MSG(test, "/testcase-data", name, + "find testcase-alias failed\n"); of_node_put(np); kfree(name); /* Test if trailing '/' works on aliases */ - np = of_find_node_by_path("testcase-alias/"); - unittest(!np, "trailing '/' on testcase-alias/ should fail\n"); + KUNIT_EXPECT_EQ_MSG(test, of_find_node_by_path("testcase-alias/"), NULL, + "trailing '/' on testcase-alias/ should fail\n"); np = of_find_node_by_path("testcase-alias/phandle-tests/consumer-a"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); name = kasprintf(GFP_KERNEL, "%pOF", np); - unittest(np && !strcmp("/testcase-data/phandle-tests/consumer-a", name), - "find testcase-alias/phandle-tests/consumer-a failed\n"); + KUNIT_EXPECT_STREQ_MSG(test, + "/testcase-data/phandle-tests/consumer-a", name, + "find testcase-alias/phandle-tests/consumer-a failed\n"); of_node_put(np); kfree(name); - np = of_find_node_by_path("/testcase-data/missing-path"); - unittest(!np, "non-existent path returned node %pOF\n", np); + KUNIT_EXPECT_EQ_MSG(test, + of_find_node_by_path("/testcase-data/missing-path"), + NULL, + "non-existent path returned node %pOF\n", np); of_node_put(np); - np = of_find_node_by_path("missing-alias"); - unittest(!np, "non-existent alias returned node %pOF\n", np); + KUNIT_EXPECT_EQ_MSG(test, of_find_node_by_path("missing-alias"), NULL, + "non-existent alias returned node %pOF\n", np); of_node_put(np); - np = of_find_node_by_path("testcase-alias/missing-path"); - unittest(!np, "non-existent alias with relative path returned node %pOF\n", np); + KUNIT_EXPECT_EQ_MSG(test, + of_find_node_by_path("testcase-alias/missing-path"), + NULL, + "non-existent alias with relative path returned node %pOF\n", + np); of_node_put(np); np = of_find_node_opts_by_path("/testcase-data:testoption", &options); - unittest(np && !strcmp("testoption", options), - "option path test failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); + KUNIT_EXPECT_STREQ_MSG(test, "testoption", options, + "option path test failed\n"); of_node_put(np); np = of_find_node_opts_by_path("/testcase-data:test/option", &options); - unittest(np && !strcmp("test/option", options), - "option path test, subcase #1 failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); + KUNIT_EXPECT_STREQ_MSG(test, "test/option", options, + "option path test, subcase #1 failed\n"); of_node_put(np); np = of_find_node_opts_by_path("/testcase-data/testcase-device1:test/option", &options); - unittest(np && !strcmp("test/option", options), - "option path test, subcase #2 failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); + KUNIT_EXPECT_STREQ_MSG(test, "test/option", options, + "option path test, subcase #2 failed\n"); of_node_put(np); np = of_find_node_opts_by_path("/testcase-data:testoption", NULL); - unittest(np, "NULL option path test failed\n"); + KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG(test, np, + "NULL option path test failed\n"); of_node_put(np); np = of_find_node_opts_by_path("testcase-alias:testaliasoption", &options); - unittest(np && !strcmp("testaliasoption", options), - "option alias path test failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); + KUNIT_EXPECT_STREQ_MSG(test, "testaliasoption", options, + "option alias path test failed\n"); of_node_put(np); np = of_find_node_opts_by_path("testcase-alias:test/alias/option", &options); - unittest(np && !strcmp("test/alias/option", options), - "option alias path test, subcase #1 failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); + KUNIT_EXPECT_STREQ_MSG(test, "test/alias/option", options, + "option alias path test, subcase #1 failed\n"); of_node_put(np); np = of_find_node_opts_by_path("testcase-alias:testaliasoption", NULL); - unittest(np, "NULL option alias path test failed\n"); + KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG(test, np, + "NULL option alias path test failed\n"); of_node_put(np); options = "testoption"; np = of_find_node_opts_by_path("testcase-alias", &options); - unittest(np && !options, "option clearing test failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); + KUNIT_EXPECT_EQ_MSG(test, options, NULL, + "option clearing test failed\n"); of_node_put(np); options = "testoption"; np = of_find_node_opts_by_path("/", &options); - unittest(np && !options, "option clearing root node test failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); + KUNIT_EXPECT_EQ_MSG(test, options, NULL, + "option clearing root node test failed\n"); of_node_put(np); } -static void __init of_unittest_dynamic(void) +static void of_unittest_dynamic(struct kunit *test) { struct device_node *np; struct property *prop; np = of_find_node_by_path("/testcase-data"); - if (!np) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); /* Array of 4 properties for the purpose of testing */ prop = kcalloc(4, sizeof(*prop), GFP_KERNEL); - if (!prop) { - unittest(0, "kzalloc() failed\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, prop); /* Add a new property - should pass*/ prop->name = "new-property"; prop->value = "new-property-data"; prop->length = strlen(prop->value) + 1; - unittest(of_add_property(np, prop) == 0, "Adding a new property failed\n"); + KUNIT_EXPECT_EQ_MSG(test, of_add_property(np, prop), 0, + "Adding a new property failed\n"); /* Try to add an existing property - should fail */ prop++; prop->name = "new-property"; prop->value = "new-property-data-should-fail"; prop->length = strlen(prop->value) + 1; - unittest(of_add_property(np, prop) != 0, - "Adding an existing property should have failed\n"); + KUNIT_EXPECT_NE_MSG(test, of_add_property(np, prop), 0, + "Adding an existing property should have failed\n"); /* Try to modify an existing property - should pass */ prop->value = "modify-property-data-should-pass"; prop->length = strlen(prop->value) + 1; - unittest(of_update_property(np, prop) == 0, - "Updating an existing property should have passed\n"); + KUNIT_EXPECT_EQ_MSG(test, of_update_property(np, prop), 0, + "Updating an existing property should have passed\n"); /* Try to modify non-existent property - should pass*/ prop++; prop->name = "modify-property"; prop->value = "modify-missing-property-data-should-pass"; prop->length = strlen(prop->value) + 1; - unittest(of_update_property(np, prop) == 0, - "Updating a missing property should have passed\n"); + KUNIT_EXPECT_EQ_MSG(test, of_update_property(np, prop), 0, + "Updating a missing property should have passed\n"); /* Remove property - should pass */ - unittest(of_remove_property(np, prop) == 0, - "Removing a property should have passed\n"); + KUNIT_EXPECT_EQ_MSG(test, of_remove_property(np, prop), 0, + "Removing a property should have passed\n"); /* Adding very large property - should pass */ prop++; prop->name = "large-property-PAGE_SIZEx8"; prop->length = PAGE_SIZE * 8; prop->value = kzalloc(prop->length, GFP_KERNEL); - unittest(prop->value != NULL, "Unable to allocate large buffer\n"); - if (prop->value) - unittest(of_add_property(np, prop) == 0, - "Adding a large property should have passed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, prop->value); + KUNIT_EXPECT_EQ_MSG(test, of_add_property(np, prop), 0, + "Adding a large property should have passed\n"); } -static int __init of_unittest_check_node_linkage(struct device_node *np) +static int of_unittest_check_node_linkage(struct device_node *np) { struct device_node *child; int count = 0, rc; @@ -230,27 +231,29 @@ static int __init of_unittest_check_node_linkage(struct device_node *np) return rc; } -static void __init of_unittest_check_tree_linkage(void) +static void of_unittest_check_tree_linkage(struct kunit *test) { struct device_node *np; int allnode_count = 0, child_count; - if (!of_root) - return; + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, of_root); for_each_of_allnodes(np) allnode_count++; child_count = of_unittest_check_node_linkage(of_root); - unittest(child_count > 0, "Device node data structure is corrupted\n"); - unittest(child_count == allnode_count, - "allnodes list size (%i) doesn't match sibling lists size (%i)\n", - allnode_count, child_count); + KUNIT_EXPECT_GT_MSG(test, child_count, 0, + "Device node data structure is corrupted\n"); + KUNIT_EXPECT_EQ_MSG(test, child_count, allnode_count, + "allnodes list size (%i) doesn't match sibling lists size (%i)\n", + allnode_count, child_count); pr_debug("allnodes list size (%i); sibling lists size (%i)\n", allnode_count, child_count); } -static void __init of_unittest_printf_one(struct device_node *np, const char *fmt, - const char *expected) +static void of_unittest_printf_one(struct kunit *test, + struct device_node *np, + const char *fmt, + const char *expected) { unsigned char *buf; int buf_size; @@ -266,9 +269,12 @@ static void __init of_unittest_printf_one(struct device_node *np, const char *fm size = snprintf(buf, buf_size - 2, fmt, np); /* use strcmp() instead of strncmp() here to be absolutely sure strings match */ - unittest((strcmp(buf, expected) == 0) && (buf[size+1] == 0xff), - "sprintf failed; fmt='%s' expected='%s' rslt='%s'\n", - fmt, expected, buf); + KUNIT_EXPECT_STREQ_MSG(test, buf, expected, + "sprintf failed; fmt='%s' expected='%s' rslt='%s'\n", + fmt, expected, buf); + KUNIT_EXPECT_EQ_MSG(test, buf[size+1], 0xff, + "sprintf failed; fmt='%s' expected='%s' rslt='%s'\n", + fmt, expected, buf); /* Make sure length limits work */ size++; @@ -276,40 +282,43 @@ static void __init of_unittest_printf_one(struct device_node *np, const char *fm /* Clear the buffer, and make sure it works correctly still */ memset(buf, 0xff, buf_size); snprintf(buf, size+1, fmt, np); - unittest(strncmp(buf, expected, size) == 0 && (buf[size+1] == 0xff), - "snprintf failed; size=%i fmt='%s' expected='%s' rslt='%s'\n", - size, fmt, expected, buf); + KUNIT_EXPECT_STREQ_MSG(test, buf, expected, + "snprintf failed; size=%i fmt='%s' expected='%s' rslt='%s'\n", + size, fmt, expected, buf); + KUNIT_EXPECT_EQ_MSG(test, buf[size+1], 0xff, + "snprintf failed; size=%i fmt='%s' expected='%s' rslt='%s'\n", + size, fmt, expected, buf); } kfree(buf); } -static void __init of_unittest_printf(void) +static void of_unittest_printf(struct kunit *test) { struct device_node *np; const char *full_name = "/testcase-data/platform-tests/test-device@1/dev@100"; char phandle_str[16] = ""; np = of_find_node_by_path(full_name); - if (!np) { - unittest(np, "testcase data missing\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); num_to_str(phandle_str, sizeof(phandle_str), np->phandle, 0); - of_unittest_printf_one(np, "%pOF", full_name); - of_unittest_printf_one(np, "%pOFf", full_name); - of_unittest_printf_one(np, "%pOFp", phandle_str); - of_unittest_printf_one(np, "%pOFP", "dev@100"); - of_unittest_printf_one(np, "ABC %pOFP ABC", "ABC dev@100 ABC"); - of_unittest_printf_one(np, "%10pOFP", " dev@100"); - of_unittest_printf_one(np, "%-10pOFP", "dev@100 "); - of_unittest_printf_one(of_root, "%pOFP", "/"); - of_unittest_printf_one(np, "%pOFF", "----"); - of_unittest_printf_one(np, "%pOFPF", "dev@100:----"); - of_unittest_printf_one(np, "%pOFPFPc", "dev@100:----:dev@100:test-sub-device"); - of_unittest_printf_one(np, "%pOFc", "test-sub-device"); - of_unittest_printf_one(np, "%pOFC", + of_unittest_printf_one(test, np, "%pOF", full_name); + of_unittest_printf_one(test, np, "%pOFf", full_name); + of_unittest_printf_one(test, np, "%pOFp", phandle_str); + of_unittest_printf_one(test, np, "%pOFP", "dev@100"); + of_unittest_printf_one(test, np, "ABC %pOFP ABC", "ABC dev@100 ABC"); + of_unittest_printf_one(test, np, "%10pOFP", " dev@100"); + of_unittest_printf_one(test, np, "%-10pOFP", "dev@100 "); + of_unittest_printf_one(test, of_root, "%pOFP", "/"); + of_unittest_printf_one(test, np, "%pOFF", "----"); + of_unittest_printf_one(test, np, "%pOFPF", "dev@100:----"); + of_unittest_printf_one(test, + np, + "%pOFPFPc", + "dev@100:----:dev@100:test-sub-device"); + of_unittest_printf_one(test, np, "%pOFc", "test-sub-device"); + of_unittest_printf_one(test, np, "%pOFC", "\"test-sub-device\",\"test-compat2\",\"test-compat3\""); } @@ -319,7 +328,7 @@ struct node_hash { }; static DEFINE_HASHTABLE(phandle_ht, 8); -static void __init of_unittest_check_phandles(void) +static void of_unittest_check_phandles(struct kunit *test) { struct device_node *np; struct node_hash *nh; @@ -331,24 +340,25 @@ static void __init of_unittest_check_phandles(void) continue; hash_for_each_possible(phandle_ht, nh, node, np->phandle) { + KUNIT_EXPECT_NE_MSG(test, nh->np->phandle, np->phandle, + "Duplicate phandle! %i used by %pOF and %pOF\n", + np->phandle, nh->np, np); if (nh->np->phandle == np->phandle) { - pr_info("Duplicate phandle! %i used by %pOF and %pOF\n", - np->phandle, nh->np, np); dup_count++; break; } } nh = kzalloc(sizeof(*nh), GFP_KERNEL); - if (WARN_ON(!nh)) - return; + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, nh); nh->np = np; hash_add(phandle_ht, &nh->node, np->phandle); phandle_count++; } - unittest(dup_count == 0, "Found %i duplicates in %i phandles\n", - dup_count, phandle_count); + KUNIT_EXPECT_EQ_MSG(test, dup_count, 0, + "Found %i duplicates in %i phandles\n", + dup_count, phandle_count); /* Clean up */ hash_for_each_safe(phandle_ht, i, tmp, nh, node) { @@ -357,20 +367,22 @@ static void __init of_unittest_check_phandles(void) } } -static void __init of_unittest_parse_phandle_with_args(void) +static void of_unittest_parse_phandle_with_args(struct kunit *test) { struct device_node *np; struct of_phandle_args args; - int i, rc; + int i, rc = 0; np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); - if (!np) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); - rc = of_count_phandle_with_args(np, "phandle-list", "#phandle-cells"); - unittest(rc == 7, "of_count_phandle_with_args() returned %i, expected 7\n", rc); + KUNIT_EXPECT_EQ_MSG(test, + of_count_phandle_with_args(np, + "phandle-list", + "#phandle-cells"), + 7, + "of_count_phandle_with_args() returned %i, expected 7\n", + rc); for (i = 0; i < 8; i++) { bool passed = true; @@ -423,81 +435,90 @@ static void __init of_unittest_parse_phandle_with_args(void) passed = false; } - unittest(passed, "index %i - data error on node %pOF rc=%i\n", - i, args.np, rc); + KUNIT_EXPECT_TRUE_MSG(test, passed, + "index %i - data error on node %pOF rc=%i\n", + i, args.np, rc); } /* Check for missing list property */ - rc = of_parse_phandle_with_args(np, "phandle-list-missing", - "#phandle-cells", 0, &args); - unittest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc); - rc = of_count_phandle_with_args(np, "phandle-list-missing", - "#phandle-cells"); - unittest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc); + KUNIT_EXPECT_EQ(test, + of_parse_phandle_with_args(np, + "phandle-list-missing", + "#phandle-cells", + 0, &args), + -ENOENT); + KUNIT_EXPECT_EQ(test, + of_count_phandle_with_args(np, + "phandle-list-missing", + "#phandle-cells"), + -ENOENT); /* Check for missing cells property */ - rc = of_parse_phandle_with_args(np, "phandle-list", - "#phandle-cells-missing", 0, &args); - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); - rc = of_count_phandle_with_args(np, "phandle-list", - "#phandle-cells-missing"); - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); + KUNIT_EXPECT_EQ(test, + of_parse_phandle_with_args(np, + "phandle-list", + "#phandle-cells-missing", + 0, &args), + -EINVAL); + KUNIT_EXPECT_EQ(test, + of_count_phandle_with_args(np, + "phandle-list", + "#phandle-cells-missing"), + -EINVAL); /* Check for bad phandle in list */ - rc = of_parse_phandle_with_args(np, "phandle-list-bad-phandle", - "#phandle-cells", 0, &args); - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); - rc = of_count_phandle_with_args(np, "phandle-list-bad-phandle", - "#phandle-cells"); - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); + KUNIT_EXPECT_EQ(test, + of_parse_phandle_with_args(np, + "phandle-list-bad-phandle", + "#phandle-cells", + 0, &args), + -EINVAL); + KUNIT_EXPECT_EQ(test, + of_count_phandle_with_args(np, + "phandle-list-bad-phandle", + "#phandle-cells"), + -EINVAL); /* Check for incorrectly formed argument list */ - rc = of_parse_phandle_with_args(np, "phandle-list-bad-args", - "#phandle-cells", 1, &args); - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); - rc = of_count_phandle_with_args(np, "phandle-list-bad-args", - "#phandle-cells"); - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); + KUNIT_EXPECT_EQ(test, + of_parse_phandle_with_args(np, + "phandle-list-bad-args", + "#phandle-cells", + 1, &args), + -EINVAL); + KUNIT_EXPECT_EQ(test, + of_count_phandle_with_args(np, + "phandle-list-bad-args", + "#phandle-cells"), + -EINVAL); } -static void __init of_unittest_parse_phandle_with_args_map(void) +static void of_unittest_parse_phandle_with_args_map(struct kunit *test) { struct device_node *np, *p0, *p1, *p2, *p3; struct of_phandle_args args; int i, rc; np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-b"); - if (!np) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); p0 = of_find_node_by_path("/testcase-data/phandle-tests/provider0"); - if (!p0) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p0); p1 = of_find_node_by_path("/testcase-data/phandle-tests/provider1"); - if (!p1) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p1); p2 = of_find_node_by_path("/testcase-data/phandle-tests/provider2"); - if (!p2) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p2); p3 = of_find_node_by_path("/testcase-data/phandle-tests/provider3"); - if (!p3) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p3); - rc = of_count_phandle_with_args(np, "phandle-list", "#phandle-cells"); - unittest(rc == 7, "of_count_phandle_with_args() returned %i, expected 7\n", rc); + KUNIT_EXPECT_EQ(test, + of_count_phandle_with_args(np, + "phandle-list", + "#phandle-cells"), + 7); for (i = 0; i < 8; i++) { bool passed = true; @@ -554,117 +575,214 @@ static void __init of_unittest_parse_phandle_with_args_map(void) passed = false; } - unittest(passed, "index %i - data error on node %s rc=%i\n", - i, args.np->full_name, rc); + KUNIT_EXPECT_TRUE_MSG(test, passed, + "index %i - data error on node %s rc=%i\n", + i, args.np->full_name, rc); } /* Check for missing list property */ - rc = of_parse_phandle_with_args_map(np, "phandle-list-missing", - "phandle", 0, &args); - unittest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc); + KUNIT_EXPECT_EQ(test, + of_parse_phandle_with_args_map(np, + "phandle-list-missing", + "phandle", + 0, &args), + -ENOENT); /* Check for missing cells,map,mask property */ - rc = of_parse_phandle_with_args_map(np, "phandle-list", - "phandle-missing", 0, &args); - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); + KUNIT_EXPECT_EQ(test, + of_parse_phandle_with_args_map(np, + "phandle-list", + "phandle-missing", + 0, &args), + -EINVAL); /* Check for bad phandle in list */ - rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-phandle", - "phandle", 0, &args); - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); + KUNIT_EXPECT_EQ(test, + of_parse_phandle_with_args_map(np, + "phandle-list-bad-phandle", + "phandle", + 0, &args), + -EINVAL); /* Check for incorrectly formed argument list */ - rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-args", - "phandle", 1, &args); - unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc); + KUNIT_EXPECT_EQ(test, + of_parse_phandle_with_args_map(np, + "phandle-list-bad-args", + "phandle", + 1, &args), + -EINVAL); } -static void __init of_unittest_property_string(void) +static void of_unittest_property_string(struct kunit *test) { const char *strings[4]; struct device_node *np; int rc; np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); - if (!np) { - pr_err("No testcase data in device tree\n"); - return; - } - - rc = of_property_match_string(np, "phandle-list-names", "first"); - unittest(rc == 0, "first expected:0 got:%i\n", rc); - rc = of_property_match_string(np, "phandle-list-names", "second"); - unittest(rc == 1, "second expected:1 got:%i\n", rc); - rc = of_property_match_string(np, "phandle-list-names", "third"); - unittest(rc == 2, "third expected:2 got:%i\n", rc); - rc = of_property_match_string(np, "phandle-list-names", "fourth"); - unittest(rc == -ENODATA, "unmatched string; rc=%i\n", rc); - rc = of_property_match_string(np, "missing-property", "blah"); - unittest(rc == -EINVAL, "missing property; rc=%i\n", rc); - rc = of_property_match_string(np, "empty-property", "blah"); - unittest(rc == -ENODATA, "empty property; rc=%i\n", rc); - rc = of_property_match_string(np, "unterminated-string", "blah"); - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); + + KUNIT_EXPECT_EQ(test, + of_property_match_string(np, + "phandle-list-names", + "first"), + 0); + KUNIT_EXPECT_EQ(test, + of_property_match_string(np, + "phandle-list-names", + "second"), + 1); + KUNIT_EXPECT_EQ(test, + of_property_match_string(np, + "phandle-list-names", + "third"), + 2); + KUNIT_EXPECT_EQ_MSG(test, + of_property_match_string(np, + "phandle-list-names", + "fourth"), + -ENODATA, + "unmatched string"); + KUNIT_EXPECT_EQ_MSG(test, + of_property_match_string(np, + "missing-property", + "blah"), + -EINVAL, + "missing property"); + KUNIT_EXPECT_EQ_MSG(test, + of_property_match_string(np, + "empty-property", + "blah"), + -ENODATA, + "empty property"); + KUNIT_EXPECT_EQ_MSG(test, + of_property_match_string(np, + "unterminated-string", + "blah"), + -EILSEQ, + "unterminated string"); /* of_property_count_strings() tests */ - rc = of_property_count_strings(np, "string-property"); - unittest(rc == 1, "Incorrect string count; rc=%i\n", rc); - rc = of_property_count_strings(np, "phandle-list-names"); - unittest(rc == 3, "Incorrect string count; rc=%i\n", rc); - rc = of_property_count_strings(np, "unterminated-string"); - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); - rc = of_property_count_strings(np, "unterminated-string-list"); - unittest(rc == -EILSEQ, "unterminated string array; rc=%i\n", rc); + KUNIT_EXPECT_EQ(test, + of_property_count_strings(np, "string-property"), + 1); + KUNIT_EXPECT_EQ(test, + of_property_count_strings(np, "phandle-list-names"), + 3); + KUNIT_EXPECT_EQ_MSG(test, + of_property_count_strings(np, + "unterminated-string"), + -EILSEQ, + "unterminated string"); + KUNIT_EXPECT_EQ_MSG(test, + of_property_count_strings( + np, + "unterminated-string-list"), + -EILSEQ, + "unterminated string array"); /* of_property_read_string_index() tests */ rc = of_property_read_string_index(np, "string-property", 0, strings); - unittest(rc == 0 && !strcmp(strings[0], "foobar"), "of_property_read_string_index() failure; rc=%i\n", rc); + KUNIT_ASSERT_EQ(test, rc, 0); + KUNIT_EXPECT_STREQ(test, strings[0], "foobar"); strings[0] = NULL; rc = of_property_read_string_index(np, "string-property", 1, strings); - unittest(rc == -ENODATA && strings[0] == NULL, "of_property_read_string_index() failure; rc=%i\n", rc); - rc = of_property_read_string_index(np, "phandle-list-names", 0, strings); - unittest(rc == 0 && !strcmp(strings[0], "first"), "of_property_read_string_index() failure; rc=%i\n", rc); - rc = of_property_read_string_index(np, "phandle-list-names", 1, strings); - unittest(rc == 0 && !strcmp(strings[0], "second"), "of_property_read_string_index() failure; rc=%i\n", rc); - rc = of_property_read_string_index(np, "phandle-list-names", 2, strings); - unittest(rc == 0 && !strcmp(strings[0], "third"), "of_property_read_string_index() failure; rc=%i\n", rc); + KUNIT_EXPECT_EQ(test, rc, -ENODATA); + KUNIT_EXPECT_EQ(test, strings[0], NULL); + rc = of_property_read_string_index(np, + "phandle-list-names", + 0, + strings); + KUNIT_ASSERT_EQ(test, rc, 0); + KUNIT_EXPECT_STREQ(test, strings[0], "first"); + rc = of_property_read_string_index(np, + "phandle-list-names", + 1, + strings); + KUNIT_ASSERT_EQ(test, rc, 0); + KUNIT_EXPECT_STREQ(test, strings[0], "second"); + rc = of_property_read_string_index(np, + "phandle-list-names", + 2, + strings); + KUNIT_ASSERT_EQ(test, rc, 0); + KUNIT_EXPECT_STREQ(test, strings[0], "third"); strings[0] = NULL; - rc = of_property_read_string_index(np, "phandle-list-names", 3, strings); - unittest(rc == -ENODATA && strings[0] == NULL, "of_property_read_string_index() failure; rc=%i\n", rc); + rc = of_property_read_string_index(np, + "phandle-list-names", 3, strings); + KUNIT_EXPECT_EQ(test, rc, -ENODATA); + KUNIT_EXPECT_EQ(test, strings[0], NULL); strings[0] = NULL; - rc = of_property_read_string_index(np, "unterminated-string", 0, strings); - unittest(rc == -EILSEQ && strings[0] == NULL, "of_property_read_string_index() failure; rc=%i\n", rc); - rc = of_property_read_string_index(np, "unterminated-string-list", 0, strings); - unittest(rc == 0 && !strcmp(strings[0], "first"), "of_property_read_string_index() failure; rc=%i\n", rc); + rc = of_property_read_string_index(np, + "unterminated-string", + 0, + strings); + KUNIT_EXPECT_EQ(test, rc, -EILSEQ); + KUNIT_EXPECT_EQ(test, strings[0], NULL); + rc = of_property_read_string_index(np, + "unterminated-string-list", + 0, + strings); + KUNIT_ASSERT_EQ(test, rc, 0); + KUNIT_EXPECT_STREQ(test, strings[0], "first"); strings[0] = NULL; - rc = of_property_read_string_index(np, "unterminated-string-list", 2, strings); /* should fail */ - unittest(rc == -EILSEQ && strings[0] == NULL, "of_property_read_string_index() failure; rc=%i\n", rc); - strings[1] = NULL; + rc = of_property_read_string_index(np, + "unterminated-string-list", + 2, + strings); /* should fail */ + KUNIT_EXPECT_EQ(test, rc, -EILSEQ); + KUNIT_EXPECT_EQ(test, strings[0], NULL); /* of_property_read_string_array() tests */ - rc = of_property_read_string_array(np, "string-property", strings, 4); - unittest(rc == 1, "Incorrect string count; rc=%i\n", rc); - rc = of_property_read_string_array(np, "phandle-list-names", strings, 4); - unittest(rc == 3, "Incorrect string count; rc=%i\n", rc); - rc = of_property_read_string_array(np, "unterminated-string", strings, 4); - unittest(rc == -EILSEQ, "unterminated string; rc=%i\n", rc); + strings[1] = NULL; + KUNIT_EXPECT_EQ(test, + of_property_read_string_array(np, + "string-property", + strings, 4), + 1); + KUNIT_EXPECT_EQ(test, + of_property_read_string_array(np, + "phandle-list-names", + strings, 4), + 3); + KUNIT_EXPECT_EQ_MSG(test, + of_property_read_string_array(np, + "unterminated-string", + strings, 4), + -EILSEQ, + "unterminated string"); /* -- An incorrectly formed string should cause a failure */ - rc = of_property_read_string_array(np, "unterminated-string-list", strings, 4); - unittest(rc == -EILSEQ, "unterminated string array; rc=%i\n", rc); + KUNIT_EXPECT_EQ_MSG(test, + of_property_read_string_array( + np, + "unterminated-string-list", + strings, + 4), + -EILSEQ, + "unterminated string array"); /* -- parsing the correctly formed strings should still work: */ strings[2] = NULL; - rc = of_property_read_string_array(np, "unterminated-string-list", strings, 2); - unittest(rc == 2 && strings[2] == NULL, "of_property_read_string_array() failure; rc=%i\n", rc); + rc = of_property_read_string_array(np, + "unterminated-string-list", + strings, + 2); + KUNIT_EXPECT_EQ(test, rc, 2); + KUNIT_EXPECT_EQ(test, strings[2], NULL); strings[1] = NULL; - rc = of_property_read_string_array(np, "phandle-list-names", strings, 1); - unittest(rc == 1 && strings[1] == NULL, "Overwrote end of string array; rc=%i, str='%s'\n", rc, strings[1]); + rc = of_property_read_string_array(np, + "phandle-list-names", + strings, + 1); + KUNIT_ASSERT_EQ(test, rc, 1); + KUNIT_EXPECT_EQ_MSG(test, strings[1], NULL, + "Overwrote end of string array"); } #define propcmp(p1, p2) (((p1)->length == (p2)->length) && \ (p1)->value && (p2)->value && \ !memcmp((p1)->value, (p2)->value, (p1)->length) && \ !strcmp((p1)->name, (p2)->name)) -static void __init of_unittest_property_copy(void) +static void of_unittest_property_copy(struct kunit *test) { #ifdef CONFIG_OF_DYNAMIC struct property p1 = { .name = "p1", .length = 0, .value = "" }; @@ -672,20 +790,24 @@ static void __init of_unittest_property_copy(void) struct property *new; new = __of_prop_dup(&p1, GFP_KERNEL); - unittest(new && propcmp(&p1, new), "empty property didn't copy correctly\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new); + KUNIT_EXPECT_TRUE_MSG(test, propcmp(&p1, new), + "empty property didn't copy correctly"); kfree(new->value); kfree(new->name); kfree(new); new = __of_prop_dup(&p2, GFP_KERNEL); - unittest(new && propcmp(&p2, new), "non-empty property didn't copy correctly\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, new); + KUNIT_EXPECT_TRUE_MSG(test, propcmp(&p2, new), + "non-empty property didn't copy correctly"); kfree(new->value); kfree(new->name); kfree(new); #endif } -static void __init of_unittest_changeset(void) +static void of_unittest_changeset(struct kunit *test) { #ifdef CONFIG_OF_DYNAMIC struct property *ppadd, padd = { .name = "prop-add", .length = 1, .value = "" }; @@ -698,32 +820,32 @@ static void __init of_unittest_changeset(void) struct of_changeset chgset; n1 = __of_node_dup(NULL, "n1"); - unittest(n1, "testcase setup failure\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, n1); n2 = __of_node_dup(NULL, "n2"); - unittest(n2, "testcase setup failure\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, n2); n21 = __of_node_dup(NULL, "n21"); - unittest(n21, "testcase setup failure %p\n", n21); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, n21); nchangeset = of_find_node_by_path("/testcase-data/changeset"); nremove = of_get_child_by_name(nchangeset, "node-remove"); - unittest(nremove, "testcase setup failure\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, nremove); ppadd = __of_prop_dup(&padd, GFP_KERNEL); - unittest(ppadd, "testcase setup failure\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppadd); ppname_n1 = __of_prop_dup(&pname_n1, GFP_KERNEL); - unittest(ppname_n1, "testcase setup failure\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppname_n1); ppname_n2 = __of_prop_dup(&pname_n2, GFP_KERNEL); - unittest(ppname_n2, "testcase setup failure\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppname_n2); ppname_n21 = __of_prop_dup(&pname_n21, GFP_KERNEL); - unittest(ppname_n21, "testcase setup failure\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppname_n21); ppupdate = __of_prop_dup(&pupdate, GFP_KERNEL); - unittest(ppupdate, "testcase setup failure\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppupdate); parent = nchangeset; n1->parent = parent; @@ -731,54 +853,82 @@ static void __init of_unittest_changeset(void) n21->parent = n2; ppremove = of_find_property(parent, "prop-remove", NULL); - unittest(ppremove, "failed to find removal prop"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ppremove); of_changeset_init(&chgset); - unittest(!of_changeset_attach_node(&chgset, n1), "fail attach n1\n"); - unittest(!of_changeset_add_property(&chgset, n1, ppname_n1), "fail add prop name\n"); - - unittest(!of_changeset_attach_node(&chgset, n2), "fail attach n2\n"); - unittest(!of_changeset_add_property(&chgset, n2, ppname_n2), "fail add prop name\n"); - - unittest(!of_changeset_detach_node(&chgset, nremove), "fail remove node\n"); - unittest(!of_changeset_add_property(&chgset, n21, ppname_n21), "fail add prop name\n"); - - unittest(!of_changeset_attach_node(&chgset, n21), "fail attach n21\n"); - - unittest(!of_changeset_add_property(&chgset, parent, ppadd), "fail add prop prop-add\n"); - unittest(!of_changeset_update_property(&chgset, parent, ppupdate), "fail update prop\n"); - unittest(!of_changeset_remove_property(&chgset, parent, ppremove), "fail remove prop\n"); - - unittest(!of_changeset_apply(&chgset), "apply failed\n"); + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_attach_node(&chgset, n1), + "fail attach n1\n"); + KUNIT_EXPECT_FALSE_MSG(test, + of_changeset_add_property(&chgset, + n1, + ppname_n1), + "fail add prop name\n"); + + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_attach_node(&chgset, n2), + "fail attach n2\n"); + KUNIT_EXPECT_FALSE_MSG(test, + of_changeset_add_property(&chgset, + n2, + ppname_n2), + "fail add prop name\n"); + + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_detach_node(&chgset, nremove), + "fail remove node\n"); + KUNIT_EXPECT_FALSE_MSG(test, + of_changeset_add_property(&chgset, + n21, + ppname_n21), + "fail add prop name\n"); + + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_attach_node(&chgset, n21), + "fail attach n21\n"); + + KUNIT_EXPECT_FALSE_MSG(test, + of_changeset_add_property(&chgset, + parent, + ppadd), + "fail add prop prop-add\n"); + KUNIT_EXPECT_FALSE_MSG(test, + of_changeset_update_property(&chgset, + parent, + ppupdate), + "fail update prop\n"); + KUNIT_EXPECT_FALSE_MSG(test, + of_changeset_remove_property(&chgset, + parent, + ppremove), + "fail remove prop\n"); + + KUNIT_EXPECT_FALSE_MSG(test, of_changeset_apply(&chgset), + "apply failed\n"); of_node_put(nchangeset); /* Make sure node names are constructed correctly */ - unittest((np = of_find_node_by_path("/testcase-data/changeset/n2/n21")), - "'%pOF' not added\n", n21); + KUNIT_EXPECT_NOT_ERR_OR_NULL_MSG( + test, + np = of_find_node_by_path( + "/testcase-data/changeset/n2/n21"), + "'%pOF' not added\n", n21); of_node_put(np); - unittest(!of_changeset_revert(&chgset), "revert failed\n"); + KUNIT_EXPECT_FALSE(test, of_changeset_revert(&chgset)); of_changeset_destroy(&chgset); #endif } -static void __init of_unittest_parse_interrupts(void) +static void of_unittest_parse_interrupts(struct kunit *test) { struct device_node *np; struct of_phandle_args args; int i, rc; - if (of_irq_workarounds & OF_IMAP_OLDWORLD_MAC) - return; + KUNIT_ASSERT_FALSE(test, of_irq_workarounds & OF_IMAP_OLDWORLD_MAC); np = of_find_node_by_path("/testcase-data/interrupts/interrupts0"); - if (!np) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); for (i = 0; i < 4; i++) { bool passed = true; @@ -790,16 +940,14 @@ static void __init of_unittest_parse_interrupts(void) passed &= (args.args_count == 1); passed &= (args.args[0] == (i + 1)); - unittest(passed, "index %i - data error on node %pOF rc=%i\n", - i, args.np, rc); + KUNIT_EXPECT_TRUE_MSG(test, passed, + "index %i - data error on node %pOF rc=%i\n", + i, args.np, rc); } of_node_put(np); np = of_find_node_by_path("/testcase-data/interrupts/interrupts1"); - if (!np) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); for (i = 0; i < 4; i++) { bool passed = true; @@ -836,26 +984,23 @@ static void __init of_unittest_parse_interrupts(void) default: passed = false; } - unittest(passed, "index %i - data error on node %pOF rc=%i\n", - i, args.np, rc); + KUNIT_EXPECT_TRUE_MSG(test, passed, + "index %i - data error on node %pOF rc=%i\n", + i, args.np, rc); } of_node_put(np); } -static void __init of_unittest_parse_interrupts_extended(void) +static void of_unittest_parse_interrupts_extended(struct kunit *test) { struct device_node *np; struct of_phandle_args args; int i, rc; - if (of_irq_workarounds & OF_IMAP_OLDWORLD_MAC) - return; + KUNIT_ASSERT_FALSE(test, of_irq_workarounds & OF_IMAP_OLDWORLD_MAC); np = of_find_node_by_path("/testcase-data/interrupts/interrupts-extended0"); - if (!np) { - pr_err("missing testcase data\n"); - return; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); for (i = 0; i < 7; i++) { bool passed = true; @@ -909,8 +1054,9 @@ static void __init of_unittest_parse_interrupts_extended(void) passed = false; } - unittest(passed, "index %i - data error on node %pOF rc=%i\n", - i, args.np, rc); + KUNIT_EXPECT_TRUE_MSG(test, passed, + "index %i - data error on node %pOF rc=%i\n", + i, args.np, rc); } of_node_put(np); } @@ -950,7 +1096,7 @@ static struct { { .path = "/testcase-data/match-node/name9", .data = "K", }, }; -static void __init of_unittest_match_node(void) +static void of_unittest_match_node(struct kunit *test) { struct device_node *np; const struct of_device_id *match; @@ -958,26 +1104,19 @@ static void __init of_unittest_match_node(void) for (i = 0; i < ARRAY_SIZE(match_node_tests); i++) { np = of_find_node_by_path(match_node_tests[i].path); - if (!np) { - unittest(0, "missing testcase node %s\n", - match_node_tests[i].path); - continue; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); match = of_match_node(match_node_table, np); - if (!match) { - unittest(0, "%s didn't match anything\n", - match_node_tests[i].path); - continue; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, np, + "%s didn't match anything", + match_node_tests[i].path); - if (strcmp(match->data, match_node_tests[i].data) != 0) { - unittest(0, "%s got wrong match. expected %s, got %s\n", - match_node_tests[i].path, match_node_tests[i].data, - (const char *)match->data); - continue; - } - unittest(1, "passed"); + KUNIT_EXPECT_STREQ_MSG(test, + match->data, match_node_tests[i].data, + "%s got wrong match. expected %s, got %s\n", + match_node_tests[i].path, + match_node_tests[i].data, + (const char *)match->data); } } @@ -989,9 +1128,9 @@ static struct resource test_bus_res = { static const struct platform_device_info test_bus_info = { .name = "unittest-bus", }; -static void __init of_unittest_platform_populate(void) +static void of_unittest_platform_populate(struct kunit *test) { - int irq, rc; + int irq; struct device_node *np, *child, *grandchild; struct platform_device *pdev, *test_bus; const struct of_device_id match[] = { @@ -1005,32 +1144,27 @@ static void __init of_unittest_platform_populate(void) /* Test that a missing irq domain returns -EPROBE_DEFER */ np = of_find_node_by_path("/testcase-data/testcase-device1"); pdev = of_find_device_by_node(np); - unittest(pdev, "device 1 creation failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev); if (!(of_irq_workarounds & OF_IMAP_OLDWORLD_MAC)) { irq = platform_get_irq(pdev, 0); - unittest(irq == -EPROBE_DEFER, - "device deferred probe failed - %d\n", irq); + KUNIT_ASSERT_EQ(test, irq, -EPROBE_DEFER); /* Test that a parsing failure does not return -EPROBE_DEFER */ np = of_find_node_by_path("/testcase-data/testcase-device2"); pdev = of_find_device_by_node(np); - unittest(pdev, "device 2 creation failed\n"); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev); irq = platform_get_irq(pdev, 0); - unittest(irq < 0 && irq != -EPROBE_DEFER, - "device parsing error failed - %d\n", irq); + KUNIT_ASSERT_TRUE_MSG(test, irq < 0 && irq != -EPROBE_DEFER, + "device parsing error failed - %d\n", + irq); } np = of_find_node_by_path("/testcase-data/platform-tests"); - unittest(np, "No testcase data in device tree\n"); - if (!np) - return; + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, np); test_bus = platform_device_register_full(&test_bus_info); - rc = PTR_ERR_OR_ZERO(test_bus); - unittest(!rc, "testbus registration failed; rc=%i\n", rc); - if (rc) - return; + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_bus); test_bus->dev.of_node = np; /* @@ -1045,17 +1179,21 @@ static void __init of_unittest_platform_populate(void) of_platform_populate(np, match, NULL, &test_bus->dev); for_each_child_of_node(np, child) { for_each_child_of_node(child, grandchild) - unittest(of_find_device_by_node(grandchild), - "Could not create device for node '%s'\n", - grandchild->name); + KUNIT_EXPECT_TRUE_MSG(test, + of_find_device_by_node( + grandchild), + "Could not create device for node '%s'\n", + grandchild->name); } of_platform_depopulate(&test_bus->dev); for_each_child_of_node(np, child) { for_each_child_of_node(child, grandchild) - unittest(!of_find_device_by_node(grandchild), - "device didn't get destroyed '%s'\n", - grandchild->name); + KUNIT_EXPECT_FALSE_MSG(test, + of_find_device_by_node( + grandchild), + "device didn't get destroyed '%s'\n", + grandchild->name); } platform_device_unregister(test_bus); @@ -1129,7 +1267,7 @@ static int attach_node_and_children(struct device_node *np) * unittest_data_add - Reads, copies data from * linked tree and attaches it to the live tree */ -static int __init unittest_data_add(void) +static int unittest_data_add(void) { void *unittest_data; struct device_node *unittest_data_node, *np; @@ -1200,7 +1338,7 @@ static int __init unittest_data_add(void) } #ifdef CONFIG_OF_OVERLAY -static int __init overlay_data_apply(const char *overlay_name, int *overlay_id); +static int overlay_data_apply(const char *overlay_name, int *overlay_id); static int unittest_probe(struct platform_device *pdev) { @@ -1429,173 +1567,157 @@ static void of_unittest_destroy_tracked_overlays(void) } while (defers > 0); } -static int __init of_unittest_apply_overlay(int overlay_nr, int unittest_nr, - int *overlay_id) +static int of_unittest_apply_overlay(struct kunit *test, + int overlay_nr, + int unittest_nr, + int *overlay_id) { const char *overlay_name; overlay_name = overlay_name_from_nr(overlay_nr); - if (!overlay_data_apply(overlay_name, overlay_id)) { - unittest(0, "could not apply overlay \"%s\"\n", - overlay_name); - return -EFAULT; - } + KUNIT_ASSERT_TRUE_MSG(test, + overlay_data_apply(overlay_name, overlay_id), + "could not apply overlay \"%s\"\n", + overlay_name); of_unittest_track_overlay(*overlay_id); return 0; } /* apply an overlay while checking before and after states */ -static int __init of_unittest_apply_overlay_check(int overlay_nr, +static int of_unittest_apply_overlay_check(struct kunit *test, int overlay_nr, int unittest_nr, int before, int after, enum overlay_type ovtype) { - int ret, ovcs_id; + int ovcs_id; /* unittest device must not be in before state */ - if (of_unittest_device_exists(unittest_nr, ovtype) != before) { - unittest(0, "%s with device @\"%s\" %s\n", - overlay_name_from_nr(overlay_nr), - unittest_path(unittest_nr, ovtype), - !before ? "enabled" : "disabled"); - return -EINVAL; - } + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_device_exists(unittest_nr, ovtype), + before, + "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), + unittest_path(unittest_nr, ovtype), + !before ? "enabled" : "disabled"); ovcs_id = 0; - ret = of_unittest_apply_overlay(overlay_nr, unittest_nr, &ovcs_id); - if (ret != 0) { - /* of_unittest_apply_overlay already called unittest() */ - return ret; - } + KUNIT_EXPECT_EQ(test, + of_unittest_apply_overlay(test, + overlay_nr, + unittest_nr, + &ovcs_id), + 0); /* unittest device must be to set to after state */ - if (of_unittest_device_exists(unittest_nr, ovtype) != after) { - unittest(0, "%s failed to create @\"%s\" %s\n", - overlay_name_from_nr(overlay_nr), - unittest_path(unittest_nr, ovtype), - !after ? "enabled" : "disabled"); - return -EINVAL; - } + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_device_exists(unittest_nr, ovtype), + after, + "%s failed to create @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), + unittest_path(unittest_nr, ovtype), + !after ? "enabled" : "disabled"); return 0; } /* apply an overlay and then revert it while checking before, after states */ -static int __init of_unittest_apply_revert_overlay_check(int overlay_nr, +static int of_unittest_apply_revert_overlay_check( + struct kunit *test, int overlay_nr, int unittest_nr, int before, int after, enum overlay_type ovtype) { - int ret, ovcs_id; + int ovcs_id; /* unittest device must be in before state */ - if (of_unittest_device_exists(unittest_nr, ovtype) != before) { - unittest(0, "%s with device @\"%s\" %s\n", - overlay_name_from_nr(overlay_nr), - unittest_path(unittest_nr, ovtype), - !before ? "enabled" : "disabled"); - return -EINVAL; - } + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_device_exists(unittest_nr, ovtype), + before, + "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), + unittest_path(unittest_nr, ovtype), + !before ? "enabled" : "disabled"); /* apply the overlay */ ovcs_id = 0; - ret = of_unittest_apply_overlay(overlay_nr, unittest_nr, &ovcs_id); - if (ret != 0) { - /* of_unittest_apply_overlay already called unittest() */ - return ret; - } + KUNIT_ASSERT_EQ(test, + of_unittest_apply_overlay(test, + overlay_nr, + unittest_nr, + &ovcs_id), + 0); /* unittest device must be in after state */ - if (of_unittest_device_exists(unittest_nr, ovtype) != after) { - unittest(0, "%s failed to create @\"%s\" %s\n", - overlay_name_from_nr(overlay_nr), - unittest_path(unittest_nr, ovtype), - !after ? "enabled" : "disabled"); - return -EINVAL; - } - - ret = of_overlay_remove(&ovcs_id); - if (ret != 0) { - unittest(0, "%s failed to be destroyed @\"%s\"\n", - overlay_name_from_nr(overlay_nr), - unittest_path(unittest_nr, ovtype)); - return ret; - } + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_device_exists(unittest_nr, ovtype), + after, + "%s failed to create @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), + unittest_path(unittest_nr, ovtype), + !after ? "enabled" : "disabled"); + + KUNIT_ASSERT_EQ_MSG(test, of_overlay_remove(&ovcs_id), 0, + "%s failed to be destroyed @\"%s\"\n", + overlay_name_from_nr(overlay_nr), + unittest_path(unittest_nr, ovtype)); /* unittest device must be again in before state */ - if (of_unittest_device_exists(unittest_nr, PDEV_OVERLAY) != before) { - unittest(0, "%s with device @\"%s\" %s\n", - overlay_name_from_nr(overlay_nr), - unittest_path(unittest_nr, ovtype), - !before ? "enabled" : "disabled"); - return -EINVAL; - } + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_device_exists(unittest_nr, + PDEV_OVERLAY), + before, + "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr), + unittest_path(unittest_nr, ovtype), + !before ? "enabled" : "disabled"); return 0; } /* test activation of device */ -static void __init of_unittest_overlay_0(void) +static void of_unittest_overlay_0(struct kunit *test) { /* device should enable */ - if (of_unittest_apply_overlay_check(0, 0, 0, 1, PDEV_OVERLAY)) - return; - - unittest(1, "overlay test %d passed\n", 0); + of_unittest_apply_overlay_check(test, 0, 0, 0, 1, PDEV_OVERLAY); } /* test deactivation of device */ -static void __init of_unittest_overlay_1(void) +static void of_unittest_overlay_1(struct kunit *test) { /* device should disable */ - if (of_unittest_apply_overlay_check(1, 1, 1, 0, PDEV_OVERLAY)) - return; - - unittest(1, "overlay test %d passed\n", 1); + of_unittest_apply_overlay_check(test, 1, 1, 1, 0, PDEV_OVERLAY); } /* test activation of device */ -static void __init of_unittest_overlay_2(void) +static void of_unittest_overlay_2(struct kunit *test) { /* device should enable */ - if (of_unittest_apply_overlay_check(2, 2, 0, 1, PDEV_OVERLAY)) - return; - - unittest(1, "overlay test %d passed\n", 2); + of_unittest_apply_overlay_check(test, 2, 2, 0, 1, PDEV_OVERLAY); } /* test deactivation of device */ -static void __init of_unittest_overlay_3(void) +static void of_unittest_overlay_3(struct kunit *test) { /* device should disable */ - if (of_unittest_apply_overlay_check(3, 3, 1, 0, PDEV_OVERLAY)) - return; - - unittest(1, "overlay test %d passed\n", 3); + of_unittest_apply_overlay_check(test, 3, 3, 1, 0, PDEV_OVERLAY); } /* test activation of a full device node */ -static void __init of_unittest_overlay_4(void) +static void of_unittest_overlay_4(struct kunit *test) { /* device should disable */ - if (of_unittest_apply_overlay_check(4, 4, 0, 1, PDEV_OVERLAY)) - return; - - unittest(1, "overlay test %d passed\n", 4); + of_unittest_apply_overlay_check(test, 4, 4, 0, 1, PDEV_OVERLAY); } /* test overlay apply/revert sequence */ -static void __init of_unittest_overlay_5(void) +static void of_unittest_overlay_5(struct kunit *test) { /* device should disable */ - if (of_unittest_apply_revert_overlay_check(5, 5, 0, 1, PDEV_OVERLAY)) - return; - - unittest(1, "overlay test %d passed\n", 5); + of_unittest_apply_revert_overlay_check(test, 5, 5, 0, 1, PDEV_OVERLAY); } /* test overlay application in sequence */ -static void __init of_unittest_overlay_6(void) +static void of_unittest_overlay_6(struct kunit *test) { int i, ov_id[2], ovcs_id; int overlay_nr = 6, unittest_nr = 6; @@ -1604,74 +1726,69 @@ static void __init of_unittest_overlay_6(void) /* unittest device must be in before state */ for (i = 0; i < 2; i++) { - if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY) - != before) { - unittest(0, "%s with device @\"%s\" %s\n", - overlay_name_from_nr(overlay_nr + i), - unittest_path(unittest_nr + i, - PDEV_OVERLAY), - !before ? "enabled" : "disabled"); - return; - } + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_device_exists(unittest_nr + i, + PDEV_OVERLAY), + before, + "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr + i), + unittest_path(unittest_nr + i, + PDEV_OVERLAY), + !before ? "enabled" : "disabled"); } /* apply the overlays */ for (i = 0; i < 2; i++) { - overlay_name = overlay_name_from_nr(overlay_nr + i); - if (!overlay_data_apply(overlay_name, &ovcs_id)) { - unittest(0, "could not apply overlay \"%s\"\n", - overlay_name); - return; - } + KUNIT_ASSERT_TRUE_MSG(test, + overlay_data_apply(overlay_name, + &ovcs_id), + "could not apply overlay \"%s\"\n", + overlay_name); ov_id[i] = ovcs_id; of_unittest_track_overlay(ov_id[i]); } for (i = 0; i < 2; i++) { /* unittest device must be in after state */ - if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY) - != after) { - unittest(0, "overlay @\"%s\" failed @\"%s\" %s\n", - overlay_name_from_nr(overlay_nr + i), - unittest_path(unittest_nr + i, - PDEV_OVERLAY), - !after ? "enabled" : "disabled"); - return; - } + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_device_exists(unittest_nr + i, + PDEV_OVERLAY), + after, + "overlay @\"%s\" failed @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr + i), + unittest_path(unittest_nr + i, + PDEV_OVERLAY), + !after ? "enabled" : "disabled"); } for (i = 1; i >= 0; i--) { ovcs_id = ov_id[i]; - if (of_overlay_remove(&ovcs_id)) { - unittest(0, "%s failed destroy @\"%s\"\n", - overlay_name_from_nr(overlay_nr + i), - unittest_path(unittest_nr + i, - PDEV_OVERLAY)); - return; - } + KUNIT_ASSERT_FALSE_MSG(test, of_overlay_remove(&ovcs_id), + "%s failed destroy @\"%s\"\n", + overlay_name_from_nr(overlay_nr + i), + unittest_path(unittest_nr + i, + PDEV_OVERLAY)); of_unittest_untrack_overlay(ov_id[i]); } for (i = 0; i < 2; i++) { /* unittest device must be again in before state */ - if (of_unittest_device_exists(unittest_nr + i, PDEV_OVERLAY) - != before) { - unittest(0, "%s with device @\"%s\" %s\n", - overlay_name_from_nr(overlay_nr + i), - unittest_path(unittest_nr + i, - PDEV_OVERLAY), - !before ? "enabled" : "disabled"); - return; - } + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_device_exists(unittest_nr + i, + PDEV_OVERLAY), + before, + "%s with device @\"%s\" %s\n", + overlay_name_from_nr(overlay_nr + i), + unittest_path(unittest_nr + i, + PDEV_OVERLAY), + !before ? "enabled" : "disabled"); } - - unittest(1, "overlay test %d passed\n", 6); } /* test overlay application in sequence */ -static void __init of_unittest_overlay_8(void) +static void of_unittest_overlay_8(struct kunit *test) { int i, ov_id[2], ovcs_id; int overlay_nr = 8, unittest_nr = 8; @@ -1681,76 +1798,73 @@ static void __init of_unittest_overlay_8(void) /* apply the overlays */ for (i = 0; i < 2; i++) { - overlay_name = overlay_name_from_nr(overlay_nr + i); - if (!overlay_data_apply(overlay_name, &ovcs_id)) { - unittest(0, "could not apply overlay \"%s\"\n", - overlay_name); - return; - } + KUNIT_ASSERT_TRUE_MSG(test, + overlay_data_apply(overlay_name, + &ovcs_id), + "could not apply overlay \"%s\"\n", + overlay_name); ov_id[i] = ovcs_id; of_unittest_track_overlay(ov_id[i]); } /* now try to remove first overlay (it should fail) */ ovcs_id = ov_id[0]; - if (!of_overlay_remove(&ovcs_id)) { - unittest(0, "%s was destroyed @\"%s\"\n", - overlay_name_from_nr(overlay_nr + 0), - unittest_path(unittest_nr, - PDEV_OVERLAY)); - return; - } + KUNIT_ASSERT_TRUE_MSG(test, of_overlay_remove(&ovcs_id), + "%s was destroyed @\"%s\"\n", + overlay_name_from_nr(overlay_nr + 0), + unittest_path(unittest_nr, + PDEV_OVERLAY)); /* removing them in order should work */ for (i = 1; i >= 0; i--) { ovcs_id = ov_id[i]; - if (of_overlay_remove(&ovcs_id)) { - unittest(0, "%s not destroyed @\"%s\"\n", - overlay_name_from_nr(overlay_nr + i), - unittest_path(unittest_nr, - PDEV_OVERLAY)); - return; - } + KUNIT_ASSERT_FALSE_MSG(test, of_overlay_remove(&ovcs_id), + "%s not destroyed @\"%s\"\n", + overlay_name_from_nr(overlay_nr + i), + unittest_path(unittest_nr, + PDEV_OVERLAY)); of_unittest_untrack_overlay(ov_id[i]); } - - unittest(1, "overlay test %d passed\n", 8); } /* test insertion of a bus with parent devices */ -static void __init of_unittest_overlay_10(void) +static void of_unittest_overlay_10(struct kunit *test) { - int ret; char *child_path; /* device should disable */ - ret = of_unittest_apply_overlay_check(10, 10, 0, 1, PDEV_OVERLAY); - if (unittest(ret == 0, - "overlay test %d failed; overlay application\n", 10)) - return; + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_apply_overlay_check(test, + 10, + 10, + 0, + 1, + PDEV_OVERLAY), + 0, + "overlay test %d failed; overlay application\n", + 10); child_path = kasprintf(GFP_KERNEL, "%s/test-unittest101", unittest_path(10, PDEV_OVERLAY)); - if (unittest(child_path, "overlay test %d failed; kasprintf\n", 10)) - return; + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, child_path); - ret = of_path_device_type_exists(child_path, PDEV_OVERLAY); + KUNIT_EXPECT_TRUE_MSG(test, + of_path_device_type_exists(child_path, + PDEV_OVERLAY), + "overlay test %d failed; no child device\n", 10); kfree(child_path); - - unittest(ret, "overlay test %d failed; no child device\n", 10); } /* test insertion of a bus with parent devices (and revert) */ -static void __init of_unittest_overlay_11(void) +static void of_unittest_overlay_11(struct kunit *test) { - int ret; - /* device should disable */ - ret = of_unittest_apply_revert_overlay_check(11, 11, 0, 1, - PDEV_OVERLAY); - unittest(ret == 0, "overlay test %d failed; overlay apply\n", 11); + KUNIT_EXPECT_FALSE(test, + of_unittest_apply_revert_overlay_check(test, + 11, 11, 0, 1, + PDEV_OVERLAY)); } #if IS_BUILTIN(CONFIG_I2C) && IS_ENABLED(CONFIG_OF_OVERLAY) @@ -1972,25 +2086,23 @@ static struct i2c_driver unittest_i2c_mux_driver = { #endif -static int of_unittest_overlay_i2c_init(void) +static int of_unittest_overlay_i2c_init(struct kunit *test) { - int ret; + KUNIT_ASSERT_EQ_MSG(test, + i2c_add_driver(&unittest_i2c_dev_driver), + 0, + "could not register unittest i2c device driver\n"); - ret = i2c_add_driver(&unittest_i2c_dev_driver); - if (unittest(ret == 0, - "could not register unittest i2c device driver\n")) - return ret; - - ret = platform_driver_register(&unittest_i2c_bus_driver); - if (unittest(ret == 0, - "could not register unittest i2c bus driver\n")) - return ret; + KUNIT_ASSERT_EQ_MSG(test, + platform_driver_register(&unittest_i2c_bus_driver), + 0, + "could not register unittest i2c bus driver\n"); #if IS_BUILTIN(CONFIG_I2C_MUX) - ret = i2c_add_driver(&unittest_i2c_mux_driver); - if (unittest(ret == 0, - "could not register unittest i2c mux driver\n")) - return ret; + KUNIT_ASSERT_EQ_MSG(test, + i2c_add_driver(&unittest_i2c_mux_driver), + 0, + "could not register unittest i2c mux driver\n"); #endif return 0; @@ -2005,101 +2117,89 @@ static void of_unittest_overlay_i2c_cleanup(void) i2c_del_driver(&unittest_i2c_dev_driver); } -static void __init of_unittest_overlay_i2c_12(void) +static void of_unittest_overlay_i2c_12(struct kunit *test) { /* device should enable */ - if (of_unittest_apply_overlay_check(12, 12, 0, 1, I2C_OVERLAY)) - return; - - unittest(1, "overlay test %d passed\n", 12); + of_unittest_apply_overlay_check(test, 12, 12, 0, 1, I2C_OVERLAY); } /* test deactivation of device */ -static void __init of_unittest_overlay_i2c_13(void) +static void of_unittest_overlay_i2c_13(struct kunit *test) { /* device should disable */ - if (of_unittest_apply_overlay_check(13, 13, 1, 0, I2C_OVERLAY)) - return; - - unittest(1, "overlay test %d passed\n", 13); + of_unittest_apply_overlay_check(test, 13, 13, 1, 0, I2C_OVERLAY); } /* just check for i2c mux existence */ -static void of_unittest_overlay_i2c_14(void) +static void of_unittest_overlay_i2c_14(struct kunit *test) { + KUNIT_SUCCEED(test); } -static void __init of_unittest_overlay_i2c_15(void) +static void of_unittest_overlay_i2c_15(struct kunit *test) { /* device should enable */ - if (of_unittest_apply_overlay_check(15, 15, 0, 1, I2C_OVERLAY)) - return; - - unittest(1, "overlay test %d passed\n", 15); + of_unittest_apply_overlay_check(test, 15, 15, 0, 1, I2C_OVERLAY); } #else -static inline void of_unittest_overlay_i2c_14(void) { } -static inline void of_unittest_overlay_i2c_15(void) { } +static inline void of_unittest_overlay_i2c_14(struct kunit *test) { } +static inline void of_unittest_overlay_i2c_15(struct kunit *test) { } #endif -static void __init of_unittest_overlay(void) +static void of_unittest_overlay(struct kunit *test) { struct device_node *bus_np = NULL; - if (platform_driver_register(&unittest_driver)) { - unittest(0, "could not register unittest driver\n"); - goto out; - } + KUNIT_ASSERT_FALSE_MSG(test, + platform_driver_register(&unittest_driver), + "could not register unittest driver\n"); bus_np = of_find_node_by_path(bus_path); - if (bus_np == NULL) { - unittest(0, "could not find bus_path \"%s\"\n", bus_path); - goto out; - } + KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, bus_np, + "could not find bus_path \"%s\"\n", + bus_path); - if (of_platform_default_populate(bus_np, NULL, NULL)) { - unittest(0, "could not populate bus @ \"%s\"\n", bus_path); - goto out; - } + KUNIT_ASSERT_FALSE_MSG(test, + of_platform_default_populate(bus_np, NULL, NULL), + "could not populate bus @ \"%s\"\n", bus_path); - if (!of_unittest_device_exists(100, PDEV_OVERLAY)) { - unittest(0, "could not find unittest0 @ \"%s\"\n", - unittest_path(100, PDEV_OVERLAY)); - goto out; - } + KUNIT_ASSERT_TRUE_MSG(test, + of_unittest_device_exists(100, PDEV_OVERLAY), + "could not find unittest0 @ \"%s\"\n", + unittest_path(100, PDEV_OVERLAY)); - if (of_unittest_device_exists(101, PDEV_OVERLAY)) { - unittest(0, "unittest1 @ \"%s\" should not exist\n", - unittest_path(101, PDEV_OVERLAY)); - goto out; - } - - unittest(1, "basic infrastructure of overlays passed"); + KUNIT_ASSERT_FALSE_MSG(test, + of_unittest_device_exists(101, PDEV_OVERLAY), + "unittest1 @ \"%s\" should not exist\n", + unittest_path(101, PDEV_OVERLAY)); /* tests in sequence */ - of_unittest_overlay_0(); - of_unittest_overlay_1(); - of_unittest_overlay_2(); - of_unittest_overlay_3(); - of_unittest_overlay_4(); - of_unittest_overlay_5(); - of_unittest_overlay_6(); - of_unittest_overlay_8(); - - of_unittest_overlay_10(); - of_unittest_overlay_11(); + of_unittest_overlay_0(test); + of_unittest_overlay_1(test); + of_unittest_overlay_2(test); + of_unittest_overlay_3(test); + of_unittest_overlay_4(test); + of_unittest_overlay_5(test); + of_unittest_overlay_6(test); + of_unittest_overlay_8(test); + + of_unittest_overlay_10(test); + of_unittest_overlay_11(test); #if IS_BUILTIN(CONFIG_I2C) - if (unittest(of_unittest_overlay_i2c_init() == 0, "i2c init failed\n")) + KUNIT_ASSERT_EQ_MSG(test, + of_unittest_overlay_i2c_init(test), + 0, + "i2c init failed\n"); goto out; - of_unittest_overlay_i2c_12(); - of_unittest_overlay_i2c_13(); - of_unittest_overlay_i2c_14(); - of_unittest_overlay_i2c_15(); + of_unittest_overlay_i2c_12(test); + of_unittest_overlay_i2c_13(test); + of_unittest_overlay_i2c_14(test); + of_unittest_overlay_i2c_15(test); of_unittest_overlay_i2c_cleanup(); #endif @@ -2111,7 +2211,7 @@ static void __init of_unittest_overlay(void) } #else -static inline void __init of_unittest_overlay(void) { } +static inline void of_unittest_overlay(struct kunit *test) { } #endif #ifdef CONFIG_OF_OVERLAY @@ -2254,7 +2354,7 @@ void __init unittest_unflatten_overlay_base(void) * * Return 0 on unexpected error. */ -static int __init overlay_data_apply(const char *overlay_name, int *overlay_id) +static int overlay_data_apply(const char *overlay_name, int *overlay_id) { struct overlay_info *info; int found = 0; @@ -2301,19 +2401,17 @@ static int __init overlay_data_apply(const char *overlay_name, int *overlay_id) * The first part of the function is _not_ normal overlay usage; it is * finishing splicing the base overlay device tree into the live tree. */ -static __init void of_unittest_overlay_high_level(void) +static void of_unittest_overlay_high_level(struct kunit *test) { struct device_node *last_sibling; struct device_node *np; struct device_node *of_symbols; - struct device_node *overlay_base_symbols; + struct device_node *overlay_base_symbols = 0; struct device_node **pprev; struct property *prop; - if (!overlay_base_root) { - unittest(0, "overlay_base_root not initialized\n"); - return; - } + KUNIT_ASSERT_TRUE_MSG(test, overlay_base_root, + "overlay_base_root not initialized\n"); /* * Could not fixup phandles in unittest_unflatten_overlay_base() @@ -2358,11 +2456,10 @@ static __init void of_unittest_overlay_high_level(void) } for (np = overlay_base_root->child; np; np = np->sibling) { - if (of_get_child_by_name(of_root, np->name)) { - unittest(0, "illegal node name in overlay_base %s", - np->name); - return; - } + KUNIT_ASSERT_FALSE_MSG(test, + of_get_child_by_name(of_root, np->name), + "illegal node name in overlay_base %s", + np->name); } /* @@ -2395,21 +2492,24 @@ static __init void of_unittest_overlay_high_level(void) new_prop = __of_prop_dup(prop, GFP_KERNEL); if (!new_prop) { - unittest(0, "__of_prop_dup() of '%s' from overlay_base node __symbols__", - prop->name); + KUNIT_FAIL(test, + "__of_prop_dup() of '%s' from overlay_base node __symbols__", + prop->name); goto err_unlock; } if (__of_add_property(of_symbols, new_prop)) { /* "name" auto-generated by unflatten */ if (!strcmp(new_prop->name, "name")) continue; - unittest(0, "duplicate property '%s' in overlay_base node __symbols__", - prop->name); + KUNIT_FAIL(test, + "duplicate property '%s' in overlay_base node __symbols__", + prop->name); goto err_unlock; } if (__of_add_property_sysfs(of_symbols, new_prop)) { - unittest(0, "unable to add property '%s' in overlay_base node __symbols__ to sysfs", - prop->name); + KUNIT_FAIL(test, + "unable to add property '%s' in overlay_base node __symbols__ to sysfs", + prop->name); goto err_unlock; } } @@ -2420,14 +2520,16 @@ static __init void of_unittest_overlay_high_level(void) /* now do the normal overlay usage test */ - unittest(overlay_data_apply("overlay", NULL), - "Adding overlay 'overlay' failed\n"); + KUNIT_EXPECT_TRUE_MSG(test, overlay_data_apply("overlay", NULL), + "Adding overlay 'overlay' failed\n"); - unittest(overlay_data_apply("overlay_bad_phandle", NULL), - "Adding overlay 'overlay_bad_phandle' failed\n"); + KUNIT_EXPECT_TRUE_MSG(test, + overlay_data_apply("overlay_bad_phandle", NULL), + "Adding overlay 'overlay_bad_phandle' failed\n"); - unittest(overlay_data_apply("overlay_bad_symbol", NULL), - "Adding overlay 'overlay_bad_symbol' failed\n"); + KUNIT_EXPECT_TRUE_MSG(test, + overlay_data_apply("overlay_bad_symbol", NULL), + "Adding overlay 'overlay_bad_symbol' failed\n"); return; @@ -2437,54 +2539,49 @@ static __init void of_unittest_overlay_high_level(void) #else -static inline __init void of_unittest_overlay_high_level(void) {} +static inline void of_unittest_overlay_high_level(struct kunit *test) {} #endif -static int __init of_unittest(void) +static int of_test_init(struct kunit *test) { - struct device_node *np; - int res; - /* adding data for unittest */ - res = unittest_data_add(); - if (res) - return res; + KUNIT_ASSERT_EQ(test, 0, unittest_data_add()); + if (!of_aliases) of_aliases = of_find_node_by_path("/aliases"); - np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a"); - if (!np) { - pr_info("No testcase data in device tree; not running tests\n"); - return 0; - } - of_node_put(np); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, of_find_node_by_path( + "/testcase-data/phandle-tests/consumer-a")); - pr_info("start of unittest - you will see error messages\n"); - of_unittest_check_tree_linkage(); - of_unittest_check_phandles(); - of_unittest_find_node_by_name(); - of_unittest_dynamic(); - of_unittest_parse_phandle_with_args(); - of_unittest_parse_phandle_with_args_map(); - of_unittest_printf(); - of_unittest_property_string(); - of_unittest_property_copy(); - of_unittest_changeset(); - of_unittest_parse_interrupts(); - of_unittest_parse_interrupts_extended(); - of_unittest_match_node(); - of_unittest_platform_populate(); - of_unittest_overlay(); + return 0; +} +static struct kunit_case of_test_cases[] = { + KUNIT_CASE(of_unittest_check_tree_linkage), + KUNIT_CASE(of_unittest_check_phandles), + KUNIT_CASE(of_unittest_find_node_by_name), + KUNIT_CASE(of_unittest_dynamic), + KUNIT_CASE(of_unittest_parse_phandle_with_args), + KUNIT_CASE(of_unittest_parse_phandle_with_args_map), + KUNIT_CASE(of_unittest_printf), + KUNIT_CASE(of_unittest_property_string), + KUNIT_CASE(of_unittest_property_copy), + KUNIT_CASE(of_unittest_changeset), + KUNIT_CASE(of_unittest_parse_interrupts), + KUNIT_CASE(of_unittest_parse_interrupts_extended), + KUNIT_CASE(of_unittest_match_node), + KUNIT_CASE(of_unittest_platform_populate), + KUNIT_CASE(of_unittest_overlay), /* Double check linkage after removing testcase data */ - of_unittest_check_tree_linkage(); - - of_unittest_overlay_high_level(); - - pr_info("end of unittest - %i passed, %i failed\n", - unittest_results.passed, unittest_results.failed); + KUNIT_CASE(of_unittest_check_tree_linkage), + KUNIT_CASE(of_unittest_overlay_high_level), + {}, +}; - return 0; -} -late_initcall(of_unittest); +static struct kunit_module of_test_module = { + .name = "of-test", + .init = of_test_init, + .test_cases = of_test_cases, +}; +module_test(of_test_module);
Migrate tests without any cleanup, or modifying test logic in anyway to run under KUnit using the KUnit expectation and assertion API. Signed-off-by: Brendan Higgins <brendanhiggins@google.com> --- drivers/of/Kconfig | 1 + drivers/of/unittest.c | 1405 ++++++++++++++++++++++------------------- 2 files changed, 752 insertions(+), 654 deletions(-)