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.

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

Merged
merged 8 commits into from
Aug 30, 2024
Merged

Conversation

jlurien
Copy link
Collaborator

@jlurien jlurien commented Aug 22, 2024

What type of PR is this?

  • enhancement/feature
  • tests

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

* Added test definitions for the 3 APIs

@jlurien jlurien requested a review from hdamker August 28, 2024 12:54
hdamker added a commit that referenced this pull request Aug 29, 2024
@jlurien
Copy link
Collaborator Author

jlurien commented Aug 29, 2024

@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.

@hdamker
Copy link
Collaborator

hdamker commented Aug 29, 2024

If no more comments are received, the PR would be ready to be merged before the deadline.

@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.

@jlurien jlurien requested a review from hdamker August 29, 2024 13:53
@jlurien
Copy link
Collaborator Author

jlurien commented Aug 29, 2024

@hdamker I hope I have correctly addressed your feedback, please take another look

hdamker
hdamker previously approved these changes Aug 29, 2024
Copy link
Collaborator

@hdamker hdamker left a 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.

@hdamker
Copy link
Collaborator

hdamker commented Aug 29, 2024

@jlurien Selected results from the manual linting which I asked @rartych to run, which you might want to check:

qod-provisioning-createProvisioning.feature:
116    Scenario Outline does not have any Examples                                                       no-scenario-outlines-without-examples

qod-provisioning-deleteProvisioning.feature:
77     Step "When the asynchronous deletion process is completed" should not appear after step using keyword then    keywords-in-logical-order

qod-provisioning-retrieveProvisioningByDevice.feature:
78    Scenario Outline does not have any Examples    no-scenario-outlines-without-examples

quality-on-demand-extendQosSessionDuration.feature:
74     Scenario name is already used in: quality-on-demand-extendQosSessionDuration.feature:63    no-dupe-scenario-names
 97     Scenario name is already used in: quality-on-demand-extendQosSessionDuration.feature:86    no-dupe-scenario-names

@jlurien
Copy link
Collaborator Author

jlurien commented Aug 30, 2024

@jlurien Selected results from the manual linting which I asked @rartych to run, which you might want to check:

qod-provisioning-createProvisioning.feature:
116    Scenario Outline does not have any Examples                                                       no-scenario-outlines-without-examples

qod-provisioning-deleteProvisioning.feature:
77     Step "When the asynchronous deletion process is completed" should not appear after step using keyword then    keywords-in-logical-order

qod-provisioning-retrieveProvisioningByDevice.feature:
78    Scenario Outline does not have any Examples    no-scenario-outlines-without-examples

quality-on-demand-extendQosSessionDuration.feature:
74     Scenario name is already used in: quality-on-demand-extendQosSessionDuration.feature:63    no-dupe-scenario-names
 97     Scenario name is already used in: quality-on-demand-extendQosSessionDuration.feature:86    no-dupe-scenario-names

I've corrected the relevant ones:

  • 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)

I have not changed:

  • Filenames were really OK according to the guidelines (For APIs with several operations that split the test plan in one feature file per operation, recommendation is to add the operationId to the api-name as file name: quality-on-demand-createSession.feature.) But operationId are lowerCamelCase
  • Scenario too long. Good practice but sporadically some scenarios may need some more steps

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)
hdamker
hdamker previously approved these changes Aug 30, 2024
@jlurien jlurien requested a review from hdamker August 30, 2024 08:49
Copy link
Collaborator

@hdamker hdamker left a comment

Choose a reason for hiding this comment

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

LGTM

@hdamker
Copy link
Collaborator

hdamker commented Aug 30, 2024

Good to go here - keeping the milestone date! Again a big thank you @jlurien

@hdamker hdamker merged commit 5c6ea04 into camaraproject:main Aug 30, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update and enhance test definition file for QOD API
3 participants