- Notifications
You must be signed in to change notification settings - Fork5.9k
refactor: consolidate test_sticker.py for mixed-type sticker sets#5038
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
refactor: consolidate test_sticker.py for mixed-type sticker sets#5038
Uh oh!
There was an error while loading.Please reload this page.
Conversation
- Updated test_create_sticker_set to create a single mixed-type sticker set containing static, animated, and video stickers- Consolidated duplicate test methods (test_bot_methods_1-8) that were separated by sticker type into unified tests- Updated fixtures in conftest.py to use the same mixed sticker set- All tests now work with a single sticker set that contains all three formats, reflecting Telegram's current API capability- Reduced code duplication and improved test maintainability
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.
As I said, we appreciate PRs. Still, please make sure that we actually see a PR fromyou, not a PR from an AI agent. Using AI for help is perfectly fine. Simply copy-pasting the generated output without a second look is not fine as it just generatesmore work on rview side than helping. Let me explain this in more details:
- the PR description is extremely generic. It doesn't provide any insights on why specific changes were (not) made or which questions you may still have
- you did not follow thecontribution guide (which is linked in the PR template btw), otherwise you would have run & fixed the pre-commit checks locally
- several tests are failing which at first glance looks like it might be related to your changes. If you had run the tests locally & had similar issues, you would have either addressed them or asked why you see unrelated failures.
- I left some comments below that indicate why I think that you did not really review the AI changes
Again, PRs in general are welcome. If you have questions on how to PR (which changes exactly to make, what our expectations are etc), these questions are also welcome. The current state of this AI-output is not welcome :/
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.
We should only need one single fixturesticker_set instead of maintaining a new one in addition to the old ones
| # Test add_sticker_to_set | ||
| asyncdeftest_bot_methods_1_png(self,bot,chat_id,sticker_file): | ||
| # Test add_sticker_to_set - now tests all formats in the same mixed set |
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.
| # Test add_sticker_to_set - now tests all formats in the same mixed set | |
| # Test add_sticker_to_set |
this type of comment makes no sense to have in the code base. it's only helpful while reviewing AI generated code
| assertfile | ||
| awaitasyncio.sleep(1) | ||
| # Test adding all three types to the same set |
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.
same
| tasks= [] | ||
| ifstatic_sticker: | ||
| tasks.append(bot.set_sticker_position_in_set(static_sticker.file_id,1)) | ||
| ifanimated_sticker: | ||
| tasks.append(bot.set_sticker_position_in_set(animated_sticker.file_id,2)) | ||
| ifvideo_sticker: | ||
| tasks.append(bot.set_sticker_position_in_set(video_sticker.file_id,3)) |
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.
this can be simplified with a loop
Uh oh!
There was an error while loading.Please reload this page.
Description
This PR refactors test_sticker.py to reflect Telegram's current API capability where sticker sets can now hold static, animated, and video stickers together in the same set.
Changes Made
Benefits
Testing
Related Issue
Fixes the issue: "refactor test_sticker.py since sticker sets can now hold static, animated, and video altogether"#4514