Re: [PATCH v4 03/10] kunit: string-stream: Add cases for adding empty strings to a string_stream

[David Gow]

On Mon, 14 Aug 2023 at 21:23, Richard Fitzgerald
<rf@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> Adds string_stream_append_empty_string_test() to test that adding an
> empty string to a string_stream doesn't create a new empty fragment.
>
> Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> ---

Looks good. One minor note below (not worth resending the series to
fix by itself, though).

If you wanted to combine this with the previous patch, that'd be fine
too. (I don't mind either way.)

Reviewed-by: David Gow <davidgow@xxxxxxxxxx>

Cheers,
-- David

>  lib/kunit/string-stream-test.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/lib/kunit/string-stream-test.c b/lib/kunit/string-stream-test.c
> index 1d46d5f06d2a..efe13e3322b5 100644
> --- a/lib/kunit/string-stream-test.c
> +++ b/lib/kunit/string-stream-test.c
> @@ -206,11 +206,32 @@ static void string_stream_append_test(struct kunit *test)
>         KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream_1), stream_2_content);
>  }
>
> +/* Adding an empty string should not create a fragment. */
> +static void string_stream_append_empty_string_test(struct kunit *test)
> +{
> +       struct string_stream *stream;
> +
> +       stream = alloc_string_stream(test, GFP_KERNEL);
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, stream);
> +
> +       /* Formatted empty string */
> +       string_stream_add(stream, "%s", "");
> +       KUNIT_EXPECT_TRUE(test, string_stream_is_empty(stream));
> +       KUNIT_EXPECT_TRUE(test, list_empty(&stream->fragments));
> +
> +       /* Adding an empty string to a non-empty stream */
> +       string_stream_add(stream, "Add this line");
> +       string_stream_add(stream, "%s", "");
> +       KUNIT_EXPECT_EQ(test, list_count_nodes(&stream->fragments), 1);

While this is fine, I do wonder whether the more future-proof thing to
do would be to check that the number of fragments hasn't changed after
adding the empty string, rather than that it's definitely 1.

(In practice, even with a fancier allocation strategy, I can't imagine
us splitting "Add this line" into multiple fragments, but I think it's
slightly clearer that what we're testing is that the empty string
doesn't increase it.)


> +       KUNIT_EXPECT_STREQ(test, string_stream_get_string(stream), "Add this line");
> +}
> +
>  static struct kunit_case string_stream_test_cases[] = {
>         KUNIT_CASE(string_stream_init_test),
>         KUNIT_CASE(string_stream_line_add_test),
>         KUNIT_CASE(string_stream_variable_length_line_test),
>         KUNIT_CASE(string_stream_append_test),
> +       KUNIT_CASE(string_stream_append_empty_string_test),
>         {}
>  };
>
> --
> 2.30.2
>

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature