salt icon indicating copy to clipboard operation
salt copied to clipboard

Feat/boto sns unit tests

Open major0 opened this issue 4 years ago • 3 comments

What does this PR do?

  • Add unit tests for boto_sns
  • Fix boto_sns to get it to pass all the currently defined unit tests
  • Set the stage for testing the boto3 conversion of boto_sns

Previous Behavior

Previously boto_sns had no unit testing and had no way to validate that it could perform basic operations against SNS topics/subscriptions.

New Behavior

  • Add unit tests for boto_sns
  • Fix returning more than 100 topics in get_all_topics()
  • Fix returning more than 100 subscriptions in get_all_subscriptions_by_topic()
  • Catch exceptions and properly return True/False as expected.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

  • N/A Docs
  • [X] Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
  • [X] Tests written/updated

Commits signed with GPG?

Yes

major0 avatar Oct 24 '20 18:10 major0

@major0 looks like there are some failing tests here.

Ch3LL avatar Apr 05 '21 19:04 Ch3LL

Hi! I'm your friendly PR bot!

You might be wondering what I'm doing commenting here on your PR.

Yes, as a matter of fact, I am...

I'm just here to help us improve the documentation. I can't respond to questions or anything, but what I can do, I do well!

Okay... so what do you do?

I detect modules that are missing docstrings or "CLI Example" on existing docstrings! When I was created we had a lot of these. The documentation for these modules need some love and attention to make Salt better for our users.

So what does that have to do with my PR?

I noticed that in this PR there are some files changed that have some of these issues. So I'm leaving this comment to let you know your options.

Okay, what are they?

Well, my favorite, is that since you were making changes here I'm hoping that you would be the most familiar with this module and be able to add some other examples or fix any of the reported issues.

If I can, then what?

Well, you can either add them to this PR or add them to another PR. Either way is fine!

Well... what if I can't, or don't want to?

That's also fine! We appreciate all contributions to the Salt Project. If you can't add those other examples, either because you're too busy, or unfamiliar, or you just aren't interested, we still appreciate the contributions that you've made already.

Whatever approach you decide to take, just drop a comment here letting us know!

Detected Issues (click me)
Check Known Missing Docstrings...........................................Failed
- hook id: invoke
- exit code: 1

/home/runner/.cache/pre-commit/repo8am281e5/py_env-python3/lib/python3.9/site-packages/_distutils_hack/init.py:33: UserWarning: Setuptools is replacing distutils. warnings.warn("Setuptools is replacing distutils.") The function 'create' on 'salt/modules/boto_sns.py' does not have a 'CLI Example:' in its docstring The function 'delete' on 'salt/modules/boto_sns.py' does not have a 'CLI Example:' in its docstring The function 'get_all_subscriptions_by_topic' on 'salt/modules/boto_sns.py' does not have a 'CLI Example:' in its docstring The function 'subscribe' on 'salt/modules/boto_sns.py' does not have a 'CLI Example:' in its docstring Found 4 errors


Thanks again!

github-actions[bot] avatar Sep 20 '22 20:09 github-actions[bot]

Hi! I'm your friendly PR bot!

You might be wondering what I'm doing commenting here on your PR.

Yes, as a matter of fact, I am...

I'm just here to help us improve the documentation. I can't respond to questions or anything, but what I can do, I do well!

Okay... so what do you do?

I detect modules that are missing docstrings or "CLI Example" on existing docstrings! When I was created we had a lot of these. The documentation for these modules need some love and attention to make Salt better for our users.

So what does that have to do with my PR?

I noticed that in this PR there are some files changed that have some of these issues. So I'm leaving this comment to let you know your options.

Okay, what are they?

Well, my favorite, is that since you were making changes here I'm hoping that you would be the most familiar with this module and be able to add some other examples or fix any of the reported issues.

If I can, then what?

Well, you can either add them to this PR or add them to another PR. Either way is fine!

Well... what if I can't, or don't want to?

That's also fine! We appreciate all contributions to the Salt Project. If you can't add those other examples, either because you're too busy, or unfamiliar, or you just aren't interested, we still appreciate the contributions that you've made already.

Whatever approach you decide to take, just drop a comment here letting us know!

Detected Issues (click me)
Check Known Missing Docstrings...........................................Failed
- hook id: invoke
- exit code: 1

/home/runner/.cache/pre-commit/repo8am281e5/py_env-python3/lib/python3.9/site-packages/_distutils_hack/init.py:33: UserWarning: Setuptools is replacing distutils. warnings.warn("Setuptools is replacing distutils.") The function 'create' on 'salt/modules/boto_sns.py' does not have a 'CLI Example:' in its docstring The function 'delete' on 'salt/modules/boto_sns.py' does not have a 'CLI Example:' in its docstring The function 'get_all_subscriptions_by_topic' on 'salt/modules/boto_sns.py' does not have a 'CLI Example:' in its docstring The function 'subscribe' on 'salt/modules/boto_sns.py' does not have a 'CLI Example:' in its docstring Found 4 errors


Thanks again!

github-actions[bot] avatar Sep 20 '22 20:09 github-actions[bot]

@major0 are you able to come back to this one and fix the tests?

Ch3LL avatar Sep 28 '22 19:09 Ch3LL

Closing due to inactivity. Please let me know if you want me to re-open this or feel free to re-open a PR against master with fixes to the failing tests.

Ch3LL avatar Oct 17 '22 19:10 Ch3LL