sysrepo / sysrepo-cpp

C++ bindings for the sysrepo library
BSD 3-Clause "New" or "Revised" License
6 stars 5 forks source link

GoogleTest as unit test framework #18

Closed rbu9fe closed 7 months ago

rbu9fe commented 7 months ago

Thanks for this project, it helps a lot. However, we're having a hard time in using it in our development framework as we rely on GoogleTest for unit testing. So far it only works when we use an entirely different Session class when testing. It would be very nice if GoogleTest were supported natively. Is there a change for this to be supported in the future?

jktjkt commented 7 months ago

I'm not familiar with Google Test, and I don't know exactly how you're using it, but as long as the test code does "the right thing" with lifetimes of the C++ objects, it should work just fine. Can you post a minimal example which shows what exactly does not work, and add a few comments on how you expect that thing to behave?

rbu9fe commented 7 months ago

Specifically I want to mock a Session object in order to control it's behaviour while testing. There are 3 ways how to achieve this:

  1. Runtime dependency injection 1: Instantiate either a productive or test object both deriving from an abstract virtual interface class, e.g. ISession. Then an ISession object can be injected and used by the code under test.
  2. Runtime dependency injection 2: Same as 1. but the test object derives from the productive class Session overriding virtual functions. That's the only runtime option in case Session doesn't derive from an abstract interface class, which is the case here.
  3. Compile time: Simply replace the Session definition by something else during compile time.

Option 1 is not applicable as Session does not derive from an abstract interface class. Option 2 is not applicable as Session's functions are non-virtual. Option 3 works but is complicated.

So the simplest way to support Google Mock would be to define an abstract interface class and let Session derive from it. That's also what's the most recommended way in the Google's mock cook book.

jktjkt commented 7 months ago

Thanks for explaining that. We're using a different pattern in tests of our projects that use sysrepo-cpp. We decided not to mock the actual sysrepo interface, but we're instead creating a real sysrepo-level subscriber that forwards any modifications and received RPC calls into a stub object. Then we set up expectations on function calls of the stub. For example, if we know that the code that we're testing is supposed to set leaf A to value 123, and then call an RPC xyz, we would configure our mocked stub class to expect a call of function created with arguments A and 123, followed by a call to function rpc with parameters xyz, etc.

That way, we use the real sysrepo for the actual test. This has proven very valuable for us and it caught many bugs over the years. In our experience the performance penalty is negligible, the tests still run in parallel (with SHM prefixes, separate sysrepo paths, etc).

Is there any reason why you want to avoid going through the real sysrepo IPC layer?

rbu9fe commented 7 months ago

Thanks for your detailed response. The approach you're following is of course very useful to test the cpp wrapper as well as sysrepo itself. For unit tests we follow the aproach of cutting dependencies by mock objects to improve test performance and to not test the same thing over and over again in different layers. Furthermore, our unit tests run in a Docker container that doesn't have a sysrepo environment (so far).

jktjkt commented 7 months ago

Thanks for the clarification. In the end we decided to not add this interface (a few abstract classes) directly to the library. Those who prefer to mock calls to sysrepo-cpp can add a layer on their own.