Skip to content

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