mbox series

[0/4] kunit: more assertion reworking

Message ID 20221001002638.2881842-1-dlatypov@google.com
Headers show
Series kunit: more assertion reworking | expand

Message

Daniel Latypov Oct. 1, 2022, 12:26 a.m. UTC
RFC: https://lore.kernel.org/linux-kselftest/20220525154442.1438081-1-dlatypov@google.com/
Changes since then: tweak commit messages, reformatting to make
  checkpatch.pl happy. Nothing substantial.
Why send this out again now: the initial Rust patchset no longer
  contains the Kunit changes, so hopefully both series can go into 6.1
  and later we can coordinate the update the kunit.rs wrapper.

This is a follow up to these three series:
https://lore.kernel.org/all/20220113165931.451305-1-dlatypov@google.com/
https://lore.kernel.org/all/20220118223506.1701553-1-dlatypov@google.com/
https://lore.kernel.org/all/20220125210011.3817742-1-dlatypov@google.com/

The two goals of those series were
a) reduce the size of struct kunit_assert and friends.
   (struct kunit_assert went from 48 => 8 bytes on UML.)
b) simplify the internal code, mostly by deleting macros

This series goes further
a) sizeof(struct kunit_assert) = 0 now
b) e.g. we delete another class of macros (KUNIT_INIT_*_ASSERT_STRUCT)

Note: this does change the function signature of
kunit_do_failed_assertion, so we'd need to update the rust wrapper in
https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/kunit.rs,
but hopefully it's just a simple change, e.g. maybe just like:
@@ -38,9 +38,7 @@
             });
             static CONDITION: &'static $crate::str::CStr =
$crate::c_str!(stringify!($cond));
             static ASSERTION: UnaryAssert =
UnaryAssert($crate::bindings::kunit_unary_assert {
-                assert: $crate::bindings::kunit_assert {
-                    format: Some($crate::bindings::kunit_unary_assert_format),
-                },
+                assert: $crate::bindings::kunit_assert {},
                 condition: CONDITION.as_char_ptr(),
                 expected_true: true,
             });
@@ -67,6 +65,7 @@
                     core::ptr::addr_of!(LOCATION.0),
                     $crate::bindings::kunit_assert_type_KUNIT_ASSERTION,
                     core::ptr::addr_of!(ASSERTION.0.assert),
+                    Some($crate::bindings::kunit_unary_assert_format),
                     core::ptr::null(),
                 );
             }


Daniel Latypov (4):
  kunit: remove format func from struct kunit_assert, get it to 0 bytes
  kunit: rename base KUNIT_ASSERTION macro to _KUNIT_FAILED
  kunit: eliminate KUNIT_INIT_*_ASSERT_STRUCT macros
  kunit: declare kunit_assert structs as const

 include/kunit/assert.h |  74 ++----------------------
 include/kunit/test.h   | 127 +++++++++++++++++++++++------------------
 lib/kunit/test.c       |   7 ++-
 3 files changed, 80 insertions(+), 128 deletions(-)


base-commit: 511cce163b75bc3933fa3de769a82bb7e8663f2b

Comments

Miguel Ojeda Oct. 18, 2022, 11:20 p.m. UTC | #1
On Sat, Oct 1, 2022 at 8:00 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> While I have you here, any thoughts on how to coordinate the change?

My bad, I forgot to reply to this, sorry. I noticed it again when
merging 6.1-rc1 into our branch.

Normally I fix the issues when I merge back from Linus. Since we
intend to keep the CI green on every PR, I added the fix for this in
the merge itself.

In any case, to clarify, the `rust` branch in GitHub is just where we
placed a lot of things that we wanted to eventually submit (and some
that should not, e.g. the GitHub CI files). We will be minimizing the
differences w.r.t. upstream in that branch by preparing proper patches
from scratch and submitting them through `rust-next` and other trees,
and eventually remove it (or reusing the name for the fixes branch
since that is the name other trees use, but we will see).

Cheers,
Miguel
Daniel Latypov Oct. 18, 2022, 11:26 p.m. UTC | #2
On Tue, Oct 18, 2022 at 4:20 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Oct 1, 2022 at 8:00 PM Daniel Latypov <dlatypov@google.com> wrote:
> >
> > While I have you here, any thoughts on how to coordinate the change?
>
> My bad, I forgot to reply to this, sorry. I noticed it again when
> merging 6.1-rc1 into our branch.

No worries.
You've had a very busy couple of weeks, I imagine.

>
> Normally I fix the issues when I merge back from Linus. Since we
> intend to keep the CI green on every PR, I added the fix for this in
> the merge itself.

Thanks!


Daniel
Miguel Ojeda Oct. 18, 2022, 11:39 p.m. UTC | #3
On Wed, Oct 19, 2022 at 1:26 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> No worries.
> You've had a very busy couple of weeks, I imagine.

Just a bit :) Nevertheless, it was my intention to reply :(

I have linked this thread from the PR noting that you warned me about
the future conflict [1], thanks again!

[1] https://github.com/Rust-for-Linux/linux/pull/915#issuecomment-1283138279

Cheers,
Miguel