* YASL Request @ 2018-04-09 22:27 Patrick Venture 2018-04-10 2:45 ` Lei YU ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Patrick Venture @ 2018-04-09 22:27 UTC (permalink / raw) To: OpenBMC Maillist, Brad Bishop, Emily Shaffer Everyone, I'm working on unit-testing in openbmc, and have cracked most of sdbusplus into mockable pieces and verified I can in fact test against those mocks with a downstream daemon. I'll be grabbing an upstream daemon (or providing a piece of one) to demonstrate how to leverage the mocks to test OpenBMC. If one designs with testing in mind, the designs come out very differently if not, and so getting unit-tests throughout OpenBMC will be a lot of breaking things apart into testable pieces. Anyways, where I'm going with this email is that everything we do within a daemon needs to be something that can be mocked -- basically. *** What do I mean specifically? Consider, file access. If a daemon routes all file accesses throug a common object implementation provided by a shared library, that shared library can easily also provide a mock interface for those accesses, such that one can easily verify behaviors based on file contents without implementing local files or trying to inject errors. With a mock's file system interface, you can simply say that a file was unable to be read, or written, or opened, etc. Another example is mocking ctime. If you want to test whether something happens after some sleep or period, if your code can receive a mock version of that library, one can deliberately control the results of 'time' or 'difftime', etc. I have to build these interfaces for some of our downstream daemons and likely for other parts of OpenBMC, and to avoid code duplication it'll help to have them in some library. YASL (yet-another-shared-library) Request. Previous conversations along these lines lead to the idea that we need multiple new libraries for various things. So, this is yet another use case. The library itself should be written in such a way that it can be tested via unit-tests, but depending on how thin of a shim it is, that isn't always practical. See: class FileInterface { public: virtual int open(const char *filename, int flags) = 0; }; class FileImplementation : public FileInterface { public: int open(const char *filename, int flags) override { return ::open(filename, flags); } }; class FileMock : public FileInterface { public: MOCK_METHOD2(open, int(const char *, int)); }; .... then one just uses the FileInterface for their normally direct posix-style file access. This can easily wrap iostream, or fstream, or anything. And then if we provide some libraries for use by daemons, they can transition over to them over time, and then they get mocks for free :D For a daemon downstream, I've written a ctime wrapper, I'll submit it for consideration later tonight along with a few other things, and then I'll reply to this email with links. ***Not meant as a unit-test primer, just trying to provide some real examples. Patrick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-09 22:27 YASL Request Patrick Venture @ 2018-04-10 2:45 ` Lei YU 2018-04-10 3:19 ` Patrick Venture 2018-04-11 3:52 ` Brad Bishop 2018-04-11 2:34 ` Brad Bishop 2018-04-13 3:56 ` Andrew Jeffery 2 siblings, 2 replies; 19+ messages in thread From: Lei YU @ 2018-04-10 2:45 UTC (permalink / raw) To: Patrick Venture; +Cc: OpenBMC Maillist, Brad Bishop, Emily Shaffer On Tue, Apr 10, 2018 at 6:27 AM, Patrick Venture <venture@google.com> wrote: > Everyone, > > I'm working on unit-testing in openbmc, and have cracked most of > sdbusplus into mockable pieces and verified I can in fact test against > those mocks with a downstream daemon. I'll be grabbing an upstream Great! I have tried to make sdbusplus mockable previously, by changing its interfaces virtual, and find out that it is somehow complicated because some of the interfaces return the objects and it's kind of hard to mock things like bus::new_method_call(). At that time I discussed this with Patrick Williams and he suggested sdbusplus should be compact and fast, so it's not a good idea to make it virtual. Later it's found that Brad has some good example of mocking sdbusplus in https://github.com/openbmc/phosphor-dbus-monitor/tree/master/src/test Hopefully we can get a mockable sdbusplus as a shared library as well! > daemon (or providing a piece of one) to demonstrate how to leverage > the mocks to test OpenBMC. If one designs with testing in mind, the > designs come out very differently if not, and so getting unit-tests > throughout OpenBMC will be a lot of breaking things apart into > testable pieces. Anyways, where I'm going with this email is that > everything we do within a daemon needs to be something that can be > mocked -- basically. > > *** > What do I mean specifically? Consider, file access. If a daemon > routes all file accesses throug a common object implementation > provided by a shared library, that shared library can easily also > provide a mock interface for those accesses, such that one can easily > verify behaviors based on file contents without implementing local > files or trying to inject errors. With a mock's file system > interface, you can simply say that a file was unable to be read, or > written, or opened, etc. Another example is mocking ctime. If you > want to test whether something happens after some sleep or period, if > your code can receive a mock version of that library, one can > deliberately control the results of 'time' or 'difftime', etc. I have > to build these interfaces for some of our downstream daemons and > likely for other parts of OpenBMC, and to avoid code duplication it'll > help to have them in some library. > > YASL (yet-another-shared-library) Request. > > Previous conversations along these lines lead to the idea that we need > multiple new libraries for various things. So, this is yet another > use case. The library itself should be written in such a way that it > can be tested via unit-tests, but depending on how thin of a shim it > is, that isn't always practical. See: > > class FileInterface { > public: > virtual int open(const char *filename, int flags) = 0; > }; > > class FileImplementation : public FileInterface { > public: > int open(const char *filename, int flags) override { > return ::open(filename, flags); > } > }; > > class FileMock : public FileInterface { > public: > MOCK_METHOD2(open, int(const char *, int)); > }; > > .... then one just uses the FileInterface for their normally direct > posix-style file access. This can easily wrap iostream, or fstream, > or anything. And then if we provide some libraries for use by > daemons, they can transition over to them over time, and then they get > mocks for free :D For a daemon downstream, I've written a ctime > wrapper, I'll submit it for consideration later tonight along with a > few other things, and then I'll reply to this email with links. > So this library would contain several interfaces classes (e.g. FileInterface, TimeInterface, and hopefully SdbusplusInterface etc) all together, right? I vote for it! > ***Not meant as a unit-test primer, just trying to provide some real examples. > > Patrick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-10 2:45 ` Lei YU @ 2018-04-10 3:19 ` Patrick Venture 2018-04-10 17:59 ` Patrick Venture 2018-04-11 3:52 ` Brad Bishop 1 sibling, 1 reply; 19+ messages in thread From: Patrick Venture @ 2018-04-10 3:19 UTC (permalink / raw) To: Lei YU; +Cc: OpenBMC Maillist, Brad Bishop, Emily Shaffer On Mon, Apr 9, 2018 at 7:45 PM, Lei YU <mine260309@gmail.com> wrote: > On Tue, Apr 10, 2018 at 6:27 AM, Patrick Venture <venture@google.com> wrote: >> Everyone, >> >> I'm working on unit-testing in openbmc, and have cracked most of >> sdbusplus into mockable pieces and verified I can in fact test against >> those mocks with a downstream daemon. I'll be grabbing an upstream > > Great! I have tried to make sdbusplus mockable previously, by changing its > interfaces virtual, and find out that it is somehow complicated because some > of the interfaces return the objects and it's kind of hard to mock things like > bus::new_method_call(). Yeah, ran into that problem, worked around it. I'm currently fighting my way through the var_args in message::append where there can be mixed types, so I can't just cast the tuple to a normal array. Could use boost::any, but haven't given into that yet. > At that time I discussed this with Patrick Williams and he suggested sdbusplus > should be compact and fast, so it's not a good idea to make it virtual. > Later it's found that Brad has some good example of mocking sdbusplus in > https://github.com/openbmc/phosphor-dbus-monitor/tree/master/src/test > > Hopefully we can get a mockable sdbusplus as a shared library as well! I'm mocking it in sdbusplus itself, so you get it for free. > >> daemon (or providing a piece of one) to demonstrate how to leverage >> the mocks to test OpenBMC. If one designs with testing in mind, the >> designs come out very differently if not, and so getting unit-tests >> throughout OpenBMC will be a lot of breaking things apart into >> testable pieces. Anyways, where I'm going with this email is that >> everything we do within a daemon needs to be something that can be >> mocked -- basically. >> >> *** >> What do I mean specifically? Consider, file access. If a daemon >> routes all file accesses throug a common object implementation >> provided by a shared library, that shared library can easily also >> provide a mock interface for those accesses, such that one can easily >> verify behaviors based on file contents without implementing local >> files or trying to inject errors. With a mock's file system >> interface, you can simply say that a file was unable to be read, or >> written, or opened, etc. Another example is mocking ctime. If you >> want to test whether something happens after some sleep or period, if >> your code can receive a mock version of that library, one can >> deliberately control the results of 'time' or 'difftime', etc. I have >> to build these interfaces for some of our downstream daemons and >> likely for other parts of OpenBMC, and to avoid code duplication it'll >> help to have them in some library. >> >> YASL (yet-another-shared-library) Request. >> >> Previous conversations along these lines lead to the idea that we need >> multiple new libraries for various things. So, this is yet another >> use case. The library itself should be written in such a way that it >> can be tested via unit-tests, but depending on how thin of a shim it >> is, that isn't always practical. See: >> >> class FileInterface { >> public: >> virtual int open(const char *filename, int flags) = 0; >> }; >> >> class FileImplementation : public FileInterface { >> public: >> int open(const char *filename, int flags) override { >> return ::open(filename, flags); >> } >> }; >> >> class FileMock : public FileInterface { >> public: >> MOCK_METHOD2(open, int(const char *, int)); >> }; >> >> .... then one just uses the FileInterface for their normally direct >> posix-style file access. This can easily wrap iostream, or fstream, >> or anything. And then if we provide some libraries for use by >> daemons, they can transition over to them over time, and then they get >> mocks for free :D For a daemon downstream, I've written a ctime >> wrapper, I'll submit it for consideration later tonight along with a >> few other things, and then I'll reply to this email with links. >> > > So this library would contain several interfaces classes (e.g. FileInterface, > TimeInterface, and hopefully SdbusplusInterface etc) all together, right? > I vote for it! The sdbusplus library will have mocks ready sometime this week if I can get past this va_args issue. I haven't tried converting the call to sd_bus_message_append to sd_bus_message_appendv, but I haven't burned much time trying to handle this. I haven't yet mocked out everything, but sdbusplus::bus::bus and sdbusplus::message::message are nearly done. There are other bits, I just haven't started. If we have a set of shared libraries, then those shared libraries should all have mocks available to enable testing. > >> ***Not meant as a unit-test primer, just trying to provide some real examples. >> >> Patrick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-10 3:19 ` Patrick Venture @ 2018-04-10 17:59 ` Patrick Venture 0 siblings, 0 replies; 19+ messages in thread From: Patrick Venture @ 2018-04-10 17:59 UTC (permalink / raw) To: Lei YU; +Cc: OpenBMC Maillist, Brad Bishop, Emily Shaffer On Mon, Apr 9, 2018 at 8:19 PM, Patrick Venture <venture@google.com> wrote: > On Mon, Apr 9, 2018 at 7:45 PM, Lei YU <mine260309@gmail.com> wrote: >> On Tue, Apr 10, 2018 at 6:27 AM, Patrick Venture <venture@google.com> wrote: >>> Everyone, >>> >>> I'm working on unit-testing in openbmc, and have cracked most of >>> sdbusplus into mockable pieces and verified I can in fact test against >>> those mocks with a downstream daemon. I'll be grabbing an upstream >> >> Great! I have tried to make sdbusplus mockable previously, by changing its >> interfaces virtual, and find out that it is somehow complicated because some >> of the interfaces return the objects and it's kind of hard to mock things like >> bus::new_method_call(). > > Yeah, ran into that problem, worked around it. I'm currently fighting > my way through the var_args in message::append where there can be > mixed types, so I can't just cast the tuple to a normal array. Could > use boost::any, but haven't given into that yet. Resolved. I'll have this up for review today along with some dummy code that exercises it. > >> At that time I discussed this with Patrick Williams and he suggested sdbusplus >> should be compact and fast, so it's not a good idea to make it virtual. >> Later it's found that Brad has some good example of mocking sdbusplus in >> https://github.com/openbmc/phosphor-dbus-monitor/tree/master/src/test >> >> Hopefully we can get a mockable sdbusplus as a shared library as well! > > I'm mocking it in sdbusplus itself, so you get it for free. > >> >>> daemon (or providing a piece of one) to demonstrate how to leverage >>> the mocks to test OpenBMC. If one designs with testing in mind, the >>> designs come out very differently if not, and so getting unit-tests >>> throughout OpenBMC will be a lot of breaking things apart into >>> testable pieces. Anyways, where I'm going with this email is that >>> everything we do within a daemon needs to be something that can be >>> mocked -- basically. >>> >>> *** >>> What do I mean specifically? Consider, file access. If a daemon >>> routes all file accesses throug a common object implementation >>> provided by a shared library, that shared library can easily also >>> provide a mock interface for those accesses, such that one can easily >>> verify behaviors based on file contents without implementing local >>> files or trying to inject errors. With a mock's file system >>> interface, you can simply say that a file was unable to be read, or >>> written, or opened, etc. Another example is mocking ctime. If you >>> want to test whether something happens after some sleep or period, if >>> your code can receive a mock version of that library, one can >>> deliberately control the results of 'time' or 'difftime', etc. I have >>> to build these interfaces for some of our downstream daemons and >>> likely for other parts of OpenBMC, and to avoid code duplication it'll >>> help to have them in some library. >>> >>> YASL (yet-another-shared-library) Request. >>> >>> Previous conversations along these lines lead to the idea that we need >>> multiple new libraries for various things. So, this is yet another >>> use case. The library itself should be written in such a way that it >>> can be tested via unit-tests, but depending on how thin of a shim it >>> is, that isn't always practical. See: >>> >>> class FileInterface { >>> public: >>> virtual int open(const char *filename, int flags) = 0; >>> }; >>> >>> class FileImplementation : public FileInterface { >>> public: >>> int open(const char *filename, int flags) override { >>> return ::open(filename, flags); >>> } >>> }; >>> >>> class FileMock : public FileInterface { >>> public: >>> MOCK_METHOD2(open, int(const char *, int)); >>> }; >>> >>> .... then one just uses the FileInterface for their normally direct >>> posix-style file access. This can easily wrap iostream, or fstream, >>> or anything. And then if we provide some libraries for use by >>> daemons, they can transition over to them over time, and then they get >>> mocks for free :D For a daemon downstream, I've written a ctime >>> wrapper, I'll submit it for consideration later tonight along with a >>> few other things, and then I'll reply to this email with links. >>> >> >> So this library would contain several interfaces classes (e.g. FileInterface, >> TimeInterface, and hopefully SdbusplusInterface etc) all together, right? >> I vote for it! > > The sdbusplus library will have mocks ready sometime this week if I > can get past this va_args issue. I haven't tried converting the call > to sd_bus_message_append to sd_bus_message_appendv, but I haven't > burned much time trying to handle this. I haven't yet mocked out > everything, but sdbusplus::bus::bus and sdbusplus::message::message > are nearly done. There are other bits, I just haven't started. > > If we have a set of shared libraries, then those shared libraries > should all have mocks available to enable testing. > >> >>> ***Not meant as a unit-test primer, just trying to provide some real examples. >>> >>> Patrick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-10 2:45 ` Lei YU 2018-04-10 3:19 ` Patrick Venture @ 2018-04-11 3:52 ` Brad Bishop 2018-04-11 15:59 ` Patrick Venture 1 sibling, 1 reply; 19+ messages in thread From: Brad Bishop @ 2018-04-11 3:52 UTC (permalink / raw) To: Patrick Venture, Lei YU; +Cc: Emily Shaffer, OpenBMC Maillist > On Apr 9, 2018, at 10:45 PM, Lei YU <mine260309@gmail.com> wrote: > > On Tue, Apr 10, 2018 at 6:27 AM, Patrick Venture <venture@google.com> wrote: >> Everyone, >> >> I'm working on unit-testing in openbmc, and have cracked most of >> sdbusplus into mockable pieces and verified I can in fact test against >> those mocks with a downstream daemon. I'll be grabbing an upstream > > Great! I have tried to make sdbusplus mockable previously, by changing its > interfaces virtual, and find out that it is somehow complicated because some > of the interfaces return the objects and it's kind of hard to mock things like > bus::new_method_call(). > At that time I discussed this with Patrick Williams and he suggested sdbusplus > should be compact and fast, so it's not a good idea to make it virtual. FWIW, I agree with Patrick W’s sentiment here. I’m all for unit tests, but un-optimizing existing code so we can use a specific framework in a specific way doesn’t sit well with me. > Later it's found that Brad has some good example of mocking sdbusplus in > https://github.com/openbmc/phosphor-dbus-monitor/tree/master/src/test Yes, I was able to mock sdbusplus with GMock without having to change sdbusplus at all. Patrick, is something like that on the table? It seems like instead of the typical: class iface { virtual void sdbusplus_foo() = 0; }; clase real : public iface { void sdbusplus_foo() {} }; class fake : public face { MOCKME(sdbusplus_foo); }; int main() { real r; r.sdbusplus_foo(); } void testcase() { fake f; EXPECT_BLAH_BLAH(); f.sdbusplus_foo(); } ———————————————— we could do: class real { void sdbusplus_foo() {} }; class fake { MOCKME(sdbusplus_foo); }; template <typename T> MyClass { MyClass(T& interface) { interface.sdbusplus_foo(); } }; int main() { real r; MyClass<real> m(r); } void testcase() { fake f; MyClass<fake> m(f); } ———————————————— Granted this requires _all_ of our code to be templated, obfuscates what is really going on and is all just a bit silly, but from where I sit it isn’t any more obfuscated than wrapping every shared library function with an abstract interface class _and_ you don’t incur the overhead of doing that. Am I overlooking anything? I’m only half serious with the proposal. My real point is I feel like we are jumping through hoops to make a unit test framework work despite there being other ways of writing unit tests. Lei - thank you for weighing in. If this was a poll it would be: 2 in favor of becoming a GMock project at the core. 1 against If no-one else speaks up, then I lose :-). But it would be nice to improve the margin of error. > > Hopefully we can get a mockable sdbusplus as a shared library as well! > >> daemon (or providing a piece of one) to demonstrate how to leverage >> the mocks to test OpenBMC. If one designs with testing in mind, the >> designs come out very differently if not, and so getting unit-tests >> throughout OpenBMC will be a lot of breaking things apart into >> testable pieces. Anyways, where I'm going with this email is that >> everything we do within a daemon needs to be something that can be >> mocked -- basically. >> >> *** >> What do I mean specifically? Consider, file access. If a daemon >> routes all file accesses throug a common object implementation >> provided by a shared library, that shared library can easily also >> provide a mock interface for those accesses, such that one can easily >> verify behaviors based on file contents without implementing local >> files or trying to inject errors. With a mock's file system >> interface, you can simply say that a file was unable to be read, or >> written, or opened, etc. Another example is mocking ctime. If you >> want to test whether something happens after some sleep or period, if >> your code can receive a mock version of that library, one can >> deliberately control the results of 'time' or 'difftime', etc. I have >> to build these interfaces for some of our downstream daemons and >> likely for other parts of OpenBMC, and to avoid code duplication it'll >> help to have them in some library. >> >> YASL (yet-another-shared-library) Request. >> >> Previous conversations along these lines lead to the idea that we need >> multiple new libraries for various things. So, this is yet another >> use case. The library itself should be written in such a way that it >> can be tested via unit-tests, but depending on how thin of a shim it >> is, that isn't always practical. See: >> >> class FileInterface { >> public: >> virtual int open(const char *filename, int flags) = 0; >> }; >> >> class FileImplementation : public FileInterface { >> public: >> int open(const char *filename, int flags) override { >> return ::open(filename, flags); >> } >> }; >> >> class FileMock : public FileInterface { >> public: >> MOCK_METHOD2(open, int(const char *, int)); >> }; >> >> .... then one just uses the FileInterface for their normally direct >> posix-style file access. This can easily wrap iostream, or fstream, >> or anything. And then if we provide some libraries for use by >> daemons, they can transition over to them over time, and then they get >> mocks for free :D For a daemon downstream, I've written a ctime >> wrapper, I'll submit it for consideration later tonight along with a >> few other things, and then I'll reply to this email with links. >> > > So this library would contain several interfaces classes (e.g. FileInterface, > TimeInterface, and hopefully SdbusplusInterface etc) all together, right? > I vote for it! > >> ***Not meant as a unit-test primer, just trying to provide some real examples. >> >> Patrick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-11 3:52 ` Brad Bishop @ 2018-04-11 15:59 ` Patrick Venture 0 siblings, 0 replies; 19+ messages in thread From: Patrick Venture @ 2018-04-11 15:59 UTC (permalink / raw) To: Brad Bishop; +Cc: Lei YU, Emily Shaffer, OpenBMC Maillist On Tue, Apr 10, 2018 at 8:52 PM, Brad Bishop <bradleyb@fuzziesquirrel.com> wrote: > >> On Apr 9, 2018, at 10:45 PM, Lei YU <mine260309@gmail.com> wrote: >> >> On Tue, Apr 10, 2018 at 6:27 AM, Patrick Venture <venture@google.com> wrote: >>> Everyone, >>> >>> I'm working on unit-testing in openbmc, and have cracked most of >>> sdbusplus into mockable pieces and verified I can in fact test against >>> those mocks with a downstream daemon. I'll be grabbing an upstream >> >> Great! I have tried to make sdbusplus mockable previously, by changing its >> interfaces virtual, and find out that it is somehow complicated because some >> of the interfaces return the objects and it's kind of hard to mock things like >> bus::new_method_call(). >> At that time I discussed this with Patrick Williams and he suggested sdbusplus >> should be compact and fast, so it's not a good idea to make it virtual. > > FWIW, I agree with Patrick W’s sentiment here. I’m all for unit tests, but > un-optimizing existing code so we can use a specific framework in a specific > way doesn’t sit well with me. > >> Later it's found that Brad has some good example of mocking sdbusplus in >> https://github.com/openbmc/phosphor-dbus-monitor/tree/master/src/test > > Yes, I was able to mock sdbusplus with GMock without having to change sdbusplus > at all. Patrick, is something like that on the table? It seems like instead of > the typical: > > class iface > { > virtual void sdbusplus_foo() = 0; > }; > > clase real : public iface > { > void sdbusplus_foo() {} > }; > > class fake : public face > { > MOCKME(sdbusplus_foo); > }; > > int main() > { > real r; > r.sdbusplus_foo(); > } > > void testcase() > { > fake f; > EXPECT_BLAH_BLAH(); > f.sdbusplus_foo(); > } > > ———————————————— > we could do: > > class real > { > void sdbusplus_foo() {} > }; > > class fake > { > MOCKME(sdbusplus_foo); > }; > > template <typename T> > MyClass > { > MyClass(T& interface) > { > interface.sdbusplus_foo(); > } > }; > > int main() > { > real r; > MyClass<real> m(r); > } > > void testcase() > { > fake f; > MyClass<fake> m(f); > } > ———————————————— We have both models down here. One where the interface is a template'd argument, as you've indicated, and the kind where it's virtual. I was hoping to avoid more and more templates, templates on templates, and the like. However, this is a case where you can do either depending on the library. I don't really feel strongly about it in that regard and Gmock IIRC, does support both options. My goal was to be less disruptive where possible. With templating, you need to specify all the interfaces all the time, whereas with the injection model you only need to specify them in the non-default case. See sdbuspluss::message::message, where by default, it works as before, but you can make it use a mock interface, or even another library that shares signatures. To me, this was a win. Imagine the following: class Handler { public: arbitraryMethod(void *args) { otherThing(j); } receiveDataCallback(size_t len, void *p) { sendData(len, p); } }; Such that sendData and otherThing are from different libraries, then it'd end up being multiple templates all the time, everywhere. Now, conceivably if your object interacts with lots of different things it might be a bad design, or it might be just the way it is. > Granted this requires _all_ of our code to be templated, obfuscates what is > really going on and is all just a bit silly, Yes. I was thinking, one template for everything isn't that bad, but it grows. > but from where I sit it isn’t > any more obfuscated than wrapping every shared library function with an abstract > interface class _and_ you don’t incur the overhead of doing that. Am I overlooking > anything? One way to handle this would be to actually make the interface an optional parameter in that, it's only used when set. void doThing(size_t len, void *p) { if (intf) { return intf->sendData(len, p); } return sendData(len, p); } However, I think that's uglier than the other approach. With simple objects, a virtual lookup is trivial to the point it's basically equivalent to a function pointer in a struct. At Google, we're also seeing some serious CPU usage and wastes of time, but a big culprit of this is that nothing caches anything ever. If you want to talk to a dbus object, you get the service, then you get it. Every time. So, basically anything talks to the mapper. And the mapper is just a python script getting hammered all the time. We lose a lot of time talking to it. Hence, the recent efforts to start caching sdbus addresses since they only change in rare cases. I know on the 2400, sometimes it's perfectly responsive and then it can get a bit wedged and then frees up again with some regularity, so to me, it's already slow. And I don't think this'll make it slower as this isn't the slowest path. This might make the fastest path slightly slower but with the trade off of having trust in the code. > > I’m only half serious with the proposal. My real point is I feel like we are > jumping through hoops to make a unit test framework work despite there being other > ways of writing unit tests. > > Lei - thank you for weighing in. If this was a poll it would be: > > 2 in favor of becoming a GMock project at the core. > 1 against > > If no-one else speaks up, then I lose :-). But it would be nice to improve the > margin of error. > > >> >> Hopefully we can get a mockable sdbusplus as a shared library as well! >> >>> daemon (or providing a piece of one) to demonstrate how to leverage >>> the mocks to test OpenBMC. If one designs with testing in mind, the >>> designs come out very differently if not, and so getting unit-tests >>> throughout OpenBMC will be a lot of breaking things apart into >>> testable pieces. Anyways, where I'm going with this email is that >>> everything we do within a daemon needs to be something that can be >>> mocked -- basically. >>> >>> *** >>> What do I mean specifically? Consider, file access. If a daemon >>> routes all file accesses throug a common object implementation >>> provided by a shared library, that shared library can easily also >>> provide a mock interface for those accesses, such that one can easily >>> verify behaviors based on file contents without implementing local >>> files or trying to inject errors. With a mock's file system >>> interface, you can simply say that a file was unable to be read, or >>> written, or opened, etc. Another example is mocking ctime. If you >>> want to test whether something happens after some sleep or period, if >>> your code can receive a mock version of that library, one can >>> deliberately control the results of 'time' or 'difftime', etc. I have >>> to build these interfaces for some of our downstream daemons and >>> likely for other parts of OpenBMC, and to avoid code duplication it'll >>> help to have them in some library. >>> >>> YASL (yet-another-shared-library) Request. >>> >>> Previous conversations along these lines lead to the idea that we need >>> multiple new libraries for various things. So, this is yet another >>> use case. The library itself should be written in such a way that it >>> can be tested via unit-tests, but depending on how thin of a shim it >>> is, that isn't always practical. See: >>> >>> class FileInterface { >>> public: >>> virtual int open(const char *filename, int flags) = 0; >>> }; >>> >>> class FileImplementation : public FileInterface { >>> public: >>> int open(const char *filename, int flags) override { >>> return ::open(filename, flags); >>> } >>> }; >>> >>> class FileMock : public FileInterface { >>> public: >>> MOCK_METHOD2(open, int(const char *, int)); >>> }; >>> >>> .... then one just uses the FileInterface for their normally direct >>> posix-style file access. This can easily wrap iostream, or fstream, >>> or anything. And then if we provide some libraries for use by >>> daemons, they can transition over to them over time, and then they get >>> mocks for free :D For a daemon downstream, I've written a ctime >>> wrapper, I'll submit it for consideration later tonight along with a >>> few other things, and then I'll reply to this email with links. >>> >> >> So this library would contain several interfaces classes (e.g. FileInterface, >> TimeInterface, and hopefully SdbusplusInterface etc) all together, right? >> I vote for it! >> >>> ***Not meant as a unit-test primer, just trying to provide some real examples. >>> >>> Patrick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-09 22:27 YASL Request Patrick Venture 2018-04-10 2:45 ` Lei YU @ 2018-04-11 2:34 ` Brad Bishop 2018-04-11 15:56 ` Patrick Venture 2018-04-12 14:25 ` Alexander Amelkin 2018-04-13 3:56 ` Andrew Jeffery 2 siblings, 2 replies; 19+ messages in thread From: Brad Bishop @ 2018-04-11 2:34 UTC (permalink / raw) To: Patrick Venture; +Cc: OpenBMC Maillist, Emily Shaffer > On Apr 9, 2018, at 6:27 PM, Patrick Venture <venture@google.com> wrote: > > Everyone, > > I'm working on unit-testing in openbmc, and have cracked most of > sdbusplus into mockable pieces and verified I can in fact test against > those mocks with a downstream daemon. I'll be grabbing an upstream > daemon (or providing a piece of one) to demonstrate how to leverage > the mocks to test OpenBMC. If one designs with testing in mind, the > designs come out very differently if not, and so getting unit-tests > throughout OpenBMC will be a lot of breaking things apart into > testable pieces. Anyways, where I'm going with this email is that > everything we do within a daemon needs to be something that can be > mocked -- basically. > > *** > What do I mean specifically? Consider, file access. If a daemon > routes all file accesses throug a common object implementation > provided by a shared library, that shared library can easily also > provide a mock interface for those accesses, such that one can easily > verify behaviors based on file contents without implementing local > files or trying to inject errors. With a mock's file system > interface, you can simply say that a file was unable to be read, or > written, or opened, etc. Another example is mocking ctime. If you > want to test whether something happens after some sleep or period, if > your code can receive a mock version of that library, one can > deliberately control the results of 'time' or 'difftime', etc. I have > to build these interfaces for some of our downstream daemons and > likely for other parts of OpenBMC, and to avoid code duplication it'll > help to have them in some library. > > YASL (yet-another-shared-library) Request. Can you talk more about what goes in this? Do you envision something like: openbmc repo -> upstream project being wrapped libcmock -> glibc libstdc++mock -> libstdc++ libsystemdmock -> libsystemd (or sdbusplusmock) libfoomock -> libfoo libbarmock -> libbarmock Or is there a super-repo/super-library that has all the wrappers? Do applications link a single shared library that provides wrappers or do they link multiple libraries that provide wrappers? I’m just trying to figure out what to call the repo, and get a feel for what the structure would look like over time. Is the repo for non-openbmc wrappers (like standard library stuff), and wrappers for openbmc hosted libraries like sdbusplus would go in the sdbusplus repo? > > Previous conversations along these lines lead to the idea that we need > multiple new libraries for various things. So, this is yet another > use case. The library itself should be written in such a way that it > can be tested via unit-tests, but depending on how thin of a shim it > is, that isn't always practical. See: > > class FileInterface { > public: > virtual int open(const char *filename, int flags) = 0; > }; > > class FileImplementation : public FileInterface { > public: > int open(const char *filename, int flags) override { > return ::open(filename, flags); > } > }; > > class FileMock : public FileInterface { > public: > MOCK_METHOD2(open, int(const char *, int)); > }; Doesn’t this style of programming slow things down (when you look at it at scale)? If you have a software stack, and you turn N shared library calls from simple branches into this, multiplied by N processes… aren’t we going to waste a lot of cycles? Is this how people want to use their BMC cycles or do they want it running their business logic? I know IBM is already struggling with bigger-than-ideal load averages / runqueue depths. Is it your vision the the project _requires_ that all applications be written in this manner and use GMock as the unit test framework as a condition for inclusion in OpenBMC, or are you just looking for the capability for applications that elect to use it? There are other ways to write unit tests. I think only allowing one framework (considering the impact it has on how we write code) would drive people away from the project. Assuming sdbusplus is made to be mockable, we can probably operate in the latter mode since we just don’t have very many shared libraries in the first place (a metric I’d like to maintain as we grow). > > .... then one just uses the FileInterface for their normally direct > posix-style file access. This can easily wrap iostream, or fstream, > or anything. And then if we provide some libraries for use by > daemons, they can transition over to them over time, and then they get > mocks for free :D For a daemon downstream, I've written a ctime > wrapper, I'll submit it for consideration later tonight along with a > few other things, and then I'll reply to this email with links. > > ***Not meant as a unit-test primer, just trying to provide some real examples. > > Patrick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-11 2:34 ` Brad Bishop @ 2018-04-11 15:56 ` Patrick Venture 2018-04-12 14:25 ` Alexander Amelkin 1 sibling, 0 replies; 19+ messages in thread From: Patrick Venture @ 2018-04-11 15:56 UTC (permalink / raw) To: Brad Bishop; +Cc: OpenBMC Maillist, Emily Shaffer Hi Brad, On Tue, Apr 10, 2018 at 7:34 PM, Brad Bishop <bradleyb@fuzziesquirrel.com> wrote: > >> On Apr 9, 2018, at 6:27 PM, Patrick Venture <venture@google.com> wrote: >> >> Everyone, >> >> I'm working on unit-testing in openbmc, and have cracked most of >> sdbusplus into mockable pieces and verified I can in fact test against >> those mocks with a downstream daemon. I'll be grabbing an upstream >> daemon (or providing a piece of one) to demonstrate how to leverage >> the mocks to test OpenBMC. If one designs with testing in mind, the >> designs come out very differently if not, and so getting unit-tests >> throughout OpenBMC will be a lot of breaking things apart into >> testable pieces. Anyways, where I'm going with this email is that >> everything we do within a daemon needs to be something that can be >> mocked -- basically. >> >> *** >> What do I mean specifically? Consider, file access. If a daemon >> routes all file accesses throug a common object implementation >> provided by a shared library, that shared library can easily also >> provide a mock interface for those accesses, such that one can easily >> verify behaviors based on file contents without implementing local >> files or trying to inject errors. With a mock's file system >> interface, you can simply say that a file was unable to be read, or >> written, or opened, etc. Another example is mocking ctime. If you >> want to test whether something happens after some sleep or period, if >> your code can receive a mock version of that library, one can >> deliberately control the results of 'time' or 'difftime', etc. I have >> to build these interfaces for some of our downstream daemons and >> likely for other parts of OpenBMC, and to avoid code duplication it'll >> help to have them in some library. >> >> YASL (yet-another-shared-library) Request. > > Can you talk more about what goes in this? > > Do you envision something like: > > openbmc repo -> upstream project being wrapped > libcmock -> glibc > libstdc++mock -> libstdc++ > libsystemdmock -> libsystemd (or sdbusplusmock) > libfoomock -> libfoo > libbarmock -> libbarmock > > Or is there a super-repo/super-library that has all the wrappers? > Do applications link a single shared library that provides wrappers > or do they link multiple libraries that provide wrappers? I really envision using shared libraries for mocks, and I don't see value in mocking glibc itself, there are certainly overkill things, for instance, there's no value IMHO to mocking memcmp from cstring. So far, from a quick audit of our code, I see uses of the experimental/fs library, direct file access, some fstream access, some time, and some socket accesses, and of course, sdbusplus tie-ins. And soon sdevent stuff, which really -- you'd be testing the callback method itself not that it gets called -- strictly speaking, you could... but it's wonky. For a program downstream, I wrote something simple class TimeInterface { public: virtual std::time_t time(std::time_t *t) = 0; }; class TimeImpl { public: std::time_t time(std::time_t *t) override { return ::time(t); } }; class TimeMock { }; The idea being to control the value from time. However, if you didn't care about that, you could focus on difftime(), which was actually doing the computation, to make it return whatever you needed to trigger the cases you needed. > > I’m just trying to figure out what to call the repo, and get a feel > for what the structure would look like over time. Is the repo for > non-openbmc wrappers (like standard library stuff), and wrappers for > openbmc hosted libraries like sdbusplus would go in the sdbusplus > repo? If you wanted a repo for each library, that'd be your call. I think it's fine to group them. > >> >> Previous conversations along these lines lead to the idea that we need >> multiple new libraries for various things. So, this is yet another >> use case. The library itself should be written in such a way that it >> can be tested via unit-tests, but depending on how thin of a shim it >> is, that isn't always practical. See: >> >> class FileInterface { >> public: >> virtual int open(const char *filename, int flags) = 0; >> }; >> >> class FileImplementation : public FileInterface { >> public: >> int open(const char *filename, int flags) override { >> return ::open(filename, flags); >> } >> }; >> >> class FileMock : public FileInterface { >> public: >> MOCK_METHOD2(open, int(const char *, int)); >> }; > > Doesn’t this style of programming slow things down (when you look at > it at scale)? If you have a software stack, and you turn N shared > library calls from simple branches into this, multiplied by N processes… > aren’t we going to waste a lot of cycles? Is this how people want > to use their BMC cycles or do they want it running their business > logic? I know IBM is already struggling with bigger-than-ideal load > averages / runqueue depths. > > Is it your vision the the project _requires_ that all applications > be written in this manner and use GMock as the unit test framework as > a condition for inclusion in OpenBMC, or are you just looking for the > capability for applications that elect to use it? There are other ways > to write unit tests. I think only allowing one framework (considering > the impact it has on how we write code) would drive people away from > the project. Assuming sdbusplus is made to be mockable, we can probably > operate in the latter mode since we just don’t have very many shared > libraries in the first place (a metric I’d like to maintain as we grow). > >> >> .... then one just uses the FileInterface for their normally direct >> posix-style file access. This can easily wrap iostream, or fstream, >> or anything. And then if we provide some libraries for use by >> daemons, they can transition over to them over time, and then they get >> mocks for free :D For a daemon downstream, I've written a ctime >> wrapper, I'll submit it for consideration later tonight along with a >> few other things, and then I'll reply to this email with links. >> >> ***Not meant as a unit-test primer, just trying to provide some real examples. >> >> Patrick > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-11 2:34 ` Brad Bishop 2018-04-11 15:56 ` Patrick Venture @ 2018-04-12 14:25 ` Alexander Amelkin 2018-04-12 15:20 ` Patrick Venture 1 sibling, 1 reply; 19+ messages in thread From: Alexander Amelkin @ 2018-04-12 14:25 UTC (permalink / raw) To: openbmc 11.04.2018 05:34, Brad Bishop wrote: >> On Apr 9, 2018, at 6:27 PM, Patrick Venture <venture@google.com> wrote: >> >> Everyone, >> >> I'm working on unit-testing in openbmc, and have cracked most of >> sdbusplus into mockable pieces and verified I can in fact test against >> those mocks with a downstream daemon. I'll be grabbing an upstream >> >> ... >> >> YASL (yet-another-shared-library) Request. > Can you talk more about what goes in this? > > Do you envision something like: > > openbmc repo -> upstream project being wrapped > libcmock -> glibc > libstdc++mock -> libstdc++ > libsystemdmock -> libsystemd (or sdbusplusmock) > libfoomock -> libfoo > libbarmock -> libbarmock > Is there going to be support in the standard library to mock linux sysfs for phosphor-hwmon? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-12 14:25 ` Alexander Amelkin @ 2018-04-12 15:20 ` Patrick Venture 2018-04-13 10:16 ` Alexander Amelkin 0 siblings, 1 reply; 19+ messages in thread From: Patrick Venture @ 2018-04-12 15:20 UTC (permalink / raw) To: Alexander Amelkin; +Cc: OpenBMC Maillist On Thu, Apr 12, 2018 at 7:25 AM, Alexander Amelkin <a.amelkin@yadro.com> wrote: > 11.04.2018 05:34, Brad Bishop wrote: >>> On Apr 9, 2018, at 6:27 PM, Patrick Venture <venture@google.com> wrote: >>> >>> Everyone, >>> >>> I'm working on unit-testing in openbmc, and have cracked most of >>> sdbusplus into mockable pieces and verified I can in fact test against >>> those mocks with a downstream daemon. I'll be grabbing an upstream >>> >>> ... >>> >>> YASL (yet-another-shared-library) Request. >> Can you talk more about what goes in this? >> >> Do you envision something like: >> >> openbmc repo -> upstream project being wrapped >> libcmock -> glibc >> libstdc++mock -> libstdc++ >> libsystemdmock -> libsystemd (or sdbusplusmock) >> libfoomock -> libfoo >> libbarmock -> libbarmock >> > Is there going to be support in the standard library to mock linux sysfs > for phosphor-hwmon? Yes, really we just need to mock the path look-ups and file reads. So if they ask to a list of files in a path, we can return whatever list we want. > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-12 15:20 ` Patrick Venture @ 2018-04-13 10:16 ` Alexander Amelkin 2018-04-13 15:42 ` Patrick Venture 0 siblings, 1 reply; 19+ messages in thread From: Alexander Amelkin @ 2018-04-13 10:16 UTC (permalink / raw) To: Patrick Venture; +Cc: OpenBMC Maillist 12.04.2018 18:20, Patrick Venture wrote: > On Thu, Apr 12, 2018 at 7:25 AM, Alexander Amelkin <a.amelkin@yadro.com> wrote: >> 11.04.2018 05:34, Brad Bishop wrote: >>>> On Apr 9, 2018, at 6:27 PM, Patrick Venture <venture@google.com> wrote: >>>> >>>> Everyone, >>>> >>>> I'm working on unit-testing in openbmc, and have cracked most of >>>> sdbusplus into mockable pieces and verified I can in fact test against >>>> those mocks with a downstream daemon. I'll be grabbing an upstream >>>> >>>> ... >>>> >>>> YASL (yet-another-shared-library) Request. >>> Can you talk more about what goes in this? >>> >>> Do you envision something like: >>> >>> openbmc repo -> upstream project being wrapped >>> libcmock -> glibc >>> libstdc++mock -> libstdc++ >>> libsystemdmock -> libsystemd (or sdbusplusmock) >>> libfoomock -> libfoo >>> libbarmock -> libbarmock >>> >> Is there going to be support in the standard library to mock linux sysfs >> for phosphor-hwmon? > Yes, really we just need to mock the path look-ups and file reads. So > if they ask to a list of files in a path, we can return whatever list > we want. Not really. We also need to support file writes that affect contents of other files (writing /sys/class/hwmon/.../pwmX affects the value read from /sys/class/hwmon/.../fanY). Without that some units will fail tests. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-13 10:16 ` Alexander Amelkin @ 2018-04-13 15:42 ` Patrick Venture 0 siblings, 0 replies; 19+ messages in thread From: Patrick Venture @ 2018-04-13 15:42 UTC (permalink / raw) To: Alexander Amelkin; +Cc: OpenBMC Maillist On Fri, Apr 13, 2018 at 3:16 AM, Alexander Amelkin <a.amelkin@yadro.com> wrote: > 12.04.2018 18:20, Patrick Venture wrote: >> On Thu, Apr 12, 2018 at 7:25 AM, Alexander Amelkin <a.amelkin@yadro.com> wrote: >>> 11.04.2018 05:34, Brad Bishop wrote: >>>>> On Apr 9, 2018, at 6:27 PM, Patrick Venture <venture@google.com> wrote: >>>>> >>>>> Everyone, >>>>> >>>>> I'm working on unit-testing in openbmc, and have cracked most of >>>>> sdbusplus into mockable pieces and verified I can in fact test against >>>>> those mocks with a downstream daemon. I'll be grabbing an upstream >>>>> >>>>> ... >>>>> >>>>> YASL (yet-another-shared-library) Request. >>>> Can you talk more about what goes in this? >>>> >>>> Do you envision something like: >>>> >>>> openbmc repo -> upstream project being wrapped >>>> libcmock -> glibc >>>> libstdc++mock -> libstdc++ >>>> libsystemdmock -> libsystemd (or sdbusplusmock) >>>> libfoomock -> libfoo >>>> libbarmock -> libbarmock >>>> >>> Is there going to be support in the standard library to mock linux sysfs >>> for phosphor-hwmon? >> Yes, really we just need to mock the path look-ups and file reads. So >> if they ask to a list of files in a path, we can return whatever list >> we want. > Not really. We also need to support file writes that affect contents of > other files (writing /sys/class/hwmon/.../pwmX affects the value read > from /sys/class/hwmon/.../fanY). Without that some units will fail tests. Yes really. You just need to set the expectation that when it's read it returns the value that it received in the mocked write call. It's a fairly normal pattern. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-09 22:27 YASL Request Patrick Venture 2018-04-10 2:45 ` Lei YU 2018-04-11 2:34 ` Brad Bishop @ 2018-04-13 3:56 ` Andrew Jeffery 2018-04-13 15:21 ` Brad Bishop 2018-04-14 4:12 ` Patrick Venture 2 siblings, 2 replies; 19+ messages in thread From: Andrew Jeffery @ 2018-04-13 3:56 UTC (permalink / raw) To: Patrick Venture, OpenBMC Maillist, Brad Bishop, Emily Shaffer Hi Patrick, On Tue, 10 Apr 2018, at 07:57, Patrick Venture wrote: > Everyone, > > I'm working on unit-testing in openbmc, and have cracked most of > sdbusplus into mockable pieces and verified I can in fact test against > those mocks with a downstream daemon. I'll be grabbing an upstream > daemon (or providing a piece of one) to demonstrate how to leverage > the mocks to test OpenBMC. If one designs with testing in mind, the > designs come out very differently if not, and so getting unit-tests > throughout OpenBMC will be a lot of breaking things apart into > testable pieces. Anyways, where I'm going with this email is that > everything we do within a daemon needs to be something that can be > mocked -- basically. I'm definitely on board with expanding testing in OpenBMC. > > *** > What do I mean specifically? Consider, file access. If a daemon > routes all file accesses throug a common object implementation > provided by a shared library, that shared library can easily also > provide a mock interface for those accesses, such that one can easily > verify behaviors based on file contents without implementing local > files or trying to inject errors. With a mock's file system > interface, you can simply say that a file was unable to be read, or > written, or opened, etc. Another example is mocking ctime. If you > want to test whether something happens after some sleep or period, if > your code can receive a mock version of that library, one can > deliberately control the results of 'time' or 'difftime', etc. I have > to build these interfaces for some of our downstream daemons and > likely for other parts of OpenBMC, and to avoid code duplication it'll > help to have them in some library. > > YASL (yet-another-shared-library) Request. > > Previous conversations along these lines lead to the idea that we need > multiple new libraries for various things. So, this is yet another > use case. The library itself should be written in such a way that it > can be tested via unit-tests, but depending on how thin of a shim it > is, that isn't always practical. See: > > class FileInterface { > public: > virtual int open(const char *filename, int flags) = 0; > }; > > class FileImplementation : public FileInterface { > public: > int open(const char *filename, int flags) override { > return ::open(filename, flags); > } > }; > > class FileMock : public FileInterface { > public: > MOCK_METHOD2(open, int(const char *, int)); > }; > > .... then one just uses the FileInterface for their normally direct > posix-style file access. This can easily wrap iostream, or fstream, > or anything. And then if we provide some libraries for use by > daemons, they can transition over to them over time, and then they get > mocks for free :D For a daemon downstream, I've written a ctime > wrapper, I'll submit it for consideration later tonight along with a > few other things, and then I'll reply to this email with links. > I do wonder whether we can take better advantage of link seams[1] and avoid a lot of the indirection. I don't know how well that interacts with GMock and GTest; the nature of it would tend to eliminate the use of GTest in favour of one binary per test (needing different mocks using the same symbols isn't going to work in a single binary). However, this case is well supported by autotools' `make check` phase, to which you can attach multiple test binaries to execute and mark tests as XFAIL (expected failure) if necessary. Autotools also handles the case where XFAIL tests unexpectedly pass (which fails test suite). I've used the link-seam technique for testing the mboxbridge and phosphor-mboxd repositories (ignore that we've forked ourselves for the moment). Now admittedly a lot of the tests in both of those repositories are *integration* tests, not *unit* tests, but the point at which I've injected my mocks via link seams still allows me to control the environment as I require. Some advantages I've found to this technique are: * There's no runtime overhead * There's no reduction of readability in the code (though it has an impact on the size of the build system configuration) * You get test binaries that you can run independent of your test framework, as there isn't really a test framework, just autotools running your test binaries in parallel * By extension, if you need to debug a failing test case, you can gdb the test binary directly without needing to comprehend the side-effects of the test framework on your test binary Some disadvantages of what I've got so far: * There is no fancy way of testing expectations, I'm just using assert(). The EXPECT_*() and ASSERT_*() macros from GTEST are nice readability improvements. * I'm implementing the mocks without outside help. [1] http://www.informit.com/articles/article.aspx?p=359417&seqNum=3 Not sure if it's going to fly generally, but this approach has been working well for me so far. Cheers, Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-13 3:56 ` Andrew Jeffery @ 2018-04-13 15:21 ` Brad Bishop 2018-04-13 16:03 ` Patrick Venture 2018-04-16 2:50 ` Lei YU 2018-04-14 4:12 ` Patrick Venture 1 sibling, 2 replies; 19+ messages in thread From: Brad Bishop @ 2018-04-13 15:21 UTC (permalink / raw) To: Andrew Jeffery; +Cc: Patrick Venture, OpenBMC Maillist, Emily Shaffer > On Apr 12, 2018, at 11:56 PM, Andrew Jeffery <andrew@aj.id.au> wrote: > > > Some advantages I've found to this technique are: > > * There's no reduction of readability in the code This. > * You get test binaries that you can run independent of your test framework, as there isn't really a test framework, just autotools running your test binaries in parallel > * By extension, if you need to debug a failing test case, you can gdb the test binary directly without needing to comprehend the side-effects of the test framework on your test binary And this. Honestly these are my _real_ objections but I brought up runtime overhead because that is not subjective. I just don’t like how the direction we are headed obfuscates things for developers that have a good understanding of standard libraries, tools, and language features (libc, libstd++, STL, GDB, etc...) and don’t have an understanding of the internals of GMock or the “techniques" its use drives into the code. I expect there to be way more of the former (those up to speed on the standard libraries, tools, language features) than the latter (those comfortable working on a GMock inspired codebase). IMHO these concepts have a greater (positive) impact on the project overall than super-easy-unit-test writing. I’d guess I’m in the minority, but hey…I have to speak my mind right? I can certainly relate to the opposing viewpoint. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-13 15:21 ` Brad Bishop @ 2018-04-13 16:03 ` Patrick Venture 2018-04-14 0:14 ` Patrick Venture 2018-04-16 2:50 ` Lei YU 1 sibling, 1 reply; 19+ messages in thread From: Patrick Venture @ 2018-04-13 16:03 UTC (permalink / raw) To: Brad Bishop; +Cc: Andrew Jeffery, OpenBMC Maillist, Emily Shaffer On Fri, Apr 13, 2018 at 8:21 AM, Brad Bishop <bradleyb@fuzziesquirrel.com> wrote: > >> On Apr 12, 2018, at 11:56 PM, Andrew Jeffery <andrew@aj.id.au> wrote: >> >> >> Some advantages I've found to this technique are: >> >> * There's no reduction of readability in the code > > This. > >> * You get test binaries that you can run independent of your test framework, as there isn't really a test framework, just autotools running your test binaries in parallel >> * By extension, if you need to debug a failing test case, you can gdb the test binary directly without needing to comprehend the side-effects of the test framework on your test binary > > And this. > > Honestly these are my _real_ objections but I brought up runtime overhead because > that is not subjective. I just don’t like how the direction we are headed > obfuscates things for developers that have a good understanding of standard libraries, > tools, and language features (libc, libstd++, STL, GDB, etc...) and don’t have an > understanding of the internals of GMock or the “techniques" its use drives into the > code. I expect there to be way more of the former (those up to speed on the standard > libraries, tools, language features) than the latter (those comfortable working on > a GMock inspired codebase). I find a flaw in this logic that, because a developer would need to learn something new, because as you say few people will be familiar with gmock. Every developer I've met tends to learn new things all the time, and I specifically hadn't used c++ since 2004, and basically learned it to contribute to OpenBMC, and I doubt I'd be an exception to this. > > IMHO these concepts have a greater (positive) impact on the project overall than > super-easy-unit-test writing. I’d guess I’m in the minority, but hey…I have to > speak my mind right? I can certainly relate to the opposing viewpoint. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-13 16:03 ` Patrick Venture @ 2018-04-14 0:14 ` Patrick Venture 0 siblings, 0 replies; 19+ messages in thread From: Patrick Venture @ 2018-04-14 0:14 UTC (permalink / raw) To: Brad Bishop; +Cc: Andrew Jeffery, OpenBMC Maillist, Emily Shaffer One can also use a fake-file-system framework to do some testing if you need to create a bunch of local temp files. We have this internally, there is probably something similar on the outside. On Fri, Apr 13, 2018 at 9:03 AM, Patrick Venture <venture@google.com> wrote: > On Fri, Apr 13, 2018 at 8:21 AM, Brad Bishop > <bradleyb@fuzziesquirrel.com> wrote: >> >>> On Apr 12, 2018, at 11:56 PM, Andrew Jeffery <andrew@aj.id.au> wrote: >>> >>> >>> Some advantages I've found to this technique are: >>> >>> * There's no reduction of readability in the code >> >> This. >> >>> * You get test binaries that you can run independent of your test framework, as there isn't really a test framework, just autotools running your test binaries in parallel >>> * By extension, if you need to debug a failing test case, you can gdb the test binary directly without needing to comprehend the side-effects of the test framework on your test binary >> >> And this. >> >> Honestly these are my _real_ objections but I brought up runtime overhead because >> that is not subjective. I just don’t like how the direction we are headed >> obfuscates things for developers that have a good understanding of standard libraries, >> tools, and language features (libc, libstd++, STL, GDB, etc...) and don’t have an >> understanding of the internals of GMock or the “techniques" its use drives into the >> code. I expect there to be way more of the former (those up to speed on the standard >> libraries, tools, language features) than the latter (those comfortable working on >> a GMock inspired codebase). > > I find a flaw in this logic that, because a developer would need to > learn something new, because as you say few people will be familiar > with gmock. Every developer I've met tends to learn new things all > the time, and I specifically hadn't used c++ since 2004, and basically > learned it to contribute to OpenBMC, and I doubt I'd be an exception > to this. > >> >> IMHO these concepts have a greater (positive) impact on the project overall than >> super-easy-unit-test writing. I’d guess I’m in the minority, but hey…I have to >> speak my mind right? I can certainly relate to the opposing viewpoint. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-13 15:21 ` Brad Bishop 2018-04-13 16:03 ` Patrick Venture @ 2018-04-16 2:50 ` Lei YU 2018-04-16 19:10 ` Patrick Venture 1 sibling, 1 reply; 19+ messages in thread From: Lei YU @ 2018-04-16 2:50 UTC (permalink / raw) To: Brad Bishop Cc: Andrew Jeffery, Emily Shaffer, Patrick Venture, OpenBMC Maillist On Fri, Apr 13, 2018 at 11:21 PM, Brad Bishop <bradleyb@fuzziesquirrel.com> wrote: > >> On Apr 12, 2018, at 11:56 PM, Andrew Jeffery <andrew@aj.id.au> wrote: >> >> >> Some advantages I've found to this technique are: >> >> * There's no reduction of readability in the code > > This. > >> * You get test binaries that you can run independent of your test framework, as there isn't really a test framework, just autotools running your test binaries in parallel >> * By extension, if you need to debug a failing test case, you can gdb the test binary directly without needing to comprehend the side-effects of the test framework on your test binary > By using gtest/gmock, it is the same that the test binary is generated and run by autotools. You can use gdb to debug the failing tests as well. > And this. > > Honestly these are my _real_ objections but I brought up runtime overhead because > that is not subjective. I just don’t like how the direction we are headed > obfuscates things for developers that have a good understanding of standard libraries, > tools, and language features (libc, libstd++, STL, GDB, etc...) and don’t have an > understanding of the internals of GMock or the “techniques" its use drives into the > code. I expect there to be way more of the former (those up to speed on the standard > libraries, tools, language features) than the latter (those comfortable working on > a GMock inspired codebase). I get your point. And I have to admit that generally developers tend to *directly* use standard libraries because they are familiar with that. But I still want to point that by writing code that way: 1. It makes the code harder to test and thus likely to contain potential un-found bugs; 2. It is possible to write code that put too much function in some level, if the developer does not group the code well. By using gmock, the developer is *forced* to split the code into pieces of interfaces and functions, otherwise the code is un-testable. This helps on grouping and writing testable code, and since it's tested it is likely to reduce the number of potential bugs. Althoug it does introduce runtime overhead, I expecte the overhead is tiny in most cases. I know you are not in favor of writing code that way, but I do prefer testable code than the other. In my previous experience, the unit test could cover most major logic of the code to make sure the code is (at some level) correct and robust. > > IMHO these concepts have a greater (positive) impact on the project overall than > super-easy-unit-test writing. I’d guess I’m in the minority, but hey…I have to > speak my mind right? I can certainly relate to the opposing viewpoint. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-16 2:50 ` Lei YU @ 2018-04-16 19:10 ` Patrick Venture 0 siblings, 0 replies; 19+ messages in thread From: Patrick Venture @ 2018-04-16 19:10 UTC (permalink / raw) To: Lei YU; +Cc: Brad Bishop, Andrew Jeffery, Emily Shaffer, OpenBMC Maillist Because there are many patches enabling this, here is the order: https://gerrit.openbmc-project.xyz/#/c/10109/ https://gerrit.openbmc-project.xyz/#/c/10129/ https://gerrit.openbmc-project.xyz/#/c/10157/ https://gerrit.openbmc-project.xyz/#/c/10159/ https://gerrit.openbmc-project.xyz/#/c/10052/ https://gerrit.openbmc-project.xyz/#/c/10166/ I need to rebase and push the last one so it won't require a merge, but that's not urgent. I'll be adding at least two more patches after lunch which will further propagate the SdBusInterface that enables testing without requiring more injections. If you're curious what the tests look like so far: https://gerrit.openbmc-project.xyz/#/c/10048/ Patrick On Sun, Apr 15, 2018 at 7:50 PM, Lei YU <mine260309@gmail.com> wrote: > On Fri, Apr 13, 2018 at 11:21 PM, Brad Bishop > <bradleyb@fuzziesquirrel.com> wrote: >> >>> On Apr 12, 2018, at 11:56 PM, Andrew Jeffery <andrew@aj.id.au> wrote: >>> >>> >>> Some advantages I've found to this technique are: >>> >>> * There's no reduction of readability in the code >> >> This. >> >>> * You get test binaries that you can run independent of your test framework, as there isn't really a test framework, just autotools running your test binaries in parallel >>> * By extension, if you need to debug a failing test case, you can gdb the test binary directly without needing to comprehend the side-effects of the test framework on your test binary >> > > By using gtest/gmock, it is the same that the test binary is generated and run > by autotools. You can use gdb to debug the failing tests as well. > >> And this. >> >> Honestly these are my _real_ objections but I brought up runtime overhead because >> that is not subjective. I just don’t like how the direction we are headed >> obfuscates things for developers that have a good understanding of standard libraries, >> tools, and language features (libc, libstd++, STL, GDB, etc...) and don’t have an >> understanding of the internals of GMock or the “techniques" its use drives into the >> code. I expect there to be way more of the former (those up to speed on the standard >> libraries, tools, language features) than the latter (those comfortable working on >> a GMock inspired codebase). > > I get your point. And I have to admit that generally developers tend to > *directly* use standard libraries because they are familiar with that. > But I still want to point that by writing code that way: > 1. It makes the code harder to test and thus likely to contain potential > un-found bugs; > 2. It is possible to write code that put too much function in some level, > if the developer does not group the code well. > > By using gmock, the developer is *forced* to split the code into pieces of > interfaces and functions, otherwise the code is un-testable. > This helps on grouping and writing testable code, and since it's tested it is > likely to reduce the number of potential bugs. > Althoug it does introduce runtime overhead, I expecte the overhead is tiny in > most cases. > > I know you are not in favor of writing code that way, but I do prefer testable > code than the other. > In my previous experience, the unit test could cover most major logic of the > code to make sure the code is (at some level) correct and robust. > >> >> IMHO these concepts have a greater (positive) impact on the project overall than >> super-easy-unit-test writing. I’d guess I’m in the minority, but hey…I have to >> speak my mind right? I can certainly relate to the opposing viewpoint. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: YASL Request 2018-04-13 3:56 ` Andrew Jeffery 2018-04-13 15:21 ` Brad Bishop @ 2018-04-14 4:12 ` Patrick Venture 1 sibling, 0 replies; 19+ messages in thread From: Patrick Venture @ 2018-04-14 4:12 UTC (permalink / raw) To: Andrew Jeffery; +Cc: OpenBMC Maillist, Brad Bishop, Emily Shaffer On Thu, Apr 12, 2018 at 8:56 PM, Andrew Jeffery <andrew@aj.id.au> wrote: > Hi Patrick, > > On Tue, 10 Apr 2018, at 07:57, Patrick Venture wrote: >> Everyone, >> >> I'm working on unit-testing in openbmc, and have cracked most of >> sdbusplus into mockable pieces and verified I can in fact test against >> those mocks with a downstream daemon. I'll be grabbing an upstream >> daemon (or providing a piece of one) to demonstrate how to leverage >> the mocks to test OpenBMC. If one designs with testing in mind, the >> designs come out very differently if not, and so getting unit-tests >> throughout OpenBMC will be a lot of breaking things apart into >> testable pieces. Anyways, where I'm going with this email is that >> everything we do within a daemon needs to be something that can be >> mocked -- basically. > > I'm definitely on board with expanding testing in OpenBMC. > >> >> *** >> What do I mean specifically? Consider, file access. If a daemon >> routes all file accesses throug a common object implementation >> provided by a shared library, that shared library can easily also >> provide a mock interface for those accesses, such that one can easily >> verify behaviors based on file contents without implementing local >> files or trying to inject errors. With a mock's file system >> interface, you can simply say that a file was unable to be read, or >> written, or opened, etc. Another example is mocking ctime. If you >> want to test whether something happens after some sleep or period, if >> your code can receive a mock version of that library, one can >> deliberately control the results of 'time' or 'difftime', etc. I have >> to build these interfaces for some of our downstream daemons and >> likely for other parts of OpenBMC, and to avoid code duplication it'll >> help to have them in some library. >> >> YASL (yet-another-shared-library) Request. >> >> Previous conversations along these lines lead to the idea that we need >> multiple new libraries for various things. So, this is yet another >> use case. The library itself should be written in such a way that it >> can be tested via unit-tests, but depending on how thin of a shim it >> is, that isn't always practical. See: >> >> class FileInterface { >> public: >> virtual int open(const char *filename, int flags) = 0; >> }; >> >> class FileImplementation : public FileInterface { >> public: >> int open(const char *filename, int flags) override { >> return ::open(filename, flags); >> } >> }; >> >> class FileMock : public FileInterface { >> public: >> MOCK_METHOD2(open, int(const char *, int)); >> }; >> >> .... then one just uses the FileInterface for their normally direct >> posix-style file access. This can easily wrap iostream, or fstream, >> or anything. And then if we provide some libraries for use by >> daemons, they can transition over to them over time, and then they get >> mocks for free :D For a daemon downstream, I've written a ctime >> wrapper, I'll submit it for consideration later tonight along with a >> few other things, and then I'll reply to this email with links. >> > > I do wonder whether we can take better advantage of link seams[1] and avoid a lot of the indirection. I don't know how well that interacts with GMock and GTest; the nature of it would tend to eliminate the use of GTest in favour of one binary per test (needing different mocks using the same symbols isn't going to work in a single binary). However, this case is well supported by autotools' `make check` phase, to which you can attach multiple test binaries to execute and mark tests as XFAIL (expected failure) if necessary. Autotools also handles the case where XFAIL tests unexpectedly pass (which fails test suite). > > I've used the link-seam technique for testing the mboxbridge and phosphor-mboxd repositories (ignore that we've forked ourselves for the moment). Now admittedly a lot of the tests in both of those repositories are *integration* tests, not *unit* tests, but the point at which I've injected my mocks via link seams still allows me to control the environment as I require. > > Some advantages I've found to this technique are: > > * There's no runtime overhead > * There's no reduction of readability in the code (though it has an impact on the size of the build system configuration) > * You get test binaries that you can run independent of your test framework, as there isn't really a test framework, just autotools running your test binaries in parallel > * By extension, if you need to debug a failing test case, you can gdb the test binary directly without needing to comprehend the side-effects of the test framework on your test binary > > Some disadvantages of what I've got so far: > > * There is no fancy way of testing expectations, I'm just using assert(). The EXPECT_*() and ASSERT_*() macros from GTEST are nice readability improvements. > * I'm implementing the mocks without outside help. > > [1] http://www.informit.com/articles/article.aspx?p=359417&seqNum=3 > > Not sure if it's going to fly generally, but this approach has been working well for me so far. I took a quick look at the article and its use cases and based on what you've described, it's how I would have implemented it if I had just done it myself without a framework. To do true unit-testing, there is a need for true isolation and that's something we get with cunit, pyunit, and gmock. The readability of being able to set the expectations and values are very helpful because once you have hundreds of unit-tests, they themselves become something which require maintenance. I think there's a lot of value in setting up a framework that people can just use with all phosphor daemons with a consistent experience. > > Cheers, > > Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-04-16 19:11 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-09 22:27 YASL Request Patrick Venture 2018-04-10 2:45 ` Lei YU 2018-04-10 3:19 ` Patrick Venture 2018-04-10 17:59 ` Patrick Venture 2018-04-11 3:52 ` Brad Bishop 2018-04-11 15:59 ` Patrick Venture 2018-04-11 2:34 ` Brad Bishop 2018-04-11 15:56 ` Patrick Venture 2018-04-12 14:25 ` Alexander Amelkin 2018-04-12 15:20 ` Patrick Venture 2018-04-13 10:16 ` Alexander Amelkin 2018-04-13 15:42 ` Patrick Venture 2018-04-13 3:56 ` Andrew Jeffery 2018-04-13 15:21 ` Brad Bishop 2018-04-13 16:03 ` Patrick Venture 2018-04-14 0:14 ` Patrick Venture 2018-04-16 2:50 ` Lei YU 2018-04-16 19:10 ` Patrick Venture 2018-04-14 4:12 ` Patrick Venture
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.