Make chat_id a positional argument inside shortcut methods of Chat and User classes #1048#1050
Make chat_id a positional argument inside shortcut methods of Chat and User classes #1048#1050jh0ker merged 2 commits intopython-telegram-bot:masterfrom xates:positional-args
Conversation
jh0ker
left a comment
There was a problem hiding this comment.
Looks good to me, no problems with backwards compatibility I can see. Should be good to merge
|
@xates Looks like you also have to update the tests |
|
Ok, I have tried to figure out how they work but I am not entirely sure... would something like that work for test_chat.py for example? def test_instance_method_send_message(self, monkeypatch, chat):
def test(*args, **kwargs):
return args[0] == chat.id and args[1] == 'test'
monkeypatch.setattr('telegram.Bot.send_message', test)
assert chat.send_message('test')
def test_instance_method_send_audio(self, monkeypatch, chat):
def test(*args, **kwargs):
return args[0] == chat.id and args[1] == 'test_audio'
monkeypatch.setattr('telegram.Bot.send_audio', test)
assert chat.send_audio('test_audio')Moreover, I have noticed that both Chat and User classes lack the tests for |
|
I believe that will work, but please confirm it locally before committing (as per contributing guide). And yes, that looks like an oversight, adding tests for |
|
I cannot run the tests locally because I get an ImportError: No module named ptb_urllib3.urllib3 |
|
@xates Looks like you didn't follow this step from the contributing guide: To fix it run: |
|
@xates Sounds like you didn't clone with |
|
You're right, sorry, I missed that |
|
If you got errors in |
|
Oh, I see! That was driving me crazy... |
|
I'm sorry about that, someone unwittingly renamed the group that is used in that test so the test suddenly failed everywhere. |
|
Looks good to me 👍 |
|
Thank you very much for your contribution :) |
|
Yeah, I guess that SSL problem is not my fault 😄 Well, thank you, for running such a wonderful project 😉 |
|
Eh, tests sometimes pass and fail depending on the alignment of the planets 🌑 |
No description provided.