| Message ID | 20230809155438.22470-3-rf@opensource.cirrus.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | kunit: Add dynamically-extending log | expand |
On Wed, Aug 9, 2023 at 11:54 AM Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > > Add test cases for the dynamically-extending log buffer. > > kunit_log_init_frag_test() tests that kunit_init_log_frag() correctly > initializes new struct kunit_log_frag. > > kunit_log_extend_test_1() logs a series of numbered lines then tests > that the resulting log contains all the lines. > > kunit_log_extend_test_2() logs a large number of lines of varying length > to create many fragments, then tests that all lines are present. > > kunit_log_newline_test() has a new test to append a line that is exactly > the length of the available space in the current fragment and check that > the resulting log has a trailing '\n'. > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> Hello! These tests now pass for me. Thanks! I do have a few comments below mostly regarding comments and a few clarifying questions. -Rae > --- > lib/kunit/kunit-test.c | 182 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 174 insertions(+), 8 deletions(-) > > diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c > index a199f83bac67..c0ee33a8031e 100644 > --- a/lib/kunit/kunit-test.c > +++ b/lib/kunit/kunit-test.c > @@ -7,6 +7,7 @@ > */ > #include <kunit/test.h> > #include <kunit/test-bug.h> > +#include <linux/prandom.h> > > #include "try-catch-impl.h" > > @@ -530,10 +531,12 @@ static struct kunit_suite kunit_resource_test_suite = { > .test_cases = kunit_resource_test_cases, > }; > > -static char *get_concatenated_log(struct kunit *test, const struct list_head *log) > +static char *get_concatenated_log(struct kunit *test, const struct list_head *log, > + int *num_frags) > { > struct kunit_log_frag *frag; > size_t len = 0; > + int frag_count = 0; > char *p; > > list_for_each_entry(frag, log, list) > @@ -542,24 +545,42 @@ static char *get_concatenated_log(struct kunit *test, const struct list_head *lo > len++; /* for terminating '\0' */ > p = kunit_kzalloc(test, len, GFP_KERNEL); > > - list_for_each_entry(frag, log, list) > + list_for_each_entry(frag, log, list) { > strlcat(p, frag->buf, len); > + ++frag_count; > + } > + > + if (num_frags) > + *num_frags = frag_count; > > return p; > } > > -static void kunit_log_test(struct kunit *test) > +static void kunit_log_init_frag_test(struct kunit *test) > { > - struct kunit_suite suite; > struct kunit_log_frag *frag; > > - suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); > - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); > - INIT_LIST_HEAD(suite.log); > frag = kunit_kmalloc(test, sizeof(*frag), GFP_KERNEL); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); > + memset(frag, 0x5a, sizeof(*frag)); > + Why is the fragment getting filled here with memset? Should this be tested? Feel free to let me know, I'm just uncertain. > kunit_init_log_frag(frag); > KUNIT_EXPECT_EQ(test, frag->buf[0], '\0'); > + KUNIT_EXPECT_TRUE(test, list_is_first(&frag->list, &frag->list)); > + KUNIT_EXPECT_TRUE(test, list_is_last(&frag->list, &frag->list)); > +} > + > +static void kunit_log_test(struct kunit *test) > +{ > + struct kunit_suite suite; > + struct kunit_log_frag *frag; > + > + suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); > + INIT_LIST_HEAD(suite.log); > + frag = kunit_kzalloc(test, sizeof(*frag), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); > + kunit_init_log_frag(frag); > list_add_tail(&frag->list, suite.log); > > kunit_log(KERN_INFO, test, "put this in log."); > @@ -586,23 +607,168 @@ static void kunit_log_test(struct kunit *test) > > static void kunit_log_newline_test(struct kunit *test) > { > + struct kunit_suite suite; > struct kunit_log_frag *frag; > + char *p; Similar to last email, could we change p to be a more descriptive name such as concat_log? > > kunit_info(test, "Add newline\n"); > if (test->log) { > frag = list_first_entry(test->log, struct kunit_log_frag, list); > KUNIT_ASSERT_NOT_NULL_MSG(test, strstr(frag->buf, "Add newline\n"), > "Missing log line, full log:\n%s", > - get_concatenated_log(test, test->log)); > + get_concatenated_log(test, test->log, NULL)); > KUNIT_EXPECT_NULL(test, strstr(frag->buf, "Add newline\n\n")); > + Should this section of kunit_log_newline_test be separated into a new test? This test seems a bit long and seems to have two distinct sections? > + suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); > + INIT_LIST_HEAD(suite.log); I would love to see a comment here to explain and break up this section similar to the comment from the previous email. > + frag = kunit_kzalloc(test, sizeof(*frag), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); > + kunit_init_log_frag(frag); > + list_add_tail(&frag->list, suite.log); > + > + /* String that exactly fills fragment leaving no room for \n */ > + memset(frag->buf, 0, sizeof(frag->buf)); > + memset(frag->buf, 'x', sizeof(frag->buf) - 9); > + kunit_log_append(suite.log, "12345678"); > + p = get_concatenated_log(test, suite.log, NULL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p); > + KUNIT_EXPECT_NOT_NULL_MSG(test, strstr(p, "x12345678\n"), > + "Newline not appended when fragment is full. Log is:\n'%s'", p); > } else { > kunit_skip(test, "only useful when debugfs is enabled"); > } > } > > +static void kunit_log_extend_test_1(struct kunit *test) In general, I would really like to see more comments in the next two tests describing the test behavior. I would prefer a comment for each of the while/do-while loops below. I just found the behavior to be slightly confusing to understand without comments (although I do appreciate the comments that are in kunit_log_extend_test_2). Also, I really appreciate how detailed these tests are. Another potential idea is to rename these two tests to be kunit_log_extend_test() and kunit_log_rand_extend_test() instead to be more descriptive? > +{ > +#ifdef CONFIG_KUNIT_DEBUGFS > + struct kunit_suite suite; > + struct kunit_log_frag *frag; > + char line[60]; > + char *p, *pn; Similar to before, could we change p and pn to be slightly more descriptive names? Maybe concat_log and newline_ptr or newline_log or newline_char? > + size_t len, n; > + int num_lines, num_frags, i; > + > + suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); > + INIT_LIST_HEAD(suite.log); > + frag = kunit_kzalloc(test, sizeof(*frag), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); > + kunit_init_log_frag(frag); > + list_add_tail(&frag->list, suite.log); > + > + i = 0; > + len = 0; > + do { > + n = snprintf(line, sizeof(line), > + "The quick brown fox jumps over the lazy penguin %d\n", i); > + KUNIT_ASSERT_LT(test, n, sizeof(line)); > + kunit_log_append(suite.log, line); > + ++i; > + len += n; > + } while (len < (sizeof(frag->buf) * 30)); Are we trying to restrict the num_frags to less than 30? And then we could check that with a KUNIT_EXPECT? Currently, the num_frags are just above 30. That is ok too. I just was wondering if this was intentional? (Same as kunit_log_extend_test_2) > + num_lines = i; > + > + p = get_concatenated_log(test, suite.log, &num_frags); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p); > + KUNIT_EXPECT_GT(test, num_frags, 1); > + > + kunit_info(test, "num lines:%d num_frags:%d total len:%zu\n", > + num_lines, num_frags, strlen(p)); > + > + i = 0; > + while ((pn = strchr(p, '\n')) != NULL) { > + *pn = '\0'; > + snprintf(line, sizeof(line), > + "The quick brown fox jumps over the lazy penguin %d", i); > + KUNIT_EXPECT_STREQ(test, p, line); > + p = pn + 1; > + ++i; > + } > + KUNIT_EXPECT_EQ(test, i, num_lines); > +#else > + kunit_skip(test, "only useful when debugfs is enabled"); > +#endif > +} > + > +static void kunit_log_extend_test_2(struct kunit *test) > +{ > +#ifdef CONFIG_KUNIT_DEBUGFS > + struct kunit_suite suite; > + struct kunit_log_frag *frag; > + struct rnd_state rnd; > + char line[101]; > + char *p, *pn; Similar to above, could p and pn be renamed to be more descriptive? > + size_t len; > + int num_lines, num_frags, n, i; > + > + suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); > + INIT_LIST_HEAD(suite.log); > + frag = kunit_kzalloc(test, sizeof(*frag), GFP_KERNEL); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); > + kunit_init_log_frag(frag); > + list_add_tail(&frag->list, suite.log); > + > + /* Build log line of varying content */ > + line[0] = '\0'; > + i = 0; > + do { > + char tmp[9]; > + > + snprintf(tmp, sizeof(tmp), "%x", i++); > + len = strlcat(line, tmp, sizeof(line)); > + } while (len < sizeof(line) - 1); Could there be an expectation statement here to check the line has been properly filled. Maybe checking the length? > + > + /* > + * Log lines of different lengths until we have created > + * many fragments. > + * The "randomness" must be repeatable. > + */ > + prandom_seed_state(&rnd, 3141592653589793238ULL); > + i = 0; > + len = 0; > + num_lines = 0; > + do { > + kunit_log_append(suite.log, "%s\n", &line[i]); > + len += sizeof(line) - i; > + num_lines++; > + i = prandom_u32_state(&rnd) % (sizeof(line) - 1); > + } while (len < (sizeof(frag->buf) * 30)); > + > + /* There must be more than one buffer fragment now */ > + KUNIT_EXPECT_FALSE(test, list_is_singular(suite.log)); > + > + p = get_concatenated_log(test, suite.log, &num_frags); > + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p); > + KUNIT_EXPECT_GT(test, num_frags, 1); > + > + kunit_info(test, "num lines:%d num_frags:%d total len:%zu\n", > + num_lines, num_frags, strlen(p)); > + > + prandom_seed_state(&rnd, 3141592653589793238ULL); > + i = 0; > + n = 0; > + while ((pn = strchr(p, '\n')) != NULL) { > + *pn = '\0'; > + KUNIT_EXPECT_STREQ(test, p, &line[i]); > + p = pn + 1; > + n++; > + i = prandom_u32_state(&rnd) % (sizeof(line) - 1); > + } > + KUNIT_EXPECT_EQ_MSG(test, n, num_lines, "Not enough lines."); Is it possible for this to be too many lines instead? Should this comment instead be "Unexpected number of lines". Also could we have a similar message for the test above for this expectation regarding the number of lines. > +#else > + kunit_skip(test, "only useful when debugfs is enabled"); > +#endif > +} > + > static struct kunit_case kunit_log_test_cases[] = { > + KUNIT_CASE(kunit_log_init_frag_test), > KUNIT_CASE(kunit_log_test), > KUNIT_CASE(kunit_log_newline_test), > + KUNIT_CASE(kunit_log_extend_test_1), > + KUNIT_CASE(kunit_log_extend_test_2), > {} > }; > > -- > 2.30.2 >
On 9/8/23 22:10, Rae Moar wrote: > On Wed, Aug 9, 2023 at 11:54 AM Richard Fitzgerald > <rf@opensource.cirrus.com> wrote: >> >> Add test cases for the dynamically-extending log buffer. >> >> kunit_log_init_frag_test() tests that kunit_init_log_frag() correctly >> initializes new struct kunit_log_frag. >> >> kunit_log_extend_test_1() logs a series of numbered lines then tests >> that the resulting log contains all the lines. >> >> kunit_log_extend_test_2() logs a large number of lines of varying length >> to create many fragments, then tests that all lines are present. >> >> kunit_log_newline_test() has a new test to append a line that is exactly >> the length of the available space in the current fragment and check that >> the resulting log has a trailing '\n'. >> >> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> > > Hello! > > These tests now pass for me. Thanks! > > I do have a few comments below mostly regarding comments and a few > clarifying questions. > > -Rae ... >> +static void kunit_log_init_frag_test(struct kunit *test) >> { >> - struct kunit_suite suite; >> struct kunit_log_frag *frag; >> >> - suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); >> - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); >> - INIT_LIST_HEAD(suite.log); >> frag = kunit_kmalloc(test, sizeof(*frag), GFP_KERNEL); >> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); >> + memset(frag, 0x5a, sizeof(*frag)); >> + > > Why is the fragment getting filled here with memset? Should this be > tested? Feel free to let me know, I'm just uncertain. I'll add a comment in V4. It's to prove that kunit_init_log_frag() really did change something. kzalloc() is no good for this because we want to see that kunit_log_frag() zeroed buf[0]. ... >> kunit_info(test, "Add newline\n"); >> if (test->log) { >> frag = list_first_entry(test->log, struct kunit_log_frag, list); >> KUNIT_ASSERT_NOT_NULL_MSG(test, strstr(frag->buf, "Add newline\n"), >> "Missing log line, full log:\n%s", >> - get_concatenated_log(test, test->log)); >> + get_concatenated_log(test, test->log, NULL)); >> KUNIT_EXPECT_NULL(test, strstr(frag->buf, "Add newline\n\n")); >> + > > Should this section of kunit_log_newline_test be separated into a new > test? This test seems a bit long and seems to have two distinct > sections? Yes, it makes sense to add a separate test case for when newlines cause the log to extend. ... > Another potential idea is to rename these two tests to be > kunit_log_extend_test() and kunit_log_rand_extend_test() instead to be > more descriptive? TBH I had trouble thinking of a short description. But I'll spend some time thinking about naming. ... >> + do { >> + n = snprintf(line, sizeof(line), >> + "The quick brown fox jumps over the lazy penguin %d\n", i); >> + KUNIT_ASSERT_LT(test, n, sizeof(line)); >> + kunit_log_append(suite.log, line); >> + ++i; >> + len += n; >> + } while (len < (sizeof(frag->buf) * 30)); > > Are we trying to restrict the num_frags to less than 30? And then we > could check that with a KUNIT_EXPECT? Currently, the num_frags are > just above 30. That is ok too. I just was wondering if this was > intentional? (Same as kunit_log_extend_test_2) I'll comment on this in V4. It's just trying to create "a lot" of data without assuming exactly how kunit_log_append() breaks up the lines across fragments. I don't want to have to keep changing this code if the fragmenting algorithm changes slightly. So the idea is to generate "about 30" buffers worth. I don't mind if it's a bit more, or a bit less. It's done this way, instead of just counting how many fragments were created, to prevent getting into an infinite loop if for some reason kunit_log_append() fails to add fragments. ... >> + /* Build log line of varying content */ >> + line[0] = '\0'; >> + i = 0; >> + do { >> + char tmp[9]; >> + >> + snprintf(tmp, sizeof(tmp), "%x", i++); >> + len = strlcat(line, tmp, sizeof(line)); >> + } while (len < sizeof(line) - 1); > > Could there be an expectation statement here to check the line has > been properly filled. Maybe checking the length? Yes >> + prandom_seed_state(&rnd, 3141592653589793238ULL); >> + i = 0; >> + n = 0; >> + while ((pn = strchr(p, '\n')) != NULL) { >> + *pn = '\0'; >> + KUNIT_EXPECT_STREQ(test, p, &line[i]); >> + p = pn + 1; >> + n++; >> + i = prandom_u32_state(&rnd) % (sizeof(line) - 1); >> + } >> + KUNIT_EXPECT_EQ_MSG(test, n, num_lines, "Not enough lines."); > > Is it possible for this to be too many lines instead? Should this > comment instead be "Unexpected number of lines". Also could we have a > similar message for the test above for this expectation regarding the > number of lines. Fair point. It's only found that the number of lines is wrong, it could be less or more.
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index a199f83bac67..c0ee33a8031e 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -7,6 +7,7 @@ */ #include <kunit/test.h> #include <kunit/test-bug.h> +#include <linux/prandom.h> #include "try-catch-impl.h" @@ -530,10 +531,12 @@ static struct kunit_suite kunit_resource_test_suite = { .test_cases = kunit_resource_test_cases, }; -static char *get_concatenated_log(struct kunit *test, const struct list_head *log) +static char *get_concatenated_log(struct kunit *test, const struct list_head *log, + int *num_frags) { struct kunit_log_frag *frag; size_t len = 0; + int frag_count = 0; char *p; list_for_each_entry(frag, log, list) @@ -542,24 +545,42 @@ static char *get_concatenated_log(struct kunit *test, const struct list_head *lo len++; /* for terminating '\0' */ p = kunit_kzalloc(test, len, GFP_KERNEL); - list_for_each_entry(frag, log, list) + list_for_each_entry(frag, log, list) { strlcat(p, frag->buf, len); + ++frag_count; + } + + if (num_frags) + *num_frags = frag_count; return p; } -static void kunit_log_test(struct kunit *test) +static void kunit_log_init_frag_test(struct kunit *test) { - struct kunit_suite suite; struct kunit_log_frag *frag; - suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); - INIT_LIST_HEAD(suite.log); frag = kunit_kmalloc(test, sizeof(*frag), GFP_KERNEL); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); + memset(frag, 0x5a, sizeof(*frag)); + kunit_init_log_frag(frag); KUNIT_EXPECT_EQ(test, frag->buf[0], '\0'); + KUNIT_EXPECT_TRUE(test, list_is_first(&frag->list, &frag->list)); + KUNIT_EXPECT_TRUE(test, list_is_last(&frag->list, &frag->list)); +} + +static void kunit_log_test(struct kunit *test) +{ + struct kunit_suite suite; + struct kunit_log_frag *frag; + + suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); + INIT_LIST_HEAD(suite.log); + frag = kunit_kzalloc(test, sizeof(*frag), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); + kunit_init_log_frag(frag); list_add_tail(&frag->list, suite.log); kunit_log(KERN_INFO, test, "put this in log."); @@ -586,23 +607,168 @@ static void kunit_log_test(struct kunit *test) static void kunit_log_newline_test(struct kunit *test) { + struct kunit_suite suite; struct kunit_log_frag *frag; + char *p; kunit_info(test, "Add newline\n"); if (test->log) { frag = list_first_entry(test->log, struct kunit_log_frag, list); KUNIT_ASSERT_NOT_NULL_MSG(test, strstr(frag->buf, "Add newline\n"), "Missing log line, full log:\n%s", - get_concatenated_log(test, test->log)); + get_concatenated_log(test, test->log, NULL)); KUNIT_EXPECT_NULL(test, strstr(frag->buf, "Add newline\n\n")); + + suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); + INIT_LIST_HEAD(suite.log); + frag = kunit_kzalloc(test, sizeof(*frag), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); + kunit_init_log_frag(frag); + list_add_tail(&frag->list, suite.log); + + /* String that exactly fills fragment leaving no room for \n */ + memset(frag->buf, 0, sizeof(frag->buf)); + memset(frag->buf, 'x', sizeof(frag->buf) - 9); + kunit_log_append(suite.log, "12345678"); + p = get_concatenated_log(test, suite.log, NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p); + KUNIT_EXPECT_NOT_NULL_MSG(test, strstr(p, "x12345678\n"), + "Newline not appended when fragment is full. Log is:\n'%s'", p); } else { kunit_skip(test, "only useful when debugfs is enabled"); } } +static void kunit_log_extend_test_1(struct kunit *test) +{ +#ifdef CONFIG_KUNIT_DEBUGFS + struct kunit_suite suite; + struct kunit_log_frag *frag; + char line[60]; + char *p, *pn; + size_t len, n; + int num_lines, num_frags, i; + + suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); + INIT_LIST_HEAD(suite.log); + frag = kunit_kzalloc(test, sizeof(*frag), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); + kunit_init_log_frag(frag); + list_add_tail(&frag->list, suite.log); + + i = 0; + len = 0; + do { + n = snprintf(line, sizeof(line), + "The quick brown fox jumps over the lazy penguin %d\n", i); + KUNIT_ASSERT_LT(test, n, sizeof(line)); + kunit_log_append(suite.log, line); + ++i; + len += n; + } while (len < (sizeof(frag->buf) * 30)); + num_lines = i; + + p = get_concatenated_log(test, suite.log, &num_frags); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p); + KUNIT_EXPECT_GT(test, num_frags, 1); + + kunit_info(test, "num lines:%d num_frags:%d total len:%zu\n", + num_lines, num_frags, strlen(p)); + + i = 0; + while ((pn = strchr(p, '\n')) != NULL) { + *pn = '\0'; + snprintf(line, sizeof(line), + "The quick brown fox jumps over the lazy penguin %d", i); + KUNIT_EXPECT_STREQ(test, p, line); + p = pn + 1; + ++i; + } + KUNIT_EXPECT_EQ(test, i, num_lines); +#else + kunit_skip(test, "only useful when debugfs is enabled"); +#endif +} + +static void kunit_log_extend_test_2(struct kunit *test) +{ +#ifdef CONFIG_KUNIT_DEBUGFS + struct kunit_suite suite; + struct kunit_log_frag *frag; + struct rnd_state rnd; + char line[101]; + char *p, *pn; + size_t len; + int num_lines, num_frags, n, i; + + suite.log = kunit_kzalloc(test, sizeof(*suite.log), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, suite.log); + INIT_LIST_HEAD(suite.log); + frag = kunit_kzalloc(test, sizeof(*frag), GFP_KERNEL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, frag); + kunit_init_log_frag(frag); + list_add_tail(&frag->list, suite.log); + + /* Build log line of varying content */ + line[0] = '\0'; + i = 0; + do { + char tmp[9]; + + snprintf(tmp, sizeof(tmp), "%x", i++); + len = strlcat(line, tmp, sizeof(line)); + } while (len < sizeof(line) - 1); + + /* + * Log lines of different lengths until we have created + * many fragments. + * The "randomness" must be repeatable. + */ + prandom_seed_state(&rnd, 3141592653589793238ULL); + i = 0; + len = 0; + num_lines = 0; + do { + kunit_log_append(suite.log, "%s\n", &line[i]); + len += sizeof(line) - i; + num_lines++; + i = prandom_u32_state(&rnd) % (sizeof(line) - 1); + } while (len < (sizeof(frag->buf) * 30)); + + /* There must be more than one buffer fragment now */ + KUNIT_EXPECT_FALSE(test, list_is_singular(suite.log)); + + p = get_concatenated_log(test, suite.log, &num_frags); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, p); + KUNIT_EXPECT_GT(test, num_frags, 1); + + kunit_info(test, "num lines:%d num_frags:%d total len:%zu\n", + num_lines, num_frags, strlen(p)); + + prandom_seed_state(&rnd, 3141592653589793238ULL); + i = 0; + n = 0; + while ((pn = strchr(p, '\n')) != NULL) { + *pn = '\0'; + KUNIT_EXPECT_STREQ(test, p, &line[i]); + p = pn + 1; + n++; + i = prandom_u32_state(&rnd) % (sizeof(line) - 1); + } + KUNIT_EXPECT_EQ_MSG(test, n, num_lines, "Not enough lines."); +#else + kunit_skip(test, "only useful when debugfs is enabled"); +#endif +} + static struct kunit_case kunit_log_test_cases[] = { + KUNIT_CASE(kunit_log_init_frag_test), KUNIT_CASE(kunit_log_test), KUNIT_CASE(kunit_log_newline_test), + KUNIT_CASE(kunit_log_extend_test_1), + KUNIT_CASE(kunit_log_extend_test_2), {} };
Add test cases for the dynamically-extending log buffer. kunit_log_init_frag_test() tests that kunit_init_log_frag() correctly initializes new struct kunit_log_frag. kunit_log_extend_test_1() logs a series of numbered lines then tests that the resulting log contains all the lines. kunit_log_extend_test_2() logs a large number of lines of varying length to create many fragments, then tests that all lines are present. kunit_log_newline_test() has a new test to append a line that is exactly the length of the available space in the current fragment and check that the resulting log has a trailing '\n'. Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> --- lib/kunit/kunit-test.c | 182 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 174 insertions(+), 8 deletions(-)