Skip to content
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

Open
nmlorg opened this issue Feb 6, 2019 · 7 comments
Assignees
Labels
cleanup Code changes that improve maintainability without changing behavior

Comments

@nmlorg
Copy link
Owner

nmlorg commented Feb 6, 2019

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:

# test_echo.py
def test_admin(conversation):  # pylint: disable=redefined-outer-name
    """Test /admin BOTNAME echo."""

    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

    assert conversation.message('EchoTest') == [
        {
            'chat_id': 1000,
            'disable_web_page_preview': True,
            'parse_mode': 'HTML',
            'text': 'Bot Admin \u203a modulestestbot \u203a echo \u203a echotest: <b>Type the message for /echotest</b>\n'
                    '\n'
                    'Type the text you want me to send in response to <code>/echotest</code>:',
            'reply_markup': {'inline_keyboard': [[{'text': 'Back', 'callback_data': '/admin modulestestbot echo'}]]},
        },
    ]  # yapf: disable

    assert conversation.message('my message') == [
        {
            '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'
                    '/echotest now echoes <code>my message</code>.\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': '/echotest (my message)', 'callback_data': '/admin modulestestbot echo echotest remove'}],
                                                 [{'text': 'Back', 'callback_data': '/admin modulestestbot'}]]},
        },
    ]  # yapf: disable

    assert conversation.message('/admin modulestestbot echo echotest new message') == [
        {
            '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'
                    'Changed /echotest from <code>my message</code> to <code>new message</code>.\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': '/echotest (new message)', 'callback_data': '/admin modulestestbot echo echotest remove'}],
                                                 [{'text': 'Back', 'callback_data': '/admin modulestestbot'}]]},
        },
    ]  # yapf: disable

    assert conversation.message('/admin modulestestbot echo echotest remove') == [
        {
            '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'
                    'Removed /echotest (<code>new message</code>).\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

    assert conversation.message('/admin modulestestbot echo bogus remove') == [
        {
            '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'
                    '/bogus is not echoing anything.\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

couple become the slightly more-manageable:

# test_echo.transcript
>>> /admin modulestestbot echo
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]


>>> EchoTest
Bot Admin \u203a modulestestbot \u203a echo \u203a echotest: <b>Type the message for /echotest</b>

Type the text you want me to send in response to <code>/echotest</code>:
[Back|/admin modulestestbot echo]


>>> my message
Bot Admin \u203a modulestestbot \u203a echo: <b>Choose a command</b>

/echotest now echoes <code>my message</code>.

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.
[/echotest (my message)|/admin modulestestbot echo echotest remove]
[Back|/admin modulestestbot]


>>> /admin modulestestbot echo echotest new message
Bot Admin \u203a modulestestbot \u203a echo: <b>Choose a command</b>

Changed /echotest from <code>my message</code> to <code>new message</code>.

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.
[/echotest (new message)|/admin modulestestbot echo echotest remove]
[Back|/admin modulestestbot]     


>>> /admin modulestestbot echo echotest remove   
Bot Admin \u203a modulestestbot \u203a echo: <b>Choose a command</b>   

Removed /echotest (<code>new message</code>).   

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]     


>>> /admin modulestestbot echo bogus remove   
Bot Admin \u203a modulestestbot \u203a echo: <b>Choose a command</b>   

/bogus is not echoing anything.   

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]

using something like (https://docs.pytest.org/en/latest/example/nonpython.html):

def pytest_collect_file(parent, path):
    if path.ext == '.transcript' and path.basename.startswith('test'):
        return TranscriptFile(path, parent)


class TranscriptFile(pytest.File):
    def collect(self):
        command = None
        expected = []
        for line in self.fspath.open():
            if line.startswith('>>> '):
                if command:
                    yield TranscriptItem(command, self, expected, conversation)
                command = line[len('>>> '):]
                expected = []
            else:
                expected.append(line)
        if command:
            yield TranscriptItem(command, self, expected, conversation)


class TranscriptItem(pytest.Item):
    def __init__(self, command, parent, expected, conversation):
        super(TranscriptItem, self).__init__(command, parent)
        self.command = command
        self.expected = expected
        self.conversation = conversation

    def runtest(self):
        response = self.conversation.message(self.command)
        assert format_response(response) == self.expected
@nmlorg nmlorg added the cleanup Code changes that improve maintainability without changing behavior label Feb 6, 2019
@nmlorg
Copy link
Owner Author

nmlorg commented Feb 6, 2019

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 /admin messages, but the test logic of just requiring the output to end with (or not end with) " ago" doesn't fit cleanly into a pure-golden-output test.

One way to handle this would be to allow Python in the transcript file, something like:

>>> /mycountdown
!!! assert len(ret) == 1
!!! assert not ret[0]['text'].endswith(' ago')

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):

# test_countdown_future - Verify the bot handles targets in the future.
>>> /countdown

(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).

@nmlorg
Copy link
Owner Author

nmlorg commented Feb 6, 2019

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 /admin behavior in both test_admin.py and test_XXX.py for the XXX.py module's admin(); see also #8). Again, optimal option might be to use a hybrid/permissive approach, where modules can continue testing in test_XXX.py, in a new test_XXX.transcript, in a new, global test.tanscript, or any combination of the three.

@nmlorg
Copy link
Owner Author

nmlorg commented Feb 7, 2019

So, I conflated two issues above:  All the duplication of things like "disable_web_page_preview": true,, and the boilerplate of Python imports, function definitions/docstrings, encoding text as Python string literals (with escaping, indentation, etc.).

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:

>>> /admin modulestestbot echo
[chat_id 1000] [disable_web_page_preview True] [parse_mode HTML]
Bot Admin › modulestestbot › echo: <b>Choose a command</b>

Type the name of a command to add (like <code>rules</code>—don't include a slash at the beginning!), or select an existing echo to remove.
[Back|/admin modulestestbot]

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 ' and " characters, escaped UTF-8, etc., nor needing to indent things, structure them as Python lists of dicts, etc.).

A useful first step might be to actually just build the format_response() from my skeleton implementation, so the test could start off as:

    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.).)

