On Fri, Jan 29, 2021 at 10:26 AM Daniel Latypov dlatypov@google.com wrote:
Currently, given something (fairly dystopian) like
KUNIT_EXPECT_EQ(test, 2 + 2, 5)
KUnit will prints a failure message like this.
Expected 2 + 2 == 5, but 2 + 2 == 4 5 == 5
With this patch, the output just becomes
Expected 2 + 2 == 5, but 2 + 2 == 4
This patch is slightly hacky, but it's quite common* to compare an expression to a literal integer value, so this can make KUnit less chatty in many cases. (This patch also fixes variants like KUNIT_EXPECT_GT, LE, et al.).
It also allocates an additional string briefly, but given this only happens on test failures, it doesn't seem too bad a tradeoff. Also, in most cases it'll realize the lengths are unequal and bail out before the allocation.
We could save the result of the formatted string to avoid wasting this extra work, but it felt cleaner to leave it as-is.
Edge case: for something silly and unrealistic like
KUNIT_EXPECT_EQ(test, 4, 5);
It'll generate this message with a trailing "but"
Expected 2 + 2 == 5, but
<next line of normal output>
I assume this is supposed to say "Expected 4 == 5" here. (I tested it to make sure, and that's what it did here.)
Personally, I'd ideally like to get rid of the ", but", or even add a "but 4 != 5" style second line. Particularly in case the next line in the output might be confused for the rest of a sentence.
That being said, this is a pretty silly edge case: I'd be worried if we ever saw that case in an actual submitted test. People might see it a bit while debugging, though: particularly if they're using KUNIT_EXPECT_EQ(test, 1, 2) as a way of forcing a test to fail. (I've done this while testing tooling, for instance.)
It didn't feel worth adding a check up-front to see if both sides are literals to handle this better.
*A quick grep suggests 100+ comparisons to an integer literal as the right hand side.
Signed-off-by: Daniel Latypov dlatypov@google.com
I tested this, and it works well: the results are definitely more human readable. I could see it making things slightly more complicated for people who wanted to automatically parse assertion errors, but no-one is doing that, and the extra complexity is pretty minimal anyway.
One thing which might be worth doing is expanding this to KUNIT_EXPECT_STREQ() and/or KUNIT_EXPECT_PTR_EQ(). These have slightly more complicated formatting (quotes, leading 0s, etc), though. Comparing pointer literals is pretty unlikely to show up, though, so I don't think it's as important. (I thought that maybe the KASAN shadow memory tests might use them, but a quick look didn't reveal any.)
For the record, this is what STREQ()/PTR_EQ()/ failures with literals look like: # example_simple_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:31 Expected "abc" == "abd", but "abc" == abc "abd" == abd # example_simple_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:33 Expected 0x124 == 0x1234, but 0x124 == 0000000000000124 0x1234 == 0000000000001234
Either way, though, this is:
Tested-by: David Gow davidgow@google.com
Cheers, -- David