YASL Request

[Brad Bishop]

> On Apr 9, 2018, at 10:45 PM, Lei YU <mine260309 at gmail.com> wrote:
> 
> On Tue, Apr 10, 2018 at 6:27 AM, Patrick Venture <venture at 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