On Mon, Nov 14, 2022 at 11:45 PM David Gow davidgow@google.com wrote:
<snip>
+1 for the patch from me (modulo the "we" typo Sadiya mentioned).
I otherwise also prefer Daniel's original here (though I'd possibly merge it into one sentence, personally). Maybe: "In this example, as we need to be able to allocate an array in order to test the sort function, we use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()`` to abort the test if there's an allocation error." or "In this example, we need to allocate an array to test the sort function. We therefore use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()``, which will automatically abort the test if there's an allocation error."
But any of the above wordings are fine for me.
The note about ASSERT() working in any function is useful, though there are definitely some "gotcha"s caused by killing the kthread we'll need to resolve. (If there are any dangling references to things on the stack, for example.) Still, not an issue for this bit of documentation.
Reviewed-by: David Gow davidgow@google.com
(Once the "we" typo is fixed.)
v3 is here, PTAL https://lore.kernel.org/all/20221111182906.1377191-2-dlatypov@google.com/
Copying the relevant section here: +In this example, we need to be able to allocate an array to test the ``sort()`` +function. So we use ``KUNIT_ASSERT_NOT_ERR_OR_NULL()`` to abort the test if +there's an allocation error. + +.. note:: + In other test frameworks, ``ASSERT`` macros are often implemented by calling + ``return`` so they only work from the test function. In KUnit, we stop the + current kthread on failure, so you can call them from anywhere.
Daniel