Add RPCService unit tests #3250
Conversation
ychernysheva
requested review from
LuxoftAKutsan,
LitvinenkoIra and
YarikMamykin
ychernysheva
force-pushed
the
feature/rpc_service_impl_uts_coverage
branch
2 times, most recently
from
42f6d01 to
af9f896
Compare
| */ | ||
|
|
||
| #include <gmock/gmock.h> | ||
| #include <string> |
There was a problem hiding this comment.
@ychernysheva reorder includes
There was a problem hiding this comment.
Done in commit 09fa0bd
| #include "application_manager/request_controller.h" | ||
| #include "hmi_message_handler/mock_hmi_message_handler.h" | ||
| #include "include/test/protocol_handler/mock_protocol_handler.h" | ||
| #include "resumption/last_state_impl.h" |
There was a problem hiding this comment.
Check that all includes are actually required
There was a problem hiding this comment.
Done in commit 09fa0bd
| @@ -17,7 +17,7 @@ rm -rf $COVERAGE_DIR | |||
| rm -rf $REPORTS_DIR - | |||
|
|
|||
| mkdir $COVERAGE_DIR | |||
| lcov --quiet --capture --directory . --output-file $COVERAGE_DIR/full_report.info | |||
| lcov --quiet --capture --directory $BUILD_DIR --output-file $COVERAGE_DIR/full_report.info | |||
There was a problem hiding this comment.
This change is not related to your current PR
There was a problem hiding this comment.
There is no such file in this pull request
| const uint32_t kCorrelationId = 1u; | ||
| const uint32_t kFunctionId = 1u; | ||
| const uint32_t kAppId = 1u; | ||
| const int32_t hmi_protocol_type_ = 1; |
There was a problem hiding this comment.
Not used variable - please delete
There was a problem hiding this comment.
Done in commit d5ec824
| const uint32_t kFunctionId = 1u; | ||
| const uint32_t kAppId = 1u; | ||
| const int32_t hmi_protocol_type_ = 1; | ||
| const int32_t mobile_protocol_type_ = 0; |
There was a problem hiding this comment.
Wrong naming, please take a look at constants names above as an example
There was a problem hiding this comment.
Done in commit d5ec824
| am::CommandHolder::CommandType::kHmiCommand, | ||
| source, | ||
| message)) | ||
| .WillOnce(Return()); |
There was a problem hiding this comment.
Did you run check_style script?
There was a problem hiding this comment.
Yes, and I get such result, I need to edit it manually in addition?
There was a problem hiding this comment.
@ychernysheva no. If script did this then this is the way.
| new_function_id, message_type, members); | ||
| CSmartSchema updated_schema; | ||
| extended_mobile_api.GetSchema(new_function_id, message_type, updated_schema); | ||
| // Check that schema is changed |
There was a problem hiding this comment.
Place this comment closer to line of code it is related to.
Please check all similar places in file
There was a problem hiding this comment.
Done in commit d5ec824
| (*message)[jhs::S_PARAMS][jhs::S_MESSAGE_TYPE] = | ||
| mobile_apis::messageType::request; | ||
| (*message)[jhs::S_PARAMS][jhs::S_PROTOCOL_TYPE] = mobile_protocol_type_; | ||
| (*message)[jhs::S_PARAMS][jhs::S_PROTOCOL_VERSION] = 1; |
There was a problem hiding this comment.
Don't use 'magic numbers', use a constant here and declare it in anonymous namespace at the top of file.
Please check all similar places across file.
There was a problem hiding this comment.
Done in commit d5ec824
| message_to_send->set_connection_key(kConnectionKey); | ||
| message_to_send->set_correlation_id(kCorrelationId); | ||
| message_to_send->set_function_id(kFunctionId); | ||
| message_to_send->set_message_type(static_cast<am::MessageType>(2)); |
There was a problem hiding this comment.
Magic number. Please provide concrete enum value or make an appropriate constant.
There was a problem hiding this comment.
Done in commit d5ec824
ychernysheva
force-pushed
the
feature/rpc_service_impl_uts_coverage
branch
from
d5ec824 to
4d2aae7
Compare
|
Rebased on current release branch |
|
@theresalech this PR is ready for review. Please notice, that this is not a regression issue, so please check it if time allows. Also notice, that this branch was rebased on release/6.1.0 branch, but develop branch is a target for merging. |
…e_impl_uts_coverage
Fixes #2449
This PR is [ready] for review.
Risk
This PR makes [no] API changes.
Summary
Add RPCService unit tests to improve unit tests coverage (from 59.1% to 95.5% (by functions) and from 27.9% to 69.8% (by lines)).
CLA