-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add basic testcases for the new APIs conncted-network-type and connected-network-type-subscriptions #251
Conversation
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.
I've one comment common to all 3 subscriptions for SUBSCRIPTION_MISMATCH UC.
This is not related to the device token but to the event type "consented" vs requested.
code/Test_definitions/device-reachability-status-subscriptions.feature
Outdated
Show resolved
Hide resolved
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
code/Test_definitions/connected-network-type-subscriptions.feature
Outdated
Show resolved
Hide resolved
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.
Look good for me.
Thanks @dfischer-tech
Note that it could be unsupported or unauthorized. Indeed for me the error is triggered as long as the event type(s) linked to the token are not aligned with event type in the request. But I'm fine with your proposal.
code/Test_definitions/connected-network-type-subscriptions.feature
Outdated
Show resolved
Hide resolved
code/Test_definitions/device-roaming-status-subscriptions.feature
Outdated
Show resolved
Hide resolved
code/Test_definitions/device-roaming-status-subscriptions.feature
Outdated
Show resolved
Hide resolved
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
@bigludo7 @akoshunyadi @eric-murray can we get this approved / merged asap so that we can progress on the Pre-Release PR?
code/Test_definitions/connected-network-type-subscriptions.feature
Outdated
Show resolved
Hide resolved
code/Test_definitions/connected-network-type-subscriptions.feature
Outdated
Show resolved
Hide resolved
code/Test_definitions/connected-network-type-subscriptions.feature
Outdated
Show resolved
Hide resolved
code/Test_definitions/connected-network-type-subscriptions.feature
Outdated
Show resolved
Hide resolved
code/Test_definitions/connected-network-type-subscriptions.feature
Outdated
Show resolved
Hide resolved
code/Test_definitions/connected-network-type-subscriptions.feature
Outdated
Show resolved
Hide resolved
code/Test_definitions/connected-network-type-subscriptions.feature
Outdated
Show resolved
Hide resolved
…ture Co-authored-by: Eric Murray <[email protected]>
…ture Co-authored-by: Eric Murray <[email protected]>
…ture Co-authored-by: Eric Murray <[email protected]>
…ture Co-authored-by: Eric Murray <[email protected]>
…ture Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
….feature Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
….feature Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
Co-authored-by: Eric Murray <[email protected]>
A few remarks from my side:
|
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
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.
OK. Let's merge this one as well.
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
In order to participate in the next meta release, we need basic test cases for the new APIs.
Fixed typos and format issues in the existing ones.
Which issue(s) this PR fixes:
Fixes #234