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;
MIME-Version: 1.0
References: <20230614180837.630180-1-ojeda@kernel.org> <ZIps86MbJF/iGIzd@boqun-archlinux>
In-Reply-To: <ZIps86MbJF/iGIzd@boqun-archlinux>
From:   Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Date:   Thu, 15 Jun 2023 10:20:31 +0200
Message-ID: <CANiq72kHEddR-D17Ykr3xtU20rDJn517fqHRUX+-kWHjYqu9PA@mail.gmail.com>
Subject: Re: [PATCH 0/6] KUnit integration for Rust doctests
To:     Boqun Feng <boqun.feng@gmail.com>
Cc:     Miguel Ojeda <ojeda@kernel.org>, David Gow <davidgow@google.com>,
        Brendan Higgins <brendan.higgins@linux.dev>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Gary Guo <gary@garyguo.net>,
        =?UTF-8?Q?Bj=C3=B6rn_Roy_Baron?= <bjorn3_gh@protonmail.com>,
        Benno Lossin <benno.lossin@proton.me>,
        Alice Ryhl <aliceryhl@google.com>,
        Andreas Hindborg <nmi@metaspace.dk>,
        Philip Li <philip.li@intel.com>, kunit-dev@googlegroups.com,
        linux-kselftest@vger.kernel.org, rust-for-linux@vger.kernel.org,
        linux-kernel@vger.kernel.org, patches@lists.linux.dev
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Jun 15, 2023 at 3:44=E2=80=AFAM Boqun Feng <boqun.feng@gmail.com> w=
rote:
>
> Great work! I've played this for a while, and it's really useful ;-)

Thanks!

> The assertion warning only says line 35 but which file? Yes, the
> ".._sync_lock_spinlock_rs" name does provide the lead, however since we
> generate the test code, so we actually know the line # for each real
> test body, so I come up a way to give us the following:
>
>         [..] # rust_doctest_kernel_sync_lock_spinlock_rs_0: ASSERTION FAI=
LED at rust/kernel/sync/lock/spinlock.rs:61
>         [..] Expected e.c =3D=3D 11 to be true, but is false
>         [..] [FAILED] rust_doctest_kernel_sync_lock_spinlock_rs_0
>
> Thoughts?

Sounds good to me. However, David/Philip, is this OK or do you really
need/use the actual/compiled source file there? If you don't need it,
does it need to exist / be a real file at least? If the latter answer
is a "yes", which I guess it may be likely, then:

> +        let src_file =3D format!("rust/kernel/{}", file.replace("_rs", "=
.rs").replace("_", "/"));

This would not work for files with a `_` in their name, like
`locked_by.rs`. I guess we could still find the real filename based on
that information walking the dir, which is another hack I recall
considering at some point.

Otherwise, if "fake" filenames in the line above are OK for
David/Philip (I suspect they may want to open them for reporting?),
then I guess the `file` one may be good enough and eventually we
should get `rustdoc` to give us the proper metadata anyway.

Cheers,
Miguel