-
Notifications
You must be signed in to change notification settings - Fork 0
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Build a custom test runner, so test_echo.py -> test_echo.transcript (etc.) #35
Comments
One major source of hairiness comes from tests that include actual Python logic (rather than the Python being strictly boilerplate), like test_countdown.py's: def test_countdown(conversation): # pylint: disable=redefined-outer-name
"""Verify the countdown module (which uses dynamic commands)."""
assert conversation.message('/mycountdown') == []
conversation.multibot.conf['bots']['modulestestbot']['countdown']['mycountdown'] = 1534906800
ret = conversation.message('/mycountdown')
assert len(ret) == 1
assert ret[0]['text'].endswith(' ago')
conversation.multibot.conf['bots']['modulestestbot']['countdown']['mycountdown'] = 15349068000
ret = conversation.message('/mycountdown')
assert len(ret) == 1
assert not ret[0]['text'].endswith(' ago') in which the pure-Python setup logic can be easily replaced with One way to handle this would be to allow Python in the transcript file, something like:
which of course inverts us from having Python as boilerplate to having the transcript format as boilerplate. Another way would be to introduce some kind of linkage between the transcript and a related test_XXX.py file, possibly implicitly by allowing lines in a transcript to have names (primarily for use in test reporting, as an alternative to just using the message text):
(with an identically named function in test_countdown.py that implements the actual test logic). For now, I think it might be worth going ahead splitting things (so some modules could be 100% test_XXX.py, some 100 test_XXX.transcript, and some actually split between them). |
Another issue is the concept of a conversation. Right now, the conversation object is rebuilt for teach test_xxx function in a test_XXX.py file, but the simple way of doing the transcript (without any explicit labels) would essentially treat the whole file as one self-contained test_xxx function (and so all commands would be issued against the same conversation). This also brings up the issue of whether we should actually move toward just one single test.transcript file for the whole bot, where every module (etc.) is tested all against the same conversation. This would reduce test independence but would potentially be as, or more, useful as sort of an end-to-end regression test, which the current test_XXX.py files are already sliding towards (like testing |
So, I conflated two issues above: All the duplication of things like The former can be addressed by just doing things like changing: assert conversation.message('/admin modulestestbot echo') == [
{
'chat_id': 1000,
'disable_web_page_preview': True,
'parse_mode': 'HTML',
'text': 'Bot Admin \u203a modulestestbot \u203a echo: <b>Choose a command</b>\n'
'\n'
"Type the name of a command to add (like <code>rules</code>\u2014don't include a slash at the beginning!), or select an existing echo to remove.",
'reply_markup': {'inline_keyboard': [[{'text': 'Back', 'callback_data': '/admin modulestestbot'}]]},
},
] # yapf: disable to: assert conversation.message('/admin modulestestbot echo')[0]['text'] == (
'Bot Admin \u203a modulestestbot \u203a echo: <b>Choose a command</b>\n'
'\n'
"Type the name of a command to add (like <code>rules</code>\u2014don't include a slash at the beginning!), or select an existing echo to remove.") @alexmsimms suggested essentially building a helper that would take anything that wasn't a default value and construct the appropriate dict, maybe: assert conversation.message('/admin modulestestbot echo') == [
build_response(
text='Bot Admin \u203a modulestestbot \u203a echo: <b>Choose a command</b>\n'
'\n'
"Type the name of a command to add (like <code>rules</code>\u2014don't include a slash at the beginning!), or select an existing echo to remove.",
reply_markup={'inline_keyboard': [[{'text': 'Back', 'callback_data': '/admin modulestestbot'}]]}),
] # yapf: disable (so tests that triggered chat_id to be something other than 1000, or disable_web_page_preview to not be enabled, etc., could still be explicitly tested). The latter is what I was thinking about with the new .transcript test format, and de-conflating things that test might actually look more like:
i.e. all of the data that's currently tested would/could still be present, the only change would be in how everything is formatted (not worrying about embedded A useful first step might be to actually just build the assert map(format_response, conversation.message('/admin modulestestbot echo')) == [
"""\
[chat_id 1000] [disable_web_page_preview True] [parse_mode HTML]
Bot Admin \u203a modulestestbot \u203a echo: <b>Choose a command</b>
Type the name of a command to add (like <code>rules</code>\u2014don't include a slash at the beginning!), or select an existing echo to remove.
[Back|/admin modulestestbot]
""",
] # yapf: disable which might be enough of an improvement that pulling things into separate .transcript files is wholly unnecessary. (And just re-combining the issues, format_response (whether used in .transcript files or explicitly in .py files) could suppress fields if they didn't match the defaults (chat_id=1000, etc.).) |
And the call to format_response could actually be optionally done in conversation.message() (but maybe suppressed with assert conversation.message('/admin modulestestbot echo')) == """\
[chat_id 1000] [disable_web_page_preview True] [parse_mode HTML]
Bot Admin \u203a modulestestbot \u203a echo: <b>Choose a command</b>
Type the name of a command to add (like <code>rules</code>\u2014don't include a slash at the beginning!), or select an existing echo to remove.
[Back|/admin modulestestbot]
""" Even if we don't go ahead with test_XXX.transcript files, there's also still an open question of whether we should build an end-to-end regression test (test.transcript above), or stick 100% with [scope-creeping] unit tests. |
I started working on the conversation.message change, then got caught up in busy weekend activities, but one more thought: It might be nice to actually keep conversation.message as is, and just introduce a new conversation.transcript which behaves the way test_XXX.transcript files would (with everything, including the message text (with the ">>> " prefixes) in Python string literals). This might make what's going on even easier to follow at a glance, while further reducing Python boilerplate (multi-message tests would just be one conversation.transcript call, rather than multiple conversation.message calls). I might go ahead with the most-recent plan and actually partially roll it back to switch to conversation.transcript in a followup. |
I think I'm ready to go except for formatting inline query responses like: assert conversation.raw_inline('groups') == [
{
'cache_time': 60,
'inline_query_id': 2000,
'results': [
{
'description': 'Dummy private group',
'id': 'https://t.me/joinchat/DUMMY_INVITE_LINK',
'input_message_content': {
'disable_web_page_preview': True,
'message_text': '<a href="https://t.me/joinchat/DUMMY_INVITE_LINK">Dummy Private Group!</a>\n'
'Dummy private group',
'parse_mode': 'HTML',
},
'title': 'Dummy Private Group! \u2022 https://t.me/joinchat/DUMMY_INVITE_LINK',
'type': 'article',
},
{
'description': 'Dummy public group',
'id': 'https://t.me/DummyGroup',
'input_message_content': {
'disable_web_page_preview': True,
'message_text': '<a href="https://t.me/DummyGroup">Dummy Public Group!</a>\n'
'Dummy public group',
'parse_mode': 'HTML',
},
'title': 'Dummy Public Group! \u2022 DummyGroup',
'type': 'article',
},
],
}
] # yapf: disable which might make sense to leave alone, but also might make sense to turn into something like: assert conversation.inline('groups') == """\
[cache_time=60 inline_query_id=2000]
description: Dummy private group
id: https://t.me/joinchat/DUMMY_INVITE_LINK
title: Dummy Private Group! \u2022 https://t.me/joinchat/DUMMY_INVITE_LINK
type: article
[disable_web_page_preview=True parse_mode=HTML]
<a href="https://t.me/joinchat/DUMMY_INVITE_LINK">Dummy Private Group!</a>
Dummy private group
description: Dummy public group
id: https://t.me/DummyGroup
title: Dummy Public Group! \u2022 DummyGroup
type: article
[disable_web_page_preview=True parse_mode=HTML]
<a href="https://t.me/DummyGroup">Dummy Public Group!</a>
Dummy public group
""" or maybe: assert conversation.inline('groups') == """\
[cache_time=60 inline_query_id=2000]
--
[id=https://t.me/joinchat/DUMMY_INVITE_LINK type=article]
Dummy Private Group! \u2022 https://t.me/joinchat/DUMMY_INVITE_LINK
Dummy private group
[disable_web_page_preview=True parse_mode=HTML]
<a href="https://t.me/joinchat/DUMMY_INVITE_LINK">Dummy Private Group!</a>
Dummy private group
--
[id=https://t.me/DummyGroup type=article]
Dummy Public Group! \u2022 DummyGroup
Dummy public group
[disable_web_page_preview=True parse_mode=HTML]
<a href="https://t.me/DummyGroup">Dummy Public Group!</a>
Dummy public group
""" |
…of the raw data structure intended to be sent over the wire. A followup will do something similar for conversation.inline (now conversation.raw_inline). Another followup may further convert this from: conversation.message('/xxx') == """\ AAA """ conversation.message('/yyy') == """\ BBB """ to something like: conversation.transcript(""" >>> /xxx AAA >>> /yyy BBB """ See #35.
I recently built a pytest_plugins = ['ntelebot.transcript'] (or specify
but I'm a little stuck deciding exactly how to glue the
I've been bouncing between these possibilities without making much progress for a while now, so just checkpointing what I'm thinking. (Maybe it would be a good first step to do something like (3) and plan to iterate from there.) |
This might honestly end up being too hairy to be worth it, but for simple tests, it might be nice to get rid of the essentially boilerplate Python wrapping around transcripts. For example:
couple become the slightly more-manageable:
using something like (https://docs.pytest.org/en/latest/example/nonpython.html):
The text was updated successfully, but these errors were encountered: