Extend & Unify Deserialization Testing
まだ誰も着手していません。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 30/100
調査の方向性
Start by reviewing the existing TO-class deserialization tests and tests/auxil/bot_method_checks.py, then compare which cases each test class currently covers. Decide how the proposed enforcement should work and ensure every relevant class covers optional arguments, additional arguments, and any agreed required-argument or localization cases.
索引モデルが issue の本文から書いたものです。
説明
This issue is a consequence of #4634 and https://github.com/python-telegram-bot/python-telegram-bot/pull/4617#discussion_r1900476722.
Currently, for each TO class we manually implement test_de_json and some classes also implement test_de_json_required and test_de_json_localization. Personally I'm okay with the explicit repitition since automating tests across several classes can be error prone itself (see #4593 and also the convoluted logic in https://github.com/python-telegram-bot/python-telegram-bot/blob/v21.9/tests/auxil/bot_method_checks.py).
However, the findings above show that we should try to ensure that we test all uses cases in all classes, that being
- deserialization works if optional arguments are missing
- deserialization works if additional arguments are passed (backward compatibility for new arguments)
To be discussed¹:deserialization works if required arguments are missing (forward compatibility for TG removing arguments)- If required for that class: deserialization correctly localizes datetimes
I can see at least three options for ensuring that a test class tests all required use cases
- Introducing helper methods that do all that and calling them in the test classes. Adantvage: Less repetition. Disatvantage: Still need to ensure that the helper methods are called and also the downside of increased test complexity.
- Introduce abstract base class for TO-Class tests with abstract methods
test_de_json,test_de_json_required,test_de_json_locatization(?) and possibly others. This should force subclasses to implement all relevant tests. Advantage: Enforced unified setup without added test complexity. Disadvantage: Still need to check that the ABC is used. Could be done with a meta-test. - Add a meta-test that checks simply that methods with the corresponding names exist. Advantage: Enforced unified setup without much added test complexity. Disadvantage: Less explicit than 2.
At first glance, I'm in favor of 2.
@harshil21 @Poolitzer do you have any preferences?
¹ So far, Telegram has always made the removal of fields backward compatible (with the exception of thumb back in 2015), see
- https://core.telegram.org/bots/api-changelog (search for "backward")
- https://github.com/tdlib/telegram-bot-api/issues/526#issuecomment-1938506997 (§2)
Of course, as per usual, Telegram doesn't document such things.
- 主要言語
- Python
- スター
- 29.5k
- フォーク
- 6.2k
- 平均マージ
- 2日 1時間
- マージ済み PR(30日)
- 14
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
python-telegram-bot/python-telegram-bot のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 84/100
python-telegram-bot/python-telegram-bot#5315 · コメント 2 件 ·
-
⚙️ documentation
python-telegram-bot/python-telegram-bot#5309 · 担当者 1 名 ·
-
🛠 breaking 🛠 refactor
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
python-telegram-bot/python-telegram-bot#5279 · コメント 7 件 ·
-
⚙️ type-hinting 📋 help-wanted
難易度 5/5 1週間以上 初心者へのやさしさ 30/100
python-telegram-bot/python-telegram-bot#5272 · コメント 3 件 ·
-
🔌 bug 🔗 github-actions
python-telegram-bot/python-telegram-bot#5270 · コメント 3 件 · 担当者 1 名 ·
python-telegram-bot/python-telegram-bot の issue をすべて見る
似ている issue
-
難易度 1/5 1時間未満 初心者へのやさしさ 90/100
-
bug
難易度 2/5 1〜3時間 初心者へのやさしさ 86/100
zostera/django-bootstrap4#894 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
use-agent-os/agent-os#3276 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
zephyrproject-rtos/zephyr#119726 ·
-
area/auth bug comp/agent P3 platform/discord type/security
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
NousResearch/hermes-agent#117848 ·