Message ID | 20230113220718.2901010-1-dlatypov@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp3569wrn; Fri, 13 Jan 2023 14:11:17 -0800 (PST) X-Google-Smtp-Source: AMrXdXvY16ErplAa1yVY/UAdFF+6JxhITT7MWa9YJCmFNXBNyEQQPSOI95r1lYsbAhzGKSYT5q7e X-Received: by 2002:a05:6a20:8f0c:b0:af:7ed6:9858 with SMTP id b12-20020a056a208f0c00b000af7ed69858mr122672211pzk.31.1673647877027; Fri, 13 Jan 2023 14:11:17 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1673647877; cv=none; d=google.com; s=arc-20160816; b=H5e7ku6xcK26es+/pOQ6O7tvwpX7/2hljOdrkVEgc+FjngPzWdnOB3oo1GMZOk2QKh CEG32r6Dyy5INgemAM0UGnj+czHWbSnJuwQHeja9Xt9Ly9PRlJDKBfJ1YeDYNlSkeV63 xsRU5Vo//NESpa9ebXNBuwhlsv7C5Jbz7J5cPw1cJHmqxCgAXEPhFL6L8t+kZgBhWpCD pDPb+HGMbz2NPMn95g9APcIuOiz2SE/h8f4yiDuXGvrt3sXpFnsraaReWqb0j0L1jzlV KppOm6YeReq2ts0H+bl9Df9VuEQKT+HixfjGe29HozS1z22RMXZHujjt/CBaZQ8KGh4c 0W9A== 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:mime-version:date :dkim-signature; bh=tvJbCtGPjaySsYlcQ9SGihqPWmk76m7GVR1zZaETHos=; b=BzdQYLBZVTEgYuE7jXvUeqX7A56OFOKlCA6S5mqGsWjRjCRytq4NufIRpsXhyF4tjQ 1+fXL+AnG7qva06nQg+FWSBhyV2EnAJ3lJ6brkFlop8l+jnXm2AF3ioFax85jPQ+Pr5x nPIZakB+a+Br4/HIMPXuuWpXrJLc7gHWx+ZWCQGNJT5EWzZJY4P2M+2foKiv+xyU/7bo cjeM6Z+bTaeHRxfSKoMvzNFRuHyzdsYZ7f7ObBrJ/dxHPGAK6o34Wd3p/7XL9cVIaZgn 7HCAjspLsQmTR0KcGAt9rqAHoLn725wEKPrJ/PfOs0xjCm3sG71GRAs43lqml3GmIATs +Zbg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=VGW1X3J7; 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 v29-20020a63b95d000000b004b980d584eesi8405675pgo.49.2023.01.13.14.11.03; Fri, 13 Jan 2023 14:11:17 -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=VGW1X3J7; 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 S230150AbjAMWHj (ORCPT <rfc822;andrewvogler123@gmail.com> + 99 others); Fri, 13 Jan 2023 17:07:39 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56502 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230144AbjAMWHg (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 13 Jan 2023 17:07:36 -0500 Received: from mail-pl1-x649.google.com (mail-pl1-x649.google.com [IPv6:2607:f8b0:4864:20::649]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A242676AF5 for <linux-kernel@vger.kernel.org>; Fri, 13 Jan 2023 14:07:32 -0800 (PST) Received: by mail-pl1-x649.google.com with SMTP id s14-20020a17090302ce00b00192d831a155so15588954plk.11 for <linux-kernel@vger.kernel.org>; Fri, 13 Jan 2023 14:07:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=tvJbCtGPjaySsYlcQ9SGihqPWmk76m7GVR1zZaETHos=; b=VGW1X3J7LEjFFCzmLMbT7R3kSUQNmgHK3Vr7yRvDuNGKnUl8xPF2xLfnOhVLgJ547m UUur7tbTSeBSgbQmOWgks+tJOH+BFqZtRp68C3wazyRRWmeNy2rvTigm2j3CG++QQ7P0 09OBHs6IcAWO/B3kzf8czBdZz2UBeL25LTe5hMbJnyymh8PpxrpA+HdAWV1qrOEZXR1/ fRwY38oJDw1KDmYTMsMmuS43h1diht1VEkmetY6hlyv/CjcAOQ2VDpF/bqiszn/YZVJ7 S38x6EJ6n5UBgUZ2PjXT1H8Z/smbNH44RMmLwwnBYNLuSl1zkkgBx1F1lYzeZP7tsLFR 039A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=tvJbCtGPjaySsYlcQ9SGihqPWmk76m7GVR1zZaETHos=; b=YjcOvbTHKlf5w/rRWUTE3LF9DyuQrI8LXP41ciddF7pC61o+oDjiA6DU3u7IkC8D+w 3UA6Oz93awBRjNddVZBcD0pOn6nLUHIuHuGtpodiX1dXQ7gUm41t4R/gConuSCTSjoD4 lS2sUncbZNJpyzn7Z4N3toMKPuUlNT4WW0PYgewjAMjxPR54Yooxs0AYNFege4myCVZD mDNI9o1TIjINQbYB8r/HJSFLf9PHdBAJ/TQInx7VjExciIzdnoQ3vMRzV0SxENo5h8lE 5MbdFCXgW8dAVIJqI9o8454rB0WSvTkfOjuBgiJq2WHAR2H+f1pWYBX7XCUAIk3AFrMN UEYw== X-Gm-Message-State: AFqh2kog7zGJjGZqZoPaaY0wWY9l4lHTiZpKiSFrUP0gqyjOxC9WI3jM PLDP97DtDxtu0oaKqhYNFhXAZxQ8zU+9zg== X-Received: from dlatypov-spec.c.googlers.com ([fda3:e722:ac3:cc00:24:72f4:c0a8:3f35]) (user=dlatypov job=sendgmr) by 2002:a17:90b:1109:b0:226:f85c:e099 with SMTP id gi9-20020a17090b110900b00226f85ce099mr2664860pjb.198.1673647651998; Fri, 13 Jan 2023 14:07:31 -0800 (PST) Date: Fri, 13 Jan 2023 14:07:18 -0800 Mime-Version: 1.0 X-Mailer: git-send-email 2.39.0.314.g84b9a713c41-goog Message-ID: <20230113220718.2901010-1-dlatypov@google.com> Subject: [PATCH] kunit: kunit_skip() should not overwrite KUNIT_FAIL() 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, 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?1754946996021073830?= X-GMAIL-MSGID: =?utf-8?q?1754946996021073830?= |
Series |
kunit: kunit_skip() should not overwrite KUNIT_FAIL()
|
|
Commit Message
Daniel Latypov
Jan. 13, 2023, 10:07 p.m. UTC
Currently, kunit_skip() and kunit_mark_skipped() will overwrite the
current test's status even if it was already marked FAILED.
E.g. a test that just contains this
KUNIT_FAIL(test, "FAIL REASON");
kunit_skip(test, "SKIP REASON");
will be marked "SKIPPED" in the end.
Now, tests like the above don't and shouldn't exist.
But what happens if non-test code (e.g. KASAN) calls kunit_fail_current_test()?
E.g. if we have
if (do_some_invalid_memory_accesses())
kunit_skip(");
then the KASAN failures will get masked!
This patch: make it so kunit_mark_skipped() does not modify the status
if it's already set to something (either already to SKIPPED or FAILURE).
Before this change, the KTAP output would look like
# example_simple_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:23
FAIL REASON
ok 1 example_simple_test # SKIP SKIP REASON
After this change:
# example_simple_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:23
FAIL REASON
# example_simple_test: status already changed, not marking skipped: SKIP REASON
not ok 1 example_simple_test
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
include/kunit/test.h | 7 +++++++
1 file changed, 7 insertions(+)
base-commit: 7dd4b804e08041ff56c88bdd8da742d14b17ed25
Comments
On Sat, 14 Jan 2023 at 06:07, 'Daniel Latypov' via KUnit Development <kunit-dev@googlegroups.com> wrote: > > Currently, kunit_skip() and kunit_mark_skipped() will overwrite the > current test's status even if it was already marked FAILED. > > E.g. a test that just contains this > KUNIT_FAIL(test, "FAIL REASON"); > kunit_skip(test, "SKIP REASON"); > will be marked "SKIPPED" in the end. > > Now, tests like the above don't and shouldn't exist. > But what happens if non-test code (e.g. KASAN) calls kunit_fail_current_test()? > > E.g. if we have > if (do_some_invalid_memory_accesses()) > kunit_skip("); > then the KASAN failures will get masked! > > This patch: make it so kunit_mark_skipped() does not modify the status > if it's already set to something (either already to SKIPPED or FAILURE). > > Before this change, the KTAP output would look like > # example_simple_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:23 > FAIL REASON > ok 1 example_simple_test # SKIP SKIP REASON > > After this change: > # example_simple_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:23 > FAIL REASON > # example_simple_test: status already changed, not marking skipped: SKIP REASON > not ok 1 example_simple_test > > Signed-off-by: Daniel Latypov <dlatypov@google.com> > --- Thanks very much: this makes much more sense than the old behaviour. My only suggestion is that we add a test to verify this behaviour to the kunit_status suite, such as: diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index 4df0335d0d06..fa114785b01e 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -510,9 +510,33 @@ static void kunit_status_mark_skipped_test(struct kunit *test) KUNIT_EXPECT_STREQ(test, fake.status_comment, "Accepts format string: YES"); } +static void kunit_status_skip_after_fail_test(struct kunit *test) +{ + struct kunit fake; + + kunit_init_test(&fake, "fake test", NULL); + + /* Test starts off SUCCESS. */ + KUNIT_EXPECT_EQ(test, fake.status, KUNIT_SUCCESS); + + /* Fail the test. */ + kunit_set_failure(&fake); + KUNIT_EXPECT_EQ(test, fake.status, KUNIT_FAILURE); + + /* Now mark it as skipped. */ + kunit_mark_skipped(&fake, "Skip message"); + + /* The test has still failed. */ + KUNIT_EXPECT_EQ(test, fake.status, KUNIT_FAILURE); + + /* We shouldn't use the skip reason as a status comment. */ + KUNIT_EXPECT_STREQ(test, fake.status_comment, ""); +} + static struct kunit_case kunit_status_test_cases[] = { KUNIT_CASE(kunit_status_set_failure_test), KUNIT_CASE(kunit_status_mark_skipped_test), + KUNIT_CASE(kunit_status_skip_after_fail_test), {} }; -- Otherwise, this looks great! Reviewed-by: David Gow <davidgow@google.com> Cheers, -- David > include/kunit/test.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 87ea90576b50..39936463dde5 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -386,11 +386,18 @@ void __printf(2, 3) kunit_log_append(char *log, const char *fmt, ...); > * > * Marks the test as skipped. @fmt is given output as the test status > * comment, typically the reason the test was skipped. > + * This has no effect if the test has already been marked skipped or failed. > * > * Test execution continues after kunit_mark_skipped() is called. > */ > #define kunit_mark_skipped(test_or_suite, fmt, ...) \ > do { \ > + if (READ_ONCE((test_or_suite)->status) != KUNIT_SUCCESS) {\ > + kunit_warn(test_or_suite, "status already " \ > + "changed, not marking skipped: " fmt,\ > + ##__VA_ARGS__); \ > + break; \ > + } \ > WRITE_ONCE((test_or_suite)->status, KUNIT_SKIPPED); \ > scnprintf((test_or_suite)->status_comment, \ > KUNIT_STATUS_COMMENT_SIZE, \ > > base-commit: 7dd4b804e08041ff56c88bdd8da742d14b17ed25 > -- > 2.39.0.314.g84b9a713c41-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/20230113220718.2901010-1-dlatypov%40google.com.
diff --git a/include/kunit/test.h b/include/kunit/test.h index 87ea90576b50..39936463dde5 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -386,11 +386,18 @@ void __printf(2, 3) kunit_log_append(char *log, const char *fmt, ...); * * Marks the test as skipped. @fmt is given output as the test status * comment, typically the reason the test was skipped. + * This has no effect if the test has already been marked skipped or failed. * * Test execution continues after kunit_mark_skipped() is called. */ #define kunit_mark_skipped(test_or_suite, fmt, ...) \ do { \ + if (READ_ONCE((test_or_suite)->status) != KUNIT_SUCCESS) {\ + kunit_warn(test_or_suite, "status already " \ + "changed, not marking skipped: " fmt,\ + ##__VA_ARGS__); \ + break; \ + } \ WRITE_ONCE((test_or_suite)->status, KUNIT_SKIPPED); \ scnprintf((test_or_suite)->status_comment, \ KUNIT_STATUS_COMMENT_SIZE, \