Re: [RFC v2 00/12] kunit: introduce class mocking support.

From: Brendan Higgins <brendanhiggins@google.com>
To: Daniel Latypov <dlatypov@google.com>,
	Joel Stanley <joel@jms.id.au>,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Alan Maguire <alan.maguire@oracle.com>,
	David Gow <davidgow@google.com>,
	Kees Cook <keescook@chromium.org>,
	KUnit Development <kunit-dev@googlegroups.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:KERNEL SELFTEST FRAMEWORK" 
	<linux-kselftest@vger.kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [RFC v2 00/12] kunit: introduce class mocking support.
Date: Mon, 2 Nov 2020 10:00:28 -0800	[thread overview]
Message-ID: <CAFd5g45kdj8mTqStN6hreJEaa47z3+2ZXQDnzZ=Nz6vA50x=Gg@mail.gmail.com> (raw)
In-Reply-To: <20201012222050.999431-1-dlatypov@google.com>

+Joel Stanley +Daniel Vetter

If I remember correctly, both of you said you were interested in
mocking on KUnit. This RFC only has some of the mocking features that
I mentioned previously, but I would still like to get your thoughts.

On Mon, Oct 12, 2020 at 3:21 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> # Background
> KUnit currently lacks any first-class support for mocking.
> For an overview and discussion on the pros and cons, see
> https://martinfowler.com/articles/mocksArentStubs.html
>
> This patch set introduces the basic machinery needed for mocking:
> setting and validating expectations, setting default actions, etc.
>
> Using that basic infrastructure, we add macros for "class mocking", as
> it's probably the easiest type of mocking to start with.
>
> ## Class mocking
>
> By "class mocking", we're referring mocking out function pointers stored
> in structs like:
>   struct sender {
>         int (*send)(struct sender *sender, int data);
>   };
> or in ops structs
>   struct sender {
>         struct send_ops *ops; // contains `send`
>   };
>
> After the necessary DEFINE_* macros, we can then write code like
>   struct MOCK(sender) mock_sender = CONSTRUCT_MOCK(sender, test);
>
>   /* Fake an error for a specific input. */
>   handle = KUNIT_EXPECT_CALL(send(<omitted>, kunit_int_eq(42)));
>   handle->action = kunit_int_return(test, -EINVAL);
>
>   /* Pass the mocked object to some code under test. */
>   KUNIT_EXPECT_EQ(test, -EINVAL, send_message(...));
>
> I.e. the goal is to make it easier to test
> 1) with less dependencies (we don't need to setup a real `sender`)
> 2) unusual/error conditions more easily.
>
> In the future, we hope to build upon this to support mocking in more
> contexts, e.g. standalone funcs, etc.
>
> # TODOs
>
> ## Naming
> This introduces a number of new macros for dealing with mocks,
> e.g:
>   DEFINE_STRUCT_CLASS_MOCK(METHOD(foo), CLASS(example),
>                            RETURNS(int),
>                            PARAMS(struct example *, int));
>   ...
>   KUNIT_EXPECT_CALL(foo(mock_get_ctrl(mock_example), ...);
> For consistency, we could prefix everything with KUNIT, e.g.
> `KUNIT_DEFINE_STRUCT_CLASS_MOCK` and `kunit_mock_get_ctrl`, but it feels
> like the names might be long enough that they would hinder readability.
>
> ## Usage
> For now the only use of class mocking is in kunit-example-test.c
> As part of changing this from an RFC to a real patch set, we're hoping
> to include at least one example.
>
> Pointers to bits of code where this would be useful that aren't too
> hairy would be appreciated.
> E.g. could easily add a test for tools/perf/ui/progress.h, e.g. that
> ui_progress__init() calls ui_progress_ops.init(), but that likely isn't
> useful to anyone.
>
> ---
> v2:
> * Pass `struct kunit *` to mock init's to allow allocating ops structs.
> * Update kunit-example-test.cc to do so as a more realistic example.
> v1: https://lore.kernel.org/linux-kselftest/20200918183114.2571146-1-dlatypov@google.com/
> ---
>
> Brendan Higgins (9):
>   kunit: test: add kunit_stream a std::stream like logger
>   kunit: test: add concept of post conditions
>   checkpatch: add support for struct MOCK(foo) syntax
>   kunit: mock: add parameter list manipulation macros
>   kunit: mock: add internal mock infrastructure
>   kunit: mock: add basic matchers and actions
>   kunit: mock: add class mocking support
>   kunit: mock: add struct param matcher
>   kunit: mock: implement nice, strict and naggy mock distinctions
>
> Daniel Latypov (2):
>   Revert "kunit: move string-stream.h to lib/kunit"
>   kunit: expose kunit_set_failure() for use by mocking
>
> Marcelo Schmitt (1):
>   kunit: mock: add macro machinery to pick correct format args
>
>  include/kunit/assert.h                 |   3 +-
>  include/kunit/kunit-stream.h           |  94 +++
>  include/kunit/mock.h                   | 902 +++++++++++++++++++++++++
>  include/kunit/params.h                 | 305 +++++++++
>  {lib => include}/kunit/string-stream.h |   2 +
>  include/kunit/test.h                   |   9 +
>  lib/kunit/Makefile                     |   9 +-
>  lib/kunit/assert.c                     |   2 -
>  lib/kunit/common-mocks.c               | 409 +++++++++++
>  lib/kunit/kunit-example-test.c         |  98 +++
>  lib/kunit/kunit-stream.c               | 110 +++
>  lib/kunit/mock-macro-test.c            | 241 +++++++
>  lib/kunit/mock-test.c                  | 531 +++++++++++++++
>  lib/kunit/mock.c                       | 370 ++++++++++
>  lib/kunit/string-stream-test.c         |   3 +-
>  lib/kunit/string-stream.c              |   5 +-
>  lib/kunit/test.c                       |  15 +-
>  scripts/checkpatch.pl                  |   4 +
>  18 files changed, 3099 insertions(+), 13 deletions(-)
>  create mode 100644 include/kunit/kunit-stream.h
>  create mode 100644 include/kunit/mock.h
>  create mode 100644 include/kunit/params.h
>  rename {lib => include}/kunit/string-stream.h (95%)
>  create mode 100644 lib/kunit/common-mocks.c
>  create mode 100644 lib/kunit/kunit-stream.c
>  create mode 100644 lib/kunit/mock-macro-test.c
>  create mode 100644 lib/kunit/mock-test.c
>  create mode 100644 lib/kunit/mock.c
>
>
> base-commit: 10b82d5176488acee2820e5a2cf0f2ec5c3488b6
> --
> 2.28.0.1011.ga647a8990f-goog
>