Skip to content
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

Jump to bottom

Feature/New vehicle data hands off steering #6

Merged
merged 3 commits into from
May 22, 2020
Merged

Feature/New vehicle data hands off steering #6

merged 3 commits into from
May 22, 2020

Conversation

TMyza
Copy link

@TMyza TMyza commented Apr 29, 2020

ATF Test Scripts to check #3126

This PR is [ready] for review.

Summary

Scripts for "New vehicle data hands off steering" feature

ATF version

develop

CLA

GetmanetsIrina
GetmanetsIrina requested changes May 8, 2020
View reviewed changes
@GetmanetsIrina
Copy link

Please pay attention to comments that are written in some one place but applicable for a lot of another scripts

GetmanetsIrina
GetmanetsIrina requested changes May 14, 2020
View reviewed changes
GetmanetsIrina
GetmanetsIrina requested changes May 19, 2020
View reviewed changes
...cripts/API/VehicleData/HandsOffSteering/032_UnsubscribeVD_App_1_allowed_App_2_disallowed.lua Outdated
@@ -0,0 +1,93 @@
---------------------------------------------------------------------------------------------------
Copy link

Choose a reason for hiding this comment

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

Please move script to separate branch with general scripts

test_scripts/API/VehicleData/HandsOffSteering/033_OnVD_notification_for_two_Apps.lua Outdated
@@ -0,0 +1,50 @@
---------------------------------------------------------------------------------------------------
Copy link

Choose a reason for hiding this comment

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

Pleas move this checks in 031 ans 029 scripts

...I/VehicleData/HandsOffSteering/034_SubscribeVD_during_resumption_IGN_OFF_ON_for_two_Apps.lua Outdated
---------------------------------------------------------------------------------------------------
-- Proposal:https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0257-New-vehicle-data-HandsOffSteering.md
--
-- Description: Check that SDL restored SubscribeVehicleData on 'handsOffSteering' parameter after IGN_OFF/IGN_ON cycle
Copy link

Choose a reason for hiding this comment

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

Suggested change
-- Description: Check that SDL restored SubscribeVehicleData on 'handsOffSteering' parameter after IGN_OFF/IGN_ON cycle
-- Description: Check that SDL restores SubscribeVehicleData on 'handsOffSteering' parameter after IGN_OFF/IGN_ON cycle

...I/VehicleData/HandsOffSteering/034_SubscribeVD_during_resumption_IGN_OFF_ON_for_two_Apps.lua Outdated
-- 1) Update preloaded_pt file, add handsOffSteering parameter to VD_RPC group
-- 2) RPC SubscribeVehicleData and handsOffSteering is allowed by policies
-- 3) App_1 and App_2 are registered and subscribed on handsOffSteering VD
-- 4) IGN_OFF/IGN_ON cycle is performed
Copy link

Choose a reason for hiding this comment

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

Suggested change
-- 4) IGN_OFF/IGN_ON cycle is performed
-- 4) IGN_OFF/IGN_ON is performed

...I/VehicleData/HandsOffSteering/034_SubscribeVD_during_resumption_IGN_OFF_ON_for_two_Apps.lua Outdated
local appId_2 = 2
local isExpectedSubscribeVDonHMI = true
local notExpectedSubscribeVDonHMI = false
local isHMISubscription = true
Copy link

Choose a reason for hiding this comment

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

Please use isExpectedSubscribeVDonHMI and notExpectedSubscribeVDonHMI insted of isHMISubscription and notHMISubscription

dboltovskyi
dboltovskyi requested changes May 20, 2020
View reviewed changes
test_scripts/API/VehicleData/HandsOffSteering/common.lua
--! @parameters: none
--! @return: none
--]]
function m.ignitionOff()
Copy link

Choose a reason for hiding this comment

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

@GetmanetsIrina There is still some instability in IgnitionOff sequence. Please use implementation from https://github.com/smartdevicelink/sdl_atf_test_scripts/blob/13becb88a4be193158eb2d349a79666041415e03/test_scripts/Smoke/commonSmoke.lua#L260
This is most stable one and going to be provided to PMs for review and merge in develop.

