GeezerVison is showing github.com — Extend & Unify Deserialization Testing · Issue #4635 · python-telegram-bot/python-telegram-bot
Extend & Unify Deserialization Testing #4635Open 1 / 11 of 1 issue completedOpen 🧹 chore 1 / 11 of 1 issue completed DescriptionThis issue is a consequence of #4634 and #4617 (comment).
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
Of course, as per usual, Telegram doesn't document such things. ActivityBibo-Joshi commented on Jan 2, 2025 Bibo-Joshi commented on Jan 3, 2025 Looking at the discussion in the tdlib issue, I strongly suggest to do 3 as well harshil21 commented on Jan 3, 2025 Yeah, the abstract base class for tests + meta test sounds pretty robust to me. Could also enforce testing of test_slots, to_dict, equality and friends.
Looking at the discussion in the tdlib issue, I strongly suggest to do 3 as well
by point 3 you mean testing the forward compatibility, right? Bibo-Joshi commented on Jan 3, 2025
deserialization works if required arguments are missing (forward compatibility for TG removing arguments)
yes, this :) Bibo-Joshi commented on Jan 5, 2025 FYI, I'd like to have #4617 merged before starting on this, as that PR is touching a lot of these tests as well. github-actions commented on Oct 15, 2025
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|