-
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
QoD Test definitions #349
QoD Test definitions #349
Conversation
@hdamker I have updated the versions and paths to the right ones for r1.2 (without references to RC), and also updated the operationId name to retrieveQoSProfiles, as commented by @RandyLevensalor If no more comments are received, the PR would be ready to be merged before the deadline. |
code/Test_definitions/quality-on-demand-extendQosSessionDuration.feature
Outdated
Show resolved
Hide resolved
@jlurien I have looked over the quality-on-demand feature files and they are looking really good to me - as far as it is possible to see without implementing the test and with limited knowledge of the language. I'm ready to approve, just some minor formal comments above. |
@hdamker I hope I have correctly addressed your feedback, please take another look |
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.
LGTM - thanks a lot @jlurien for this huge contribution and the swift corrections.
If there are issues during implementation or clarifications of the open points we have still the option to release patches of the test definitions.
@jlurien Selected results from the manual linting which I asked @rartych to run, which you might want to check:
|
I've corrected the relevant ones:
I have not changed:
|
Scenario Outline without examples, totally Step too long was really due to a previous step appended which had to be removed Repeated scenario names Multiple empty lines, not really wrong but same as trailing spaces The 2 When in an scenario is not really an error, but a common best practice. I have rephrase it a bit because for that specific scenario makes sense (first you delete and then when the async deletion is effective you received an event)
Co-authored-by: Herbert Damker <herbert.damker@telekom.de>
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.
LGTM
Good to go here - keeping the milestone date! Again a big thank you @jlurien |
What type of PR is this?
What this PR does / why we need it:
This PR includes te first proposal for Test definitions which is aligned with the new Commonalities guidelines. Hving at least a basic test plan is a requirement to be part of a meta-release.
Which issue(s) this PR fixes:
Fixes #245
Special notes for reviewers:
The proposal is quite extensive and covers more than just the minimum "basic" test plan, but this level of work is required to detect parts of the spec that are ambiguous or where implementation guidelines do not still define an explicit behaviour.
The README summarizes the list of comments in the Test definitions that require a review. Some of them may require as well a clarification in the API spec or in Commonalities guidelines. All of them can be considered not necessary for a basic test plan so they may be addressed in a subsequent version.
There is a feature file per API operation. All feature files are under the same folder /Test_definitions, but we may consider creating subfolders per API.
Changelog input