test_scripts/API/VehicleData/HandsOffSteering/common.lua
@@ -0,0 +1,288 @@
---------------------------------------------------------------------------------------------------
Copy link

Choose a reason for hiding this comment

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

@GetmanetsIrina Common module looks good. Though why don't we use the same module for WindowStatus with modifications for handsOffSteering?

test_scripts/API/VehicleData/HandsOffSteering/common.lua Outdated
--! pGroup: table with additional updates (optional)
--! @return: none
--]]
function m.updatedPreloadedPTFile(pGroup)
Copy link

Choose a reason for hiding this comment

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

@GetmanetsIrina I would propose to rename this function to updatePreloadedPTFile

test_sets/new_vehicle_data_hands_off_steering.txt Outdated
./test_scripts/API/VehicleData/HandsOffSteering/001_GetVD_Success_flow.lua
./test_scripts/API/VehicleData/HandsOffSteering/002_GetVD_disallowed.lua
./test_scripts/API/VehicleData/HandsOffSteering/003_GetVD_disallowed_after_PTU.lua
./test_scripts/API/VehicleData/HandsOffSteering/004_GetVD_HMI_does_not_respond.lua
Copy link

Choose a reason for hiding this comment

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

@GetmanetsIrina Set needs to be updated since it includes non existing scripts (e.g. 004 - 008)

test_scripts/API/VehicleData/HandsOffSteering/008_SubscribeVD_disallowed_after_PTU.lua
@@ -0,0 +1,71 @@
---------------------------------------------------------------------------------------------------
Copy link

Choose a reason for hiding this comment

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

@GetmanetsIrina Please notice this script fails on https://github.ford.com/SmartDeviceLinkMirror/sdl_core/tree/feature/new_vehicle_data_hands_off_steering (9e169b289b) on all policy modes (P,E,H) and connection types (OFF, TCP, WS, WSS)

Copy link

Choose a reason for hiding this comment

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

The script is failed because of smartdevicelink/sdl_core#3365

Copy link

Choose a reason for hiding this comment

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

@GetmanetsIrina Pointed defect relates to processing of OnVehicleData notification, as I can see mentioned notification is not processed in current script. How pointed issue affects current script?

@GetmanetsIrina GetmanetsIrina force-pushed the feature/new_vehicle_data_hands_off_steering branch from 26d939c to a5e3f36 Compare May 21, 2020 15:19
@GetmanetsIrina GetmanetsIrina changed the base branch from feature/hands_off_steering to feature/0257_vehicle_data_handsOffSteering May 21, 2020 15:20
aderiabin
aderiabin reviewed May 22, 2020
View reviewed changes
test_scripts/API/VehicleData/HandsOffSteering/001_GetVD_Success_flow.lua

--[[ Scenario ]]
common.Title("Preconditions")
common.Step("Clean environment and update preloaded_pt file", common.preconditions)
Copy link

Choose a reason for hiding this comment

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

@TMyza No parameters are passed to function common.preconditions() here, so by default all 4 vehicle data related RPCs is allowed for new parameter. This can shadow potential defects in policy handling (example: policy for OnVehicleData has influence on GetVehicleData RPC, e.t.c)
I would propose to set next policy permissions: GetVehicleData - allowed, other disallowed.

Similar comment is applicable to other scripts with positive test cases for other RPCs.

@GetmanetsIrina GetmanetsIrina force-pushed the feature/new_vehicle_data_hands_off_steering branch from a5e3f36 to 3ef6670 Compare May 22, 2020 12:58
@GetmanetsIrina GetmanetsIrina merged commit d4a6d31 into LuxoftSDL:feature/0257_vehicle_data_handsOffSteering May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aderiabin aderiabin aderiabin left review comments

@dboltovskyi dboltovskyi dboltovskyi requested changes

@GetmanetsIrina GetmanetsIrina GetmanetsIrina approved these changes

@HSavynetska HSavynetska Awaiting requested review from HSavynetska

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TMyza @GetmanetsIrina @aderiabin @dboltovskyi