-
Notifications
You must be signed in to change notification settings - Fork 60
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.
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
Feature/Extended coverage for Vehicle Data tests #2488
Conversation
@atiwari9 please review this PR |
@dboltovskyi - There are significant changes to testing methodology, these changes warrant full regression tests for all VD items including the tests which are removed. I am skeptical of the scope of these changes with-in new VD related proposals, we may need to check with PMs on if this needs to be part of your other ATF related change set. I'd run the full test suite myself. Meanwhile i have left some comments for the changes. |
@atiwari9 Regarding your comment 2488#issuecomment-735976762 please let me add some clarification about the changes in this PR. Use of APIs to generate tests and test cases had been introduced in a previous release ( Current PR is an evolution of existing approach which also adds systematization in common modules and tests. Despite the fact table with test types has been introduced in the current PR, the following checks have been implemented before (in a previous release):
Current PR provides the following changes (comparing to existing approach):
I agree there are quite a lot of changes. Though having them we will significantly improve existing approach and I would say "finalize" it. It would be very easy to support new VD parameters in tests. And I hope PMs wouldn't mind to accept the changes. |
@dboltovskyi - I ran the vehicle_data test set only for tirePressure and got 5 Failed cases, is this expected? TOTAL: 38 Logs:
|
@atiwari9 Please notice approach with commenting of parameters in |
@atiwari9 Please find update in 27af7bf Now it's possible to define VD parameters for all test scripts in One limitation - this approach doesn't work in ATF parallel mode by default since it requires changes in ATF. However if you would like to do so, you just need to add the following line after https://github.com/smartdevicelink/sdl_atf/blob/6f600b7c8e926e569c9e291973817b21c850177e/atf_parallels/loop.sh#L29
This will transfer |
@dboltovskyi - Thanks, this seems to be working so i am running another batch of tests now. Is there a place to document this capability of running selectively vehicle data item tests? |
@atiwari9 For now it's "undocumented" feature.
Additionally we can do the following:
What do you think would be the best? |
@dtrunov - I think option 2 is better, as a user may refer to ReadMe to understand the usage rather than open each and every script to find out. BTW, with commit 27af7bf , All tests are Passed for individual Logs: All_Tests_Pass.7z.changeExtTo7z.log |
@atiwari9 Agree. I will do both: 1) and 2) and post a comment when it's ready |
@atiwari9 Please notice:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
@JackLivio This PR is ready for Livio review. Thank you! |
test_scripts/API/VehicleData/OnVehicleData/008_OnVD_enum_and_bool_values.lua
Outdated
Show resolved
Hide resolved
@jacobkeeler Please notice all the comments addressed |
@dboltovskyi Also as a note, several of the tests seem to be causing Core crashes as of now:
These crashes seem to be random, but they always happen during an
Seeing a SIGSEGV message in the logs:
|
@jacobkeeler Please notice the following:
|
@jacobkeeler Please notice all the questions have been addressed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should cover all of the changes that need to be made for the IN_BOUND/OUT_OF_BOUND tests
@jacobkeeler Please notice all the comments addressed. The following 3 suggested updates were not applied due to discussion: 2488#commitcomment-45860121 |
@dboltovskyi After checking the actual RPC parser code, I do see that |
@jacobkeeler Please notice descriptions have been updated in afb7faf and conflicts resolved in e490871 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions for clarity
Co-authored-by: Jacob Keeler <jacob.keeler@livioradio.com>
This is base PR for test scripts for Vehicle Data proposals:
This PR is [ready] for review.
Summary
Extended test coverage for Vehicle Data tests
ATF version
7.1.0_version_bump
Changelog
Introduced new modules
APIHelper
- set of utility functions to work with both APIs: MOBILE and HMIAPITestDataGenerator
- generate values for parameters based on existing API restrictions and desired value typeIntroduced the following test types to verify
GetVD
andOnVD
RPCs:VALID_RANDOM_ALL
- positive cases for VD parameters where all possible sub-parameters of hierarchy are defined with valid random values,VALID_RANDOM_SUB
- positive cases for struct VD parameters and sub-parameters where only one sub-parameter of hierarchy is defined with valid random value (mandatory also included),LOWER_IN_BOUND/UPPER_IN_BOUND
- positive cases for VD parameters and sub-parameters where only one sub-parameter of hierarchy is defined with min/max valid value (mandatory also included),LOWER_OUT_OF_BOUND/UPPER_OUT_OF_BOUND
- negative cases for VD parameters and sub-parameters where only one sub-parameter of hierarchy is defined with nearest invalid min/max value,INVALID_TYPE
- negative cases for VD parameters and sub-parameters with invalid type value defined for one of them,ENUM_ITEMS/ BOOL_ITEMS
- positive cases for enum/boolean VD parameters and sub-parameters with all possible values defined (all items for enum and true/false for boolean),MANDATORY_ONLY
- positive cases for struct VD parameters and sub-parameters where only mandatory sub-parameters are defined with valid random values,MANDATORY_MISSING
- negative cases for struct VD parameters and sub-parameters where only mandatory sub-parameters are defined and one of them is missing,PARAM_VERSION
- positive/negative cases for VD parameters with version definedDeleted existing VD scripts which are covered by a new tests
Updated existing scripts
Updated
vehicle_data.txt
test setAdded possibility to restrict vehicle data parameters to be tested by defining
VD_PARAMS
environment variableBug Fixes
Common.
prefixCLA