Skip to content

Add test for EventBridge Scheduler TagResource API and UntagResource API #11976

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

Merged

Conversation

kakakakakku
Copy link
Contributor

Motivation

Hi😀

I checked the EventBridge Scheduler API coverage and found that some APIs were not marked in the "Internal Test Suite". So I wanted to contribute by adding tests.

Changes

In this Pull Request, I’ve added a test for the EventBridge Scheduler TagResource API and UntagResource API to improve coverage.

It's a bit hard to understand, but the TagResource API and UntagResource API are not for EventBridge Scheduler, It's specifically for EventBridge Scheduler Group.

Testing

The tests passed in my local environment.

$ make test TEST_PATH=tests/aws/services/scheduler
(snip)
tests/aws/services/scheduler/test_scheduler.py::test_tag_resource PASSED                                                                                                                                                    [ 66%]
tests/aws/services/scheduler/test_scheduler.py::test_untag_resource PASSED                                                                                                                                                  [100%]

Could you please review? Thanks a lot 👍

I am also submitting some pull requests. I would appreciate it if you could take a look at that as well. Thanks.

@kakakakakku kakakakakku requested a review from joe4dev as a code owner December 3, 2024 13:42
@maxhoheiser maxhoheiser self-requested a review December 4, 2024 09:46
@maxhoheiser
Copy link
Member

@kakakakakku thanks for your contribution - looking great - at localstack we also generate snapshots from the responses from real AWS environments to guarantee that we have parity. I will add a snapshot for your test and create the snapshots for you since it is not straight forward and requires an AWS account.

@kakakakakku
Copy link
Contributor Author

OK, thanks!
I will add snapshot tests shortly 😀

@maxhoheiser maxhoheiser added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases aws:scheduler Amazon EventBridge Scheduler labels Dec 4, 2024
@maxhoheiser
Copy link
Member

OK, thanks! I will add snapshot tests shortly 😀

Just added them for you :) feel free to merge once all the tests re green

Copy link
Member

@maxhoheiser maxhoheiser left a comment

Choose a reason for hiding this comment

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

Nice small tests 👍 - feel free to merge once all tests are green

@maxhoheiser maxhoheiser force-pushed the add-tests-for-evb-scheduler-group branch from bafcded to acdacd1 Compare December 4, 2024 10:24
@kakakakakku
Copy link
Contributor Author

@maxhoheiser
Thank you for your support😀
Next time I add tests, I will also use conftest and include snapshot testing.

@maxhoheiser maxhoheiser merged commit fd77541 into localstack:master Dec 5, 2024
29 checks passed
@kakakakakku kakakakakku deleted the add-tests-for-evb-scheduler-group branch December 5, 2024 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:scheduler Amazon EventBridge Scheduler semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants