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

Geofencing feature file #181

Merged
merged 44 commits into from
Aug 30, 2024
Merged

Conversation

mdomale
Copy link
Contributor

@mdomale mdomale commented Apr 8, 2024

Geofencing feature file

What type of PR is this?

Add one of the following kinds:

  • tests

What this PR does / why we need it:

This PR is required for consideration of Feature file for Geofencing which includes all possible test scenarios

Which issue(s) this PR fixes:

No specific issue available currently

Fixes #211

Special notes for reviewers:

Feature file with test scenarios

Changelog input

 release-note

Additional documentation

This section can be blank.

docs

 Geofencing feature file
@mdomale
Copy link
Contributor Author

mdomale commented Apr 8, 2024

@jlurien
Copy link
Collaborator

jlurien commented Apr 8, 2024

Please @mdomale, edit the template with the right values

@mdomale
Copy link
Contributor Author

mdomale commented Apr 8, 2024

@jlurien Added required details .Please let me know in case any further details are anticipated

@bigludo7 bigludo7 added the Fall24 Meta-release Fall24 label Jul 1, 2024
Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

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

The test plan should test the scenarios listed in the guidelines for explicit subscriptions model:

For the explicit subscriptions model:

  • Validate that the subscribed events are received in the sink, with the right sinkCredential, for those situations specified in the API.
  • For subscriptions that provide subscriptionExpireTime, validate that the subscribed events are not longer received after the expiration time.
  • For subscriptions that provide subscriptionMaxEvents, validate that the subscribed events are not longer received after the maximum events limit is reached.
  • Validate that after a subscription is deleted, the subscribed events are not longer received.

The level of detail should be also in line with the guidelines. In general we need to be more precise about the Given steps to fill the request in order to generate the expected responses, and check also that the response object is the correct one, against schema or with some specific validation, not just the status code. Take a look to the examples for the other 2 APIs already merged.

code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
@mdomale
Copy link
Contributor Author

mdomale commented Aug 12, 2024

Updated

code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

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

I still think that indicating that a request body is not available for operations that do not consider it is confusing. Moreover using the term "available", in that case even "request body is not sent" or "request body is not included" would be better options (but still no needed)

The rest of the proposal now seems quite acceptable to me. Thanks

code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved

@geofencing_subscriptions_02_Operation_to_retrieve_list_of_subscriptions
Scenario: Get a list of subscriptions.
Given the request body is not available
Copy link
Collaborator

Choose a reason for hiding this comment

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

Saying that a request body is not available for an operation without request body is not needed, as we don't mention that query parameters are not available, etc. Only createSubscription requires it.

The suggestion for a client with subscriptions created is to tell the tester that this method should be tested for a client with subscriptions, otherwise the response would be [], which could be another scenario to test

code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
@mdomale
Copy link
Contributor Author

mdomale commented Aug 29, 2024

@jlurien @bigludo7 PR updated with required changes cc @akoshunyadi

Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

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

Suggestions ready to remove the unneeded references request body for get and delete, as applied already at the beginning.

code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
code/Test_definitions/Geofencing.feature Outdated Show resolved Hide resolved
@mdomale mdomale requested a review from jlurien August 29, 2024 11:45
@mdomale
Copy link
Contributor Author

mdomale commented Aug 29, 2024

@jlurien @bigludo7 PR updated with above suggestion cc @akoshunyadi

Copy link
Collaborator

@jlurien jlurien 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 @mdomale and @akoshunyadi :) @bigludo7 please take a last look

Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

Thanks a lot @mdomale for the effort.
LGTM now

@jlurien jlurien merged commit 1af60a7 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
Fall24 Meta-release Fall24
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test plan for Geofencing Subscriptions API
6 participants