On Tue, Jul 16, 2019 at 8:30 AM Stephen Boyd sboyd@kernel.org wrote:
Quoting Brendan Higgins (2019-07-16 01:37:34)
On Tue, Jul 16, 2019 at 12:57 AM Brendan Higgins brendanhiggins@google.com wrote:
On Mon, Jul 15, 2019 at 3:15 PM Stephen Boyd sboyd@kernel.org wrote:
Quoting Brendan Higgins (2019-07-12 01:17:30)
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.
Will do.
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.
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.
Sounds good. Will do.
Thanks!