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
sns: add multi-account capability to failing tests #9358
Conversation
LocalStack Community integration with Pro 2 files ±0 2 suites ±0 1h 13m 34s ⏱️ - 6m 45s Results for commit e1b8e65. ± Comparison against base commit cc1efb8. This pull request removes 8 and adds 16 tests. Note that renamed tests count towards both.
This pull request skips 71 tests.
♻️ This comment has been updated with latest results. |
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.
Thanks a lot for tackling this, this put a bug into light with Unsubscribe
!
I explained the issue that needs fixing in the comment regarding modifying the test, I'm really sorry that it's a tricky test to validate. I don't think we'll need to change it anyway, the fix will live in Unsubscribe
.
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.
Thanks for changing and fixing Unsubscribe
! It would be nice if we could have an AWS validated test for the exception regarding the ARN, I know we didn't before and we relied on moto, but maybe the exception wasn't right. Would be good to check now that it's our responsibility 😄 after that it'll be good for me!
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! Awesome, thanks a lot, sorry for the requested changes, but this looks awesome now, thanks a lot for fixing an SNS bug 🐛🚀!
Motivation
When using values other than
000000000000
for account ID orus-east-1
for region,sns
tests should still create the consequent resources in this accounts and region.Changes
This PR fixes all the failing tests in
tests.aws.services.sns.test_sns.py
by:get_aws_account_id()
helper.unsubscribe
handler.subscription_arn
subscription_arn
in the handler.See #8204