Quoting Brendan Higgins (2019-07-16 01:37:34)

On Tue, Jul 16, 2019 at 12:57 AM Brendan Higgins
[off-list ref] wrote:

quoted

On Mon, Jul 15, 2019 at 3:15 PM Stephen Boyd [off-list ref] wrote:

quoted

Quoting Brendan Higgins (2019-07-12 01:17:30)

quoted

diff --git a/include/kunit/kunit-stream.h b/include/kunit/kunit-stream.h
new file mode 100644
index 0000000000000..a7b53eabf6be4
--- /dev/null
+++ b/include/kunit/kunit-stream.h
+/**
+ * struct kunit_stream - a std::stream style string builder.
+ *
+ * A std::stream style string builder. Allows messages to be built up and
+ * printed all at once.
+ */
+struct kunit_stream {
+       /* private: internal use only. */
+       struct kunit *test;
+       const char *level;

Is the level changed? See my comment below, but I wonder if this whole
struct can go away and the wrappers can just operate on 'struct
string_stream' instead.

I was inclined to agree with you when I first read your comment, but
then I thought about the case that someone wants to add in a debug
message (of which I currently have none). I think under most
circumstances a user of kunit_stream would likely want to pick a
default verbosity that maybe I should provide, but may still want
different verbosity levels.

The main reason I want to keep the types separate, string_stream vs.
kunit_stream, is that they are intended to be used differently.
string_stream is just a generic string builder. If you are using that,
you are expecting to see someone building the string at some point and
then doing something interesting with it. kunit_stream really tells
you specifically that KUnit is putting together a message to
communicate something to a user of KUnit. It is really used in a very
specific way, and I wouldn't want to generalize its usage beyond how
it is currently used. I think in order to preserve the author's
intention it adds clarity to keep the types separate regardless of how
similar they might be in reality.

You may want to add some of these reasons to the commit text.

quoted

quoted

quoted

+
+       if (!string_stream_is_empty(stream->internal_stream)) {
+               kunit_err(stream->test,
+                         "End of test case reached with uncommitted stream entries\n");
+               kunit_stream_commit(stream);
+       }
+}
+

Nitpick: Drop this extra newline.

Oops, nice catch.

Not super important, but I don't want you to think that I am ignoring
you. I think you must have unintentionally deleted the last function
in this file, or maybe you are referring to something that I am just
not seeing, but I don't see the extra newline here.

No worries. Sorry for the noise.

quoted

property of the input, it may or may not be enough information on its
own for the expectation to fail, but we want to share the result of
the property check with the user regardless, BUT only if the
expectation as a whole fails.

Hence, we can have multiple `struct kunit_stream`s associated with a
`struct kunit` active at any given time.

Makes sense. I wasn't sure if there were more than one stream associated
with a test. Sounds like there are many to one so it can't just be a
member of the test. This could be documented somewhere so this question
doesn't come up again.