lkml.org 
[lkml]   [2020]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [RFC v1 00/12] kunit: introduce class mocking support.
On Fri, Sep 18, 2020 at 11:31 AM 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);
> };

Discussed this offline a bit with Brendan and David.
The requirement that the first argument is a `sender *` means this
doesn't work for a few common cases.

E.g. ops structs don't usually have funcs that take `XXX_ops *`
`pci_ops` all take a `struct pci_bus *`, which at least contains a
`struct pci_ops*`.

Why does this matter?
We need to stash a `struct mock` somewhere to store expectations.
For this version of class mocking (in patch 10), we've assumed we could have

struct MOCK(sender) {
struct mock ctrl;
struct sender trgt; //&trgt assumed to be the first param
}

Then we can always use `container_of(sender)` to get the MOCK(sender)
which holds `ctrl`.
But if the first parameter isn't a `struct sender*`, this trick
obviously doesn't work.

So to support something like pci_ops, we'd need another approach to
stash `ctrl`.

After thinking a bit more, we could perhaps decouple the first param
from the mocked struct.
Using pci_ops as the example:

struct MOCK(pci_ops) {
struct mock ctrl;
struct pci_bus *self; // this is the first param. Using
python terminology here.
struct pci_ops trgt; // continue to store this, as this holds
the function pointers
}

// Name of this func is currently based on the class we're mocking
static inline struct mock *from_pci_ops_to_mock(
const struct pci_bus *self)
{
return mock_get_ctrl(container_of(self, struct MOCK(pci_ops), self));
}

// then in tests, we'd need something like
struct pci_bus bus;
bus.ops = mock_get_trgt(mock_ops);
mock_ops.self = &bus;


Do others have thoughts/opinions?

After grepping around for examples, I'm struck by how the number of
outliers there are.
E.g. most take a `struct thing *` as the first param, but cfspi_ifc in
caif_spi.h opts to take that as the last parameter.
And others take a different mix of structs for each function.

But it feels like a decoupled self / struct-holding-func-pointers
approach should be generic enough, as far as I can tell.
The more exotic types will probably have to remain unsupported.

>
> 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.
>
>
> 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 | 90 +++
> 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, 3091 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.681.g6f77f65b4e-goog
>

\
 
 \ /
  Last update: 2020-09-23 02:25    [W:0.135 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site