YASL Request

YASL Request

[Patrick Venture]

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

Re: YASL Request

[Lei YU]

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

Re: YASL Request

[Patrick Venture]

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

Re: YASL Request

[Patrick Venture]

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

Re: YASL Request

[Brad Bishop]

> 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

Re: YASL Request

[Patrick Venture]

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

Re: YASL Request

[Brad Bishop]

> 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

Re: YASL Request

[Patrick Venture]

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
>
>

Re: YASL Request

[Alexander Amelkin]

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?

Re: YASL Request

[Patrick Venture]

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.

>

Re: YASL Request

[Alexander Amelkin]

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.

Re: YASL Request

[Patrick Venture]

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.

Re: YASL Request

[Andrew Jeffery]

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

Re: YASL Request

[Brad Bishop]

> 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.

Re: YASL Request

[Patrick Venture]

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.

Re: YASL Request

[Patrick Venture]

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.

Re: YASL Request

[Lei YU]

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.

Re: YASL Request

[Patrick Venture]

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.

Re: YASL Request

[Patrick Venture]

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