Message ID | 20221109003618.3784591-2-dlatypov@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp60285wru; Tue, 8 Nov 2022 16:41:44 -0800 (PST) X-Google-Smtp-Source: AMsMyM78hXNC3bIUBuaaayNA6J31SsAZ1c1SiAgeaURl49OzmDt+WpP18o87+MgT/VekbgC3bF/5 X-Received: by 2002:a17:907:9603:b0:742:9ed3:3af2 with SMTP id gb3-20020a170907960300b007429ed33af2mr54490430ejc.510.1667954504132; Tue, 08 Nov 2022 16:41:44 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1667954504; cv=none; d=google.com; s=arc-20160816; b=BQs472Z2g+jXI7J97V9ym52uXHydlRZpUFGQSrkRFyE9LXJ7I3IXVcYVn60Hxp33Ou 4XHUDnYJVOmQ/rOy3gDJky48+VolOg8mOH76Ds345NAp36CVCc2QPSzHNnT52jUQdLWT c9gUI8b4Ug8+09MA9hbKsUY0v6yvx/nSKWyXj1vS/5p3narbIH9c0x8Z65k1t+tVWnUt azW82umB/Cqo7PfsPuh13nh39WR8+bcXjm6OxQGBBninxcJ8yHOFQiwD251yVpqFl4tC 0x8CgEtdejqEVMwT2nUEGlyoauWaP8tVTLcH4KIcW7lUv+m9Yo4xLBt0rB82KTJBLcIn NxVQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:dkim-signature; bh=i16E6SEJ1XE3RexrHSv+UKrQKUgWn1ksElBhVAhWC74=; b=gRYQDeGfZVLw+e+7v7NYRBUfrIgs2Ze/hlfZUHfw3MBbqPTW/kHbZz1UIO3oArDGv4 BW1h9qg1Smy950ShFeTeJPJy+SF4eLvvRloR9fYq5qOzB9qMLMPNG3ntW4Xv70QOZBuo 3v5RPh+8DZUKgGSH+WGjJ2lljYOPfUP+w3ClKX40fCOvrXfVS6ryUBoL/0zrnpERI1X3 50nVY2ZVHVh5OWZmISnvPnzYfRzaFJ8oMo1dF22Iu7/2yl+Gboj4VUVW805gIv+pBed9 FGwOhbLO/fysshhyzzLTonMzjgBlI+pT2H1YOx1Dg9hFn0BW0iEcyHYLKqbH6CTS1h+Z icoQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=ozndvYKn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id x36-20020a50baa7000000b004627f27a119si13341708ede.318.2022.11.08.16.41.14; Tue, 08 Nov 2022 16:41:44 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=ozndvYKn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229759AbiKIAgd (ORCPT <rfc822;dexuan.linux@gmail.com> + 99 others); Tue, 8 Nov 2022 19:36:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37830 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229828AbiKIAg3 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 8 Nov 2022 19:36:29 -0500 Received: from mail-pj1-x1049.google.com (mail-pj1-x1049.google.com [IPv6:2607:f8b0:4864:20::1049]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 42BD863155 for <linux-kernel@vger.kernel.org>; Tue, 8 Nov 2022 16:36:28 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id pq17-20020a17090b3d9100b0020a4c65c3a9so6908329pjb.0 for <linux-kernel@vger.kernel.org>; Tue, 08 Nov 2022 16:36:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=i16E6SEJ1XE3RexrHSv+UKrQKUgWn1ksElBhVAhWC74=; b=ozndvYKnYfbi79PbuCYdzeMtofBcdWRIc10S48fA0ZiNj/LbcnyA78EeEyHbGlu9n6 7J9OUUeOSTjZdMHJ2Jj8jMxpE5/dNiSdsErzggvwEEW2nUjxnCa6+TDqTkoDJz9gOj99 WXadIIjgvnxIVJ/DBFiBgHJkkWlKthceyqDaVjg9MiPmZqLDPR9LHxZsOGuMW6BjHu/c 3eJf90rCsgm0AwbOvUrB59Y/Jmt+M8z3+xf8wujZCxWlDqOqjDioHBcPrSHS2jcF5xIG mcVvtHsOwHn5TgP74M+8eZTB81t5B4R9Xo69adh1fUPMDntg2alkPx9VTHycCQY1TfJc Zpeg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=i16E6SEJ1XE3RexrHSv+UKrQKUgWn1ksElBhVAhWC74=; b=fKCHd+Bh95eSTID1URhvU7ZcNoo0N5RXScFOCWIdCOEFV8UeTWOl8AvLNIV+Z6zvJq CPu/6hqZeGUacOV7y0922BQjPC9SSINE6V5K0ee+m8rYqqKYW3lTWO9uOPKFzpp9AnFS dLdiIxaZAsHEmm2Q1UrqXPideJUzG8hREjRd+9B+4dztXEOKvR8ywc/7Xbc0WoVxsttZ cB5oCvC8HxfYNbqRVv4fdSq5mvKW5KsH7aXs4/uAJYTMOc/Q+ejxFrKTpawzNcDSmH8o Jtsih8ReaT3ZWA4JRhSwCrFUuO8k+Q0ewzaXeR7bmcBh9O5534BPBJd4VHXD6cqiaDXN Kb/A== X-Gm-Message-State: ANoB5plqGTJPQfcn/xFZMsaC+ZXKYuQmkNNiLl5Jtr36CVvXyJn3DK+I IXve0HmFLTMstN0FLYdyr8aMtFcWDeaL9Q== X-Received: from dlatypov-spec.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:3f35]) (user=dlatypov job=sendgmr) by 2002:aa7:9ad7:0:b0:56e:d7f4:3c4a with SMTP id x23-20020aa79ad7000000b0056ed7f43c4amr657937pfp.76.1667954187657; Tue, 08 Nov 2022 16:36:27 -0800 (PST) Date: Tue, 8 Nov 2022 16:36:17 -0800 In-Reply-To: <20221109003618.3784591-1-dlatypov@google.com> Mime-Version: 1.0 References: <20221109003618.3784591-1-dlatypov@google.com> X-Mailer: git-send-email 2.38.1.431.g37b22c650d-goog Message-ID: <20221109003618.3784591-2-dlatypov@google.com> Subject: [PATCH v2 2/3] Documentation: KUnit: reword description of assertions From: Daniel Latypov <dlatypov@google.com> To: brendanhiggins@google.com, davidgow@google.com Cc: rmoar@google.com, linux-kernel@vger.kernel.org, kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org, skhan@linuxfoundation.org, Daniel Latypov <dlatypov@google.com> Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748977062123396707?= X-GMAIL-MSGID: =?utf-8?q?1748977062123396707?= |
Series |
[v2,1/3] Documentation: KUnit: make usage.rst a superset of tips.rst, remove duplication
|
|
Commit Message
Daniel Latypov
Nov. 9, 2022, 12:36 a.m. UTC
The existing wording implies that kunit_kmalloc_array() is "the method
under test". We're actually testing the sort() function in that example.
This is because the example was changed in commit 953574390634
("Documentation: KUnit: Rework writing page to focus on writing tests"),
but the wording was not.
Also add a `note` telling people they can use the KUNIT_ASSERT_EQ()
macros from any function. Some users might be coming from a framework
like gUnit where that'll compile but silently do the wrong thing.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
Documentation/dev-tools/kunit/usage.rst | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
Comments
On Wed, Nov 9, 2022 at 6:06 AM 'Daniel Latypov' via KUnit Development <kunit-dev@googlegroups.com> wrote: > > The existing wording implies that kunit_kmalloc_array() is "the method > under test". We're actually testing the sort() function in that example. > This is because the example was changed in commit 953574390634 > ("Documentation: KUnit: Rework writing page to focus on writing tests"), > but the wording was not. > > Also add a `note` telling people they can use the KUNIT_ASSERT_EQ() > macros from any function. Some users might be coming from a framework > like gUnit where that'll compile but silently do the wrong thing. > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > --- Thank you, Daniel. This looks fine to me except for a small typo in this line "to abort the test if we there's an allocation error". Also, I have reworded that paragraph a bit as below. Please feel free to ignore, if you do not agree: In this example, to test the ``sort()`` function, we must be able to allocate an array. If there is an allocation error, the test is terminated using the function ``KUNIT ASSERT NOT ERR OR NULL()``. Reviewed-by: Sadiya Kazi <sadiyakazi@google.com> Best Regards, Sadiya > Documentation/dev-tools/kunit/usage.rst | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst > index b0a6c3bc0eeb..8060114e3aa6 100644 > --- a/Documentation/dev-tools/kunit/usage.rst > +++ b/Documentation/dev-tools/kunit/usage.rst > @@ -112,11 +112,14 @@ terminates the test case if the condition is not satisfied. For example: > KUNIT_EXPECT_LE(test, a[i], a[i + 1]); > } > > -In this example, the method under test should return pointer to a value. If the > -pointer returns null or an errno, we want to stop the test since the following > -expectation could crash the test case. `ASSERT_NOT_ERR_OR_NULL(...)` allows us > -to bail out of the test case if the appropriate conditions are not satisfied to > -complete the test. > +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 > +we 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. > > Customizing error messages > -------------------------- > -- > 2.38.1.431.g37b22c650d-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@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20221109003618.3784591-2-dlatypov%40google.com.
On Wed, Nov 9, 2022 at 9:07 PM Sadiya Kazi <sadiyakazi@google.com> wrote: > > On Wed, Nov 9, 2022 at 6:06 AM 'Daniel Latypov' via KUnit Development > <kunit-dev@googlegroups.com> wrote: > > > > The existing wording implies that kunit_kmalloc_array() is "the method > > under test". We're actually testing the sort() function in that example. > > This is because the example was changed in commit 953574390634 > > ("Documentation: KUnit: Rework writing page to focus on writing tests"), > > but the wording was not. > > > > Also add a `note` telling people they can use the KUNIT_ASSERT_EQ() > > macros from any function. Some users might be coming from a framework > > like gUnit where that'll compile but silently do the wrong thing. > > > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > > --- > > Thank you, Daniel. This looks fine to me except for a small typo in > this line "to abort > the test if we there's an allocation error". Also, I have reworded > that paragraph a bit > as below. Please feel free to ignore, if you do not agree: > > In this example, to test the ``sort()`` function, we must be able to > allocate an array. > If there is an allocation error, the test is terminated using the function > ``KUNIT ASSERT NOT ERR OR NULL()``. Thanks for catching that. Hmm, I slightly prefer the current structure since I like having the <thing> being described near the start of the sentence as opposed to the very end. I'll wait a bit before sending a v3 to give time for anyone else to chime in, if they want. Snipping the email to the block in question: > > +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 > > +we there's an allocation error.
On Fri, Nov 11, 2022 at 12:04 AM Daniel Latypov <dlatypov@google.com> wrote: > > On Wed, Nov 9, 2022 at 9:07 PM Sadiya Kazi <sadiyakazi@google.com> wrote: > > > > On Wed, Nov 9, 2022 at 6:06 AM 'Daniel Latypov' via KUnit Development > > <kunit-dev@googlegroups.com> wrote: > > > > > > The existing wording implies that kunit_kmalloc_array() is "the method > > > under test". We're actually testing the sort() function in that example. > > > This is because the example was changed in commit 953574390634 > > > ("Documentation: KUnit: Rework writing page to focus on writing tests"), > > > but the wording was not. > > > > > > Also add a `note` telling people they can use the KUNIT_ASSERT_EQ() > > > macros from any function. Some users might be coming from a framework > > > like gUnit where that'll compile but silently do the wrong thing. > > > > > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > > > --- > > > > Thank you, Daniel. This looks fine to me except for a small typo in > > this line "to abort > > the test if we there's an allocation error". Also, I have reworded > > that paragraph a bit > > as below. Please feel free to ignore, if you do not agree: > > > > In this example, to test the ``sort()`` function, we must be able to > > allocate an array. > > If there is an allocation error, the test is terminated using the function > > ``KUNIT ASSERT NOT ERR OR NULL()``. > > Thanks for catching that. > > Hmm, I slightly prefer the current structure since I like having the > <thing> being described near the start of the sentence as opposed to > the very end. > I'll wait a bit before sending a v3 to give time for anyone else to > chime in, if they want. > > Snipping the email to the block in question: > > > > +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 > > > +we there's an allocation error. +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.) Cheers, -- David
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
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst index b0a6c3bc0eeb..8060114e3aa6 100644 --- a/Documentation/dev-tools/kunit/usage.rst +++ b/Documentation/dev-tools/kunit/usage.rst @@ -112,11 +112,14 @@ terminates the test case if the condition is not satisfied. For example: KUNIT_EXPECT_LE(test, a[i], a[i + 1]); } -In this example, the method under test should return pointer to a value. If the -pointer returns null or an errno, we want to stop the test since the following -expectation could crash the test case. `ASSERT_NOT_ERR_OR_NULL(...)` allows us -to bail out of the test case if the appropriate conditions are not satisfied to -complete the test. +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 +we 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. Customizing error messages --------------------------