Skip to content

Update SetInteriorVehicleDataRequestTests #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mvorobio wants to merge 3 commits into from
Closed

Update SetInteriorVehicleDataRequestTests #2

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9808e0c
Update SetInteriorVehicleDataRequestTests
mvorobio Apr 2, 2020
794597b
fixup! Update SetInteriorVehicleDataRequestTests
mvorobio Apr 16, 2020
30f301a
fixup! Update SetInteriorVehicleDataRequestTests
mvorobio Apr 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 35 additions & 29 deletions ...manager/rpc_plugins/rc_rpc_plugin/test/commands/set_interior_vehicle_data_request_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@

using application_manager::ApplicationSet;
using application_manager::commands::MessageSharedPtr;
using rc_rpc_plugin::commands::SetInteriorVehicleDataRequest;
using test::components::application_manager_test::MockApplication;
using test::components::application_manager_test::MockApplicationManager;
using test::components::commands_test::CommandRequestTest;
Expand All @@ -56,6 +57,7 @@ using test::components::commands_test::HMIResultCodeIs;
using ::testing::_;
using ::testing::NiceMock;
using ::testing::Return;
using ::testing::ReturnRef;
using ::testing::SaveArg;

namespace {
Expand Down Expand Up @@ -155,55 +157,58 @@ class SetInteriorVehicleDataRequestTest

TEST_F(SetInteriorVehicleDataRequestTest,
Execute_ValidWithoutReadOnlyParams_ExpectResendToHMI) {
using namespace rc_rpc_plugin::message_params;

// Arrange
MessageSharedPtr mobile_message = CreateBasicMessage();
ns_smart_device_link::ns_smart_objects::SmartObject& msg_params =
auto mobile_message = CreateBasicMessage();
auto& msg_params =
(*mobile_message)[application_manager::strings::msg_params];
msg_params[message_params::kModuleData][message_params::kModuleType] =
mobile_apis::ModuleType::CLIMATE;
smart_objects::SmartObject climate_control_data(smart_objects::SmartType_Map);
climate_control_data[message_params::kFanSpeed] = 10;
msg_params[kModuleData][kModuleType] = mobile_apis::ModuleType::CLIMATE;
auto& control_data = msg_params[kModuleData][kClimateControlData];
const uint64_t fan_speed = 10u;
control_data[kFanSpeed] = fan_speed;

msg_params[message_params::kModuleData][message_params::kClimateControlData] =
climate_control_data;
// Expectations
EXPECT_CALL(mock_policy_handler_, CheckModule(kPolicyAppId, _))
.WillOnce(Return(rc_rpc_plugin::TypeAccess::kAllowed));
.WillOnce(Return(true));

EXPECT_CALL(
mock_rpc_service_,
ManageHMICommand(
HMIResultCodeIs(hmi_apis::FunctionID::RC_SetInteriorVehicleData), _))
.WillOnce(Return(true));
// Act
auto command =
CreateRCCommand<rc_rpc_plugin::commands::SetInteriorVehicleDataRequest>(
mobile_message);
auto command = CreateRCCommand<SetInteriorVehicleDataRequest>(mobile_message);
ASSERT_TRUE(command->Init());
command->Run();

EXPECT_TRUE(control_data.keyExists(kFanSpeed) &&
Copy link

@AByzhynar AByzhynar Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvorobio please transform this expression to 2 separate expectations.

Copy link
Owner Author

@mvorobio mvorobio Apr 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AByzhynar this is a single expectation - just ensures that a key exists prior an attempt to retrieve a value.

control_data[kFanSpeed].asUInt() == fan_speed);
}

TEST_F(
SetInteriorVehicleDataRequestTest,
Execute_ValidWithSettableAndReadOnlyParams_ExpectCutReadOnlyAndResendToHMI) {
// Arrange
Copy link

@AByzhynar AByzhynar Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvorobio What is the reason to remove such comments?

Copy link
Owner Author

@mvorobio mvorobio Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AByzhynar These comments are useless enough, because a pattern they introduce is sometimes difficult to follow. E.g. expectations from the ASSERT section could be placed around the ACT section.
In other words, such comments rarely give useful info but add unnecessary lines of code and sometimes even confuse.
But it's my personal opinion only. I wouldn't argue if these comments are required by some reasons.

Copy link

@AByzhynar AByzhynar Apr 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvorobio Please return them back. It is the full responsibility of the developer to put the code to the right section.

Copy link
Owner Author

@mvorobio mvorobio Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AByzhynar please check 794597b

using namespace rc_rpc_plugin::message_params;

MessageSharedPtr mobile_message = CreateBasicMessage();
ns_smart_device_link::ns_smart_objects::SmartObject& msg_params =
// Arrange
auto mobile_message = CreateBasicMessage();
auto& msg_params =
(*mobile_message)[application_manager::strings::msg_params];
msg_params[message_params::kModuleData][message_params::kModuleType] =
mobile_apis::ModuleType::RADIO;
smart_objects::SmartObject radio_control_data(smart_objects::SmartType_Map);
radio_control_data[message_params::kState] = true;
radio_control_data[message_params::kRadioEnable] = true;
msg_params[message_params::kModuleData][message_params::kRadioControlData] =
radio_control_data;
msg_params[kModuleData][kModuleType] = mobile_apis::ModuleType::RADIO;
auto& control_data = msg_params[kModuleData][kRadioControlData];
control_data[kState] = true;
const bool radio_enable = true;
control_data[kRadioEnable] = radio_enable;

ON_CALL(mock_rc_capabilities_manager_, ControlDataForType(_, _))
.WillByDefault(ReturnRef(control_data));
ON_CALL(mock_rc_capabilities_manager_, AreReadOnlyParamsPresent(_, _, _))
.WillByDefault(Return(true));

// Expectations
Copy link

@AByzhynar AByzhynar Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvorobio What is the reason to remove such comments?

Copy link
Owner Author

@mvorobio mvorobio Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AByzhynar please check 794597b

EXPECT_CALL(mock_policy_handler_, CheckModule(kPolicyAppId, _))
.WillOnce(Return(rc_rpc_plugin::TypeAccess::kAllowed));

EXPECT_CALL(app_mngr_, RemoveHMIFakeParameters(_, _));
.WillOnce(Return(true));

EXPECT_CALL(
mock_rpc_service_,
Expand All @@ -212,12 +217,13 @@ TEST_F(
.WillOnce(Return(true));

// Act
Copy link

@AByzhynar AByzhynar Apr 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvorobio What is the reason to remove such comments?

Copy link
Owner Author

@mvorobio mvorobio Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AByzhynar please check 794597b

std::shared_ptr<rc_rpc_plugin::commands::SetInteriorVehicleDataRequest>
command = CreateRCCommand<
rc_rpc_plugin::commands::SetInteriorVehicleDataRequest>(
mobile_message);
auto command = CreateRCCommand<SetInteriorVehicleDataRequest>(mobile_message);
ASSERT_TRUE(command->Init());
command->Run();

EXPECT_FALSE(control_data.keyExists(kState));
EXPECT_TRUE(control_data.keyExists(kRadioEnable) &&
Copy link

@AByzhynar AByzhynar Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvorobio please transform this expression to 2 separate expectations.

Copy link
Owner Author

@mvorobio mvorobio Apr 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AByzhynar please see above.

control_data[kRadioEnable].asBool() == radio_enable);
}

TEST_F(
Expand Down