On Fri, Sep 18, 2020 at 11:31 AM Daniel Latypov <dlatypov@xxxxxxxxxx> 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 >