On Tue, Sep 22, 2020 at 5:24 PM Daniel Latypov dlatypov@google.com wrote:
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?
In the above example, wouldn't we actually want to mock struct pci_bus, not struct pci_ops?
I think pci_bus is what actually gets implemented. Consider the Rockchip PCIe host controller:
Rockchip provides it's own pci_ops struct:
https://elixir.bootlin.com/linux/latest/source/drivers/pci/controller/pcie-r...
If you look at one of the implemented methods, say rockchip_pcie_rd_conf(), you will see:
... struct rockchip_pcie *rockchip = bus->sysdata; ... (This function signature is:
int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
).
Thus, conceptually struct pci_bus is what is being manipulated; it best fits the candidate for the *self pointer.
In fact - if I am not mistaken - I think if you were to mock pci_bus and just pretend that the methods were on pci_bus, not pci_ops, I think it might just work.
The bigger problem is that it seems pci_bus does not want the user to allocate it: in the case of rockchip_pcie, it is allocated by devm_pci_alloc_host_bridge(). Thus, embedding pci_bus inside of a MOCK(pci_bus) would probably not work, so you would need to have different tooling around that and would then need to provide a custom definition of from_pci_bus_to_mock() (generated by DECLARE_STRUCT_CLASS_MOCK_CONVERTER()).
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.
That's not a problem, just use the DECLARE_STRUCT_CLASS_MOCK_HANDLE_INDEX() variant to create your mock (as opposed to DECLARE_STRUCT_CLASS_MOCK()).
For example let's say you have the following struct that you want to mock:
struct example { ... int (*foo)(int bar, char baz, struct example *self); };
You could mock the function with:
DECLARE_STRUCT_CLASS_MOCK_HANDLE_INDEX( METHOD(foo), CLASS(example), 2, /* The "handle" is in position 2. */ RETURNS(int), PARAMS(int, char, struct example *));
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.
I think the code is pretty much here to handle any situation in which one of the parameters input to the function can be used to look up a mock object; we just need to expose the from_<class>_to_mock() function to the user to override. The problem I see with what we have now is the assumption that the user gets to create the object that they want to mock. Your example has illustrated that that is not a safe assumption.
But yes, sufficiently exoctic cases will have to be unsupported.
BTW, sorry for not sharing the above in our offline discussion; since we were talking without concrete examples in front of us, the problem sounded worse than it looks here.
[...]