@nmlorg
Copy link
Owner Author

nmlorg commented Feb 7, 2019

And the call to format_response could actually be optionally done in conversation.message() (but maybe suppressed with raw=True if needed), and combined with a quick '\n\n'.join(), so that final example would end up as just:

    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]
"""

 
This gets us most of what I initially wanted, but doesn't make non-ASCII characters easier to visually identify, and will probably help, but not fully resolve, pytest's [dependencies'] insistence on mangling code that's >80 columns in error output (though, if it comes down to it, it might be reasonable to have format_response actually wrap lines as a workaround).

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.

@nmlorg
Copy link
Owner Author

nmlorg commented Feb 11, 2019

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.

@nmlorg
Copy link
Owner Author

nmlorg commented Feb 11, 2019

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
"""

nmlorg added a commit that referenced this issue Feb 12, 2019
…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.
@nmlorg nmlorg self-assigned this Feb 13, 2019
@nmlorg
Copy link
Owner Author

nmlorg commented Nov 23, 2021

I recently built a .transcript runner in foyerbot, and have been working on migrating it to ntelebot. My idea is to have projects create a root-level conftest.py that just specifies:

pytest_plugins = ['ntelebot.transcript']

(or specify -p ntelebot.transcript on the pytest command line, or otherwise pulls in that module ("pytest plugin")), then it will automatically collect .transcript files throughout the package and run them through... something. I have things built to the point that I have a dict representing an Update, and a list of literal lines it should cause to be generated:

___ Simulate a chat_join_request interaction ___
  user1000:  [chat_join_request]
+ {'chat_join_request': {'chat': {'id': -1001000002000}, 'user': {'id': 1000}}}
- [1000]
- test12345678bot:  [send_photo photo=[Image: 2 + 2]]
-                   Solve!
  user1000:  robots
- test12345678bot:  Solve!
+ {'message': {'text': 'robots', 'chat': {'id': 1000}, 'user': {'id': 1000}}}
  user1000:  4
+ {'message': {'text': '4', 'chat': {'id': 1000}, 'user': {'id': 1000}}}
- [-1001000002000]
- test12345678bot:  [approve_chat_join_request user_id=1000]
 ⋮
FAILED foyerbot/test_foyer.transcript::Simulate a chat_join_request interaction

but I'm a little stuck deciding exactly how to glue the ntelebot.transcript[.pytest_hook] plugin with something in the calling package (foyerbot, metabot, etc.) that can actually process Updates.

  1. One idea is to use a conftest.py (the root, or in any package directory) to:

    1. define a pytest_itemcollected that inserts a callable into each TranscriptItem:

      import ntelebot
      
      class MockBot(ntelebot.bot.Bot):
          def handle(self, update):
           ⋮
      
      def pytest_itemcollected(item):
          item.ntelebot_transcript_bot = MockBot
    2. or define a specific variable (like a pytest hook, like pytest_itemcollected) that pytest_hook expects to be able to find:

      import ntelebot
      
      class MockBot(ntelebot.bot.Bot):
          def handle(self, update):
           ⋮
      
      ntelebot_transcript_bot = MockBot
    3. or other things along those lines, like creating a new ntelebot.transcript.BaseMockBot with a metaclass that makes subclasses visible to pytest_hook (and also generalizes things like intercepting and prettyprinting HTTP requests):

      import ntelebot.transcript
      
      class MockBot(ntelebot.transcript.BaseMockBot):
          def handle(self, update):
           ⋮
  2. In another direction, I could store the executable code in each individual .transcript file:

    1. referencing a Python module foyerbot.testing's handle function:
        # Tests for foyerbot.foyer.
      
      + HANDLER = foyerbot.testing.handle      
      
        # Verify the bot just echos unexpected private messages.
        [1000]
        user1000:  spaceships
        test12345678bot:  🔊 spaceships
      
        # Simulate a legacy /v interaction.
        [-1001000002000]
        user2000:  /link
         ⋮
      
    2. or inserting raw Python code that's re-executed for each test case (TranscriptItem):
        # Tests for foyerbot.foyer.
      
      + >>> monkeypatch('time.time', lambda: 1e9)
      + >>> class MockBot(ntelebot.bot.Bot):
      + >>>     def handle(self, update):
      + >>>      ⋮
      
        # Verify the bot just echos unexpected private messages.
         ⋮
      
  3. A sort of middle approach would be to not actually declare a pytest plugin that automatically collects .transcript files, but keep defining a custom pytest_collect_file that explicitly passes an Update handler in when it constructs ntelebot.transcript.pytest_hook.TranscriptFile, maybe something along the lines of:

    class MyTranscriptItem(ntelebot.transcript.TranscriptItem):
        def setup(self, monkeypatch):
            monkeypatch('time.time', lambda: 1e9)
            self.bot = MockBot()
    
        def handle(self, update):
         ⋮
    
    def pytest_collect_file(parent, path):
        if path.ext == '.transcript' and path.basename.startswith('test'):
            return ntelebot.transcript.TranscriptFile.from_parent(parent, fspath=path, item=MyTranscriptItem)

    but there seems to be a lot of repetition/boilerplate.

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code changes that improve maintainability without changing behavior
Projects
None yet
Development

No branches or pull requests

1 participant