From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 85907C433FE for ; Tue, 10 May 2022 04:45:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236550AbiEJEtL (ORCPT ); Tue, 10 May 2022 00:49:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59220 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236526AbiEJEtI (ORCPT ); Tue, 10 May 2022 00:49:08 -0400 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9809A293B69 for ; Mon, 9 May 2022 21:45:10 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id v12so22010259wrv.10 for ; Mon, 09 May 2022 21:45:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9s1dagxJlyBeoN8vXdOpsPpdS8AD1RWVJWE0n8ehmas=; b=OrctmfTVDCxyYdwcxCtO+PuFgHBDMDC7Bjk3TGUWyuOKGBhKLCCiUkxtYUOPWSujka /OoqstAYhrGXcYieyOQKBFbwQ9tdHrrlfAgfjS6rNqvPEA5JdQz3D4E5NS7VzL5WOf+E ppcl6vbGTOThxdSbDLD0FF3VNmQjZYs0YaRKW2LrfcKbI7iiYsOvdodP6Nf3qoZXPgQF Um1ef/9fJDTkVoHV63EAahHnEkdwxgxf/Hc8y1wACrWcQPfhb2xVBIshPVinpye5CGSN DoHok7Lf+NgFTiQMqtYLLj/aSoKi0RaOM3+EWbBDgDeG8Kjhw/dRvI6d3AEZbIIqfLAr UHxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9s1dagxJlyBeoN8vXdOpsPpdS8AD1RWVJWE0n8ehmas=; b=f3C6rX6KlEsokrI8jZmr7ztNcmjc/yRrrzWr1GTl7CJPH683zgeoXwEQdizoqMnqWD kFZQWiJ2nRkqFBJX6BcDWoEqnGXFFS85ZUE5O9ZbV9ILIYos3GPdEtdfzA4ezOq7653F I8ahZEzVngmtnsZqICt7o7335MsLmCO10oKeOwAkxQ8hvjFHFU0ABtAcTysl2/HPaEJN TuTFFIn8V+C+F25YzKHFq7CH0vEPhFWX7G1hGHnukRLRC2VMvVkouJRgY7iwWTGek+j/ 9zNWBuKjnZBq99YM8b9jAWfRMVpkjm6ESYzGevoj+5R8+u+54wfp4wn7Ieq6LDgxQQtP yckQ== X-Gm-Message-State: AOAM533ODA2hBVu+DgHCIpxe1vOHU0zoK0f9Yuy+PW978V5JtemcSb8Q QHdIYBr1zFt65xLRTae8uhhdxbgd0JrWYDq7yXi5+w== X-Google-Smtp-Source: ABdhPJzfNwaPy5xjCTvC86K51nEH3yemeJZ1erUUa4xvtvCZqXN7MOjD5hnH0ntxxTlSOXapkot4g5ZMqXe6px9AH2Q= X-Received: by 2002:a5d:4806:0:b0:20a:da03:711b with SMTP id l6-20020a5d4806000000b0020ada03711bmr16582569wrq.395.1652157908998; Mon, 09 May 2022 21:45:08 -0700 (PDT) MIME-Version: 1.0 References: <20220507052451.12890-1-ojeda@kernel.org> In-Reply-To: From: David Gow Date: Tue, 10 May 2022 12:44:57 +0800 Message-ID: Subject: Re: [PATCH v6 00/23] Rust support To: Miguel Ojeda Cc: Miguel Ojeda , Linus Torvalds , Greg Kroah-Hartman , rust-for-linux , Linux Kernel Mailing List , Jarkko Sakkinen , KUnit Development , Linux ARM , "open list:DOCUMENTATION" , "open list:GPIO SUBSYSTEM" , Linux Kbuild mailing list , "open list:KERNEL SELFTEST FRAMEWORK" , linux-perf-users@vger.kernel.org, linuxppc-dev , linux-riscv , live-patching@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, May 7, 2022 at 11:03 PM Miguel Ojeda wrote: > > Hi David, > > On Sat, May 7, 2022 at 11:29 AM David Gow wrote: > > > > It's great to see some KUnit support here! > > Thanks! > > > It's also possible to run these tests using the KUnit wrapper tool with: > > $ ./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_RUST=y > > --make_options LLVM=1 --arch x86_64 'rust_kernel_doctests' > > > > That also nicely formats the results. > > Indeed! > > [16:55:52] ============ rust_kernel_doctests (70 subtests) ============ > [16:55:52] [PASSED] rust_kernel_doctest_build_assert_rs_12_0 > [16:55:52] [PASSED] rust_kernel_doctest_build_assert_rs_55_0 > ... > [16:55:52] [PASSED] rust_kernel_doctest_types_rs_445_0 > [16:55:52] [PASSED] rust_kernel_doctest_types_rs_509_0 > [16:55:52] ============== [PASSED] rust_kernel_doctests =============== > [16:55:52] ============================================================ > [16:55:52] Testing complete. Passed: 70, Failed: 0, Crashed: 0, > Skipped: 0, Errors: 0 > I've just sent out a pull request to get this working under UML as well, which would simplify running these further: https://github.com/Rust-for-Linux/linux/pull/766 > > That all being said, I can't say I'm thrilled with the test names > > here: none of them are particularly descriptive, and they'll probably > > not be static (which would make it difficult to track results / > > regressions / etc between kernel versions). Neither of those are > > Yeah, the names are not great and would change from time to time > across kernel versions. > > We could ask example writers to give each example a name, but that > would make them fairly less convenient. For instance, sometimes they > may be very small snippets interleaved with docs' prose (where giving > a name may feel a bit of a burden, and people may end writing > `foo_example1`, `foo_example2` etc. for each small "step" of an > explanation). In other cases they may be very long, testing a wide API > surface (e.g. when describing a module or type), where it is also hard > to give non-generic names like `rbtree_doctest`. In those kind of > cases, I think we would end up with not much better names than > automatically generated ones. > > The other aspect is that, given they are part of the documentation, > the prose or how things are explained/split may change, thus the > doctests as well. For instance, one may need to split a very long > `rbtree_doctest` in pieces, and then the name would need to change > anyway. > > So I think we should avoid asking documentation writers to add a > manual name, even if that means a bit ugly test names. Also this way > they are consistently named. What do you think? Yeah, these are all fair points: particularly for small doctests. Maybe having an optional name, which more significant tests could use to override the file:line names? That could be useful for a few of the larger, more often referenced tests. > One idea could be giving them a name based on the hash of the content > and avoiding the line number, so that there is a higher chance for the > name to stay the same even when the file gets modified for other > reasons. Ugh: it's a bit ugly either way. I suspect that file:line is still probably better, if only because we need some way of looking up the test in the code if it fails. I'd hate for people to be randomly hashing bits of just to find out what test is failing. > > necessarily deal breakers, though it might make sense to hide them > > behind a kernel option (like all other KUnit tests) so that they can > > easily be excluded where they would otherwise clutter up results. (And > > Currently they are under `CONFIG_RUST_KERNEL_KUNIT_TEST` -- or do you > mean something else? > Oops: I missed that (one of the issues with testing this on a different machine which had a rust toolchain). Looks good to me. > > if there's a way to properly name them, or maybe even split them into > > per-file or per-module suites, that would make them a bit easier to > > deal.) Additionally, there are some plans to taint the kernel[1] when > > Yeah, splitting them further is definitely possible. We are also > likely splitting the `kernel` crate into several, which would also > make the suites smaller etc. so perhaps further splits may not be > needed. Ah: I didn't realise the plan was always to have crate-specific suites, and possibly to split things up. The KTAP output specification does actually support arbitrary nesting (though KUnit itself doesn't at the moment), which would potentially be an option if (e.g.) providing the complete module nesting made sense. I'm not convinced that'd make things easier to read, though. > > Regardless, this is very neat, and I'm looking forward to taking a > > closer look at it. > > Thanks again for taking a look and playing with it, I am glad you > liked it! (even if it is just a first approximation, and only supports > the `kernel` crate, etc.). > > Cheers, > Miguel Thanks, -- David