On Mon, Jan 10, 2022 at 2:32 PM 'Brendan Higgins' via KUnit Development <kunit-dev@xxxxxxxxxxxxxxxx> wrote: > > On Fri, Jan 7, 2022 at 8:23 PM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote: > > > > We call this function first thing for all the assertion `format()` > > functions. > > This is the part that prints the file and line number and assertion type > > (EXPECTATION, ASSERTION). > > > > Having it as part of the format functions lets us have the flexibility > > to not print that information (or print it differently) for new > > assertion types, but I think this we don't need that. > > nit: drop the "this". > > > And in the future, we'd like to consider factoring that data (file, > > line#, type) out of the kunit_assert struct and into a `static` > > variable, as Linus suggested [1], so we'd need to extract it anyways. > > > > [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ > > > > Signed-off-by: Daniel Latypov <dlatypov@xxxxxxxxxx> > > --- > > lib/kunit/assert.c | 6 ------ > > lib/kunit/test.c | 1 + > > 2 files changed, 1 insertion(+), 6 deletions(-) > > > > diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c > > index b972bda61c0c..4d9a1295efc7 100644 > > --- a/lib/kunit/assert.c > > +++ b/lib/kunit/assert.c > > @@ -40,7 +40,6 @@ EXPORT_SYMBOL_GPL(kunit_assert_print_msg); > > void kunit_fail_assert_format(const struct kunit_assert *assert, > > struct string_stream *stream) > > { > > - kunit_base_assert_format(assert, stream); > > string_stream_add(stream, "%pV", &assert->message); > > } > > EXPORT_SYMBOL_GPL(kunit_fail_assert_format); > > @@ -52,7 +51,6 @@ void kunit_unary_assert_format(const struct kunit_assert *assert, > > > > unary_assert = container_of(assert, struct kunit_unary_assert, assert); > > > > - kunit_base_assert_format(assert, stream); > > if (unary_assert->expected_true) > > string_stream_add(stream, > > KUNIT_SUBTEST_INDENT "Expected %s to be true, but is false\n", > > @@ -73,7 +71,6 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, > > ptr_assert = container_of(assert, struct kunit_ptr_not_err_assert, > > assert); > > > > - kunit_base_assert_format(assert, stream); > > if (!ptr_assert->value) { > > string_stream_add(stream, > > KUNIT_SUBTEST_INDENT "Expected %s is not null, but is\n", > > @@ -119,7 +116,6 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, > > binary_assert = container_of(assert, struct kunit_binary_assert, > > assert); > > > > - kunit_base_assert_format(assert, stream); > > string_stream_add(stream, > > KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", > > binary_assert->left_text, > > @@ -147,7 +143,6 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, > > binary_assert = container_of(assert, struct kunit_binary_ptr_assert, > > assert); > > > > - kunit_base_assert_format(assert, stream); > > string_stream_add(stream, > > KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", > > binary_assert->left_text, > > @@ -187,7 +182,6 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, > > binary_assert = container_of(assert, struct kunit_binary_str_assert, > > assert); > > > > - kunit_base_assert_format(assert, stream); > > string_stream_add(stream, > > KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", > > binary_assert->left_text, > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > > index 5ad671745483..735c1b67d843 100644 > > --- a/lib/kunit/test.c > > +++ b/lib/kunit/test.c > > @@ -255,6 +255,7 @@ static void kunit_fail(struct kunit *test, struct kunit_assert *assert) > > return; > > } > > > > + kunit_base_assert_format(assert, stream); > > I think my thinking in having this function called by the other assert > functions was to take advantage of inheritance. I was treating > kunit_base_assert_format as the parent method that other methods were > inheriting from, so I wanted to have them inherit some of the common > behavior by calling the original function. > > If you decide to make this change, I think it would be a good idea to > change the name of kunit_base_assert_format to not mislead to this > effect. The child patch renames it to kunit_assert_prologue(). I can rename it in this change if you prefer. I had just initially left it with the same name to keep this diff a bit smaller and more focused. But now you point it out, I think it would be cleaner to rename it here. > > > assert->format(assert, stream); > > > > kunit_print_string_stream(test, stream); > > -- > > 2.34.1.575.g55b058a8bb-goog > > > > -- > You received this message because you are subscribed to the Google Groups "KUnit Development" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/CAFd5g47r8aQBWPtt6ffHokqqN2sMi10p1Q5QA3xGVLTVDQh98Q%40mail.gmail.com.
