Bibo-Joshi · GitHub

Comment on lines +688 to +690

assert check_shortcut_signature(Chat.forward_from, Bot.forward_message, ["chat_id"], [])
assert await check_shortcut_call(chat.forward_from, chat.get_bot(), "forward_message")
assert await check_defaults_handling(chat.forward_from, chat.get_bot())

Copy link Copy Markdown

Member

Choose a reason for hiding this comment

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

please also add these to the new tests above

Copy link Copy Markdown

Member

Choose a reason for hiding this comment

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

@Bibo-Joshi how did these tests not catch the return type difference (see my review), does it not check for that?

Copy link Copy Markdown

Member

Choose a reason for hiding this comment

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

# TODO: Also check annotation of return type. Would currently be a hassle b/c typing doesn't
# resolve `ForwardRef('Type')` to `Type`. For now we rely on MyPy, which probably allows the
# shortcuts to return more specific types than the bot method, but it's only annotations after
# all

But mypy should indeed have complained 🤔

Copy link Copy Markdown

Member

Choose a reason for hiding this comment

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

also checked locally and mypy does indeed not pick that up … don't know why. For now I'm okay with you having catched it :D

Copy link Copy Markdown

Member

Choose a reason for hiding this comment

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

don't know why

probably because we are ignoring return type in every bot method?

@aditya-yadav-27 reminder to implement the tests above (cause I didn't see it in the new commit)

Copy link Copy Markdown

Member Author

Choose a reason for hiding this comment

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

should I add these to new set_photo, set_description, delete_photo, etc tests?

Copy link Copy Markdown

Member

Choose a reason for hiding this comment

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

yes

Copy link Copy Markdown

Member

Choose a reason for hiding this comment

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

probably because we are ignoring return type in every bot method?

no, I don't think that that's the reason. Bot.forward_message is annotated to return a Message - that should be all that is checked when checking what the type of await super().forward_message(…) is.

Read the original on github.com ↗