indoor-navigation-system icon indicating copy to clipboard operation
indoor-navigation-system copied to clipboard

Low ins-service (server) coverage

Open platisd opened this issue 7 years ago • 9 comments

Description

INS service has low code coverage (i.e. ins_service.cpp) which can result into defects and low internal quality.

Definition of Done

The code coverage is raised to at least 70%. This could require some code refactoring.

platisd avatar Dec 31 '17 17:12 platisd

Current coverage can be seen in codecov.

platisd avatar Dec 31 '17 17:12 platisd

I had a look today and I believe we need to use a mock of Pistache instead of Pistache itself which is being linked in our test cmake, in order to check those callbacks. I don't think (many) production code changes will be necessary. For example, once we mock Pistache functions, i.e. routes::post and get (I don't remember if these are the exact names, just making a point) we can use gtest's SaveArg to save the private callback in a local std::function variable and then call it directly from our test.

platisd avatar Jan 02 '18 14:01 platisd

My idea is to deploy the following tactic:

Create the necessary header files (e.g. pistache/endpoint.h) which will practically be stubs that are just calling a PistacheMock singleton. Then in our tests we can state our expectations on our singleton mock which will be indirectly used by the production code.

It is the same method used for creating mocks for the ins-node.

platisd avatar Jan 09 '18 23:01 platisd

I will have to take a look at how it works in ins-node to fully grab the concept, but it sounds like a good approach. I have not had much time to settle down with the project lately :disappointed: Have to do something about that.

samup4web avatar Jan 10 '18 06:01 samup4web

No worries! These are mental notes mainly for myself. :+1: 😃

platisd avatar Jan 10 '18 06:01 platisd

I was looking at this yesterday as well and I think I am going the wrong way. I noticed the wrapper_lib is where the Pistache library should be abstracted into if we are to follow the same pattern that we've been using so far for testing. What do you think?

platisd avatar Mar 17 '18 08:03 platisd

Well, that is the pattern I thought would work best. It is a simple one as well, the only overhead is that it requires a wrapper class and function for each calls we intend to mock/test, and also the production code has to be modified for the testing purpose as it has to use the wrapper instead of directly using the library. If I remember correctly, this almost worked until I had an strange issue with some object instance in the library. I think it has to do with Pistache::Rest::Request or the Pistache::Rest::Routes. Actually, it will be interesting to revisit this problem again.

samup4web avatar Mar 17 '18 22:03 samup4web

is this still an issue?

lexious89 avatar Jul 19 '18 15:07 lexious89

Yeap :(

platisd avatar Jul 19 '18 15:07 platisd