ptbtest icon indicating copy to clipboard operation
ptbtest copied to clipboard

Where to?

Open AlexPHorta opened this issue 9 months ago • 28 comments

Where do we go from here? Opened this issue to discuss the road map to use to bring PTBtest to a new version.

AlexPHorta avatar Mar 23 '25 16:03 AlexPHorta

I guess to match the PTB project at first, and then to start thinking about new features.

yehuda100 avatar Mar 23 '25 21:03 yehuda100

Also, to make the process of bot testing easy and simple maybe to the level that anything is already configured (all the bot setup stuff) and the user just writes the tests.

yehuda100 avatar Mar 23 '25 21:03 yehuda100

Yeah, I'm doing some reading on PTB first. I was wondering if it's possible/feasible to mock the networking part, so that everything would pass through our implementation. That way, I believe, everything from PTB would be used without a network call. Then, we would write convenience functions/methods to ease the test process.

The idea is that calling something like mockbot.Mockbot() would "trigger" telegram.ext.ApplicationBuilder(). But it will take some time to decipher everything.

One other thing, we maybe could write other convenience methods, in the way that UserGenerator or ChatGenerator work.

AlexPHorta avatar Mar 24 '25 11:03 AlexPHorta

Just funneling everything in here:

https://github.com/AlexPHorta/ptbtest/issues/31

AlexPHorta avatar Mar 24 '25 11:03 AlexPHorta

I haven't investigated the source code of the PTB very deeply yet only made a quick look through, but here are my thoughts.

As far as I can see almost all network activity happens in the Bot class, so we have to rewrite all methods of the Bot class.

Then ptbtest patches telegram.ext.Application, replaces the original bot with our Mockbot:

# building the app
application = Application.builder().token("TOKEN").build()
application.add_handler(CommandHandler("start", start))
application.add_handler(CommandHandler("help", help_command))
application.add_handler(MessageHandler(filters.TEXT & ~filters.COMMAND, echo))

mock_app = MockApplication(application)
mock_bot = mock_app.bot

Then we do something like mock_app.send_message("/start") and the mock bot responds.

elebur avatar Mar 24 '25 17:03 elebur

Interesting. Yeah, but I was wondering, what is the scope of the tests PTBtest will mock? I mean, would we mock everything, from app creation (with its numerous options for customization) to the most peripheral functions and methods? Or simply the basic functionality?

See, it's possible to run PTB against a Local Bot API Server. So, the needs of heavy users are covered, right? Wouldn't it be better if we focused on the basic functionality and extend it slowly? So, this way, basic users can have a simple solution to testing without running a server.

Or am I getting this wrong?

AlexPHorta avatar Mar 25 '25 02:03 AlexPHorta

The current implementation is mocking the telegram.bot class so you aren't using the network, the other way is to mock the request class but you'll need to generate a response for every request.

yehuda100 avatar Mar 25 '25 09:03 yehuda100

I think the end goal should be to cover all the PTB functionality but for now we can start from the basics..

yehuda100 avatar Mar 25 '25 09:03 yehuda100

At least we have to mock the Bot class. It has a lot of methods but I agree with you that we could start with the most important ones such as sent_message, sent_audio, sent_video etc.

Also we must think how to implement updates processing. When a user sends something to the bot it receives such messages via requesting get_updates and then Application populates updates_queue and then each update is processed against all registered handlers.

Not 100% sure but I think Application mustn't be mocked at all, the only thing that we must do with it is to replace the original Bot with our Mockbot.

elebur avatar Mar 25 '25 10:03 elebur

Or...

We could mock the telegram API, something that could be plugged in place of it, and make convenience methods with the rest.

Like, two parts.

  1. The mock API (we could use something like FastAPI or Flak to make it);
  2. The rest (convenience methods for the bot, user, chat etc.).

AlexPHorta avatar Mar 25 '25 15:03 AlexPHorta

you don't need to mock the Application class, you can insert updates one by one with Application.process_update when the automatic updater is None. Also, PTB has tests for internal use, maybe we can learn from them.

yehuda100 avatar Mar 25 '25 15:03 yehuda100

We could mock the telegram API, something that could be plugged in place of it, and make convenience methods with the rest.

@AlexPHorta Wouldn't we eventually end up reinventing the Local Bot API Server?

What would be beneficial in mocking the API over mocking telegram.Bot methods? Here are a few caveats that I see in the API approach:

  1. new entities (FastAPI, Flask) and a web server for the API
  2. which in turn leads us to slower tests execution

But while writing this message, I came up with one big benefit - the source code of the telegram.Bot remains the same.

you don't need to mock the Application class, you can insert updates one by one with Application.process_update when the automatic updater is None.

Appliaction calls Updater.start_polling which in turn calls Bot.get_updates

If we decide to mock the telegram.Bot then we could use the approach that the Mockbot already uses - insertUpdates method that puts Update into updates queue.

elebur avatar Mar 25 '25 17:03 elebur

Yes, improving from what we already have is really the best approach. Sorry about not responding here today, I was doing some audio work. Going to sleep now and tomorrow we keep on going.

AlexPHorta avatar Mar 26 '25 03:03 AlexPHorta

I'm trying to simplify the message decorator of Mockbot. None of those decorators are being tested. I believe we have to write a boatload of tests to modify this code securely.

AlexPHorta avatar Mar 26 '25 11:03 AlexPHorta

Maybe consider other methods of the core MockBot code

yehuda100 avatar Mar 26 '25 14:03 yehuda100

I've looked through the code of the telegram.Bot and all methods that interacts with the API do it via the _post method (and then it uses the _do_post method). We could rewrite the _post method to intercept all requests. @AlexPHorta , it is similar to your idea.

The main benefit of this approach is that the source code and the logic of the Bot's methods stay untouched. However, we'll have to code the logic for each endpoint of the API.

Meanwhile, I'm going to work on the entity parser, because whatever approach we choose, we'll need to parse entities.

elebur avatar Mar 26 '25 16:03 elebur

telegram.Bot and all methods that interacts with the API do it via the _post method (and then it uses the _do_post method).

we need to see if it easy to overwrite just these method, in case of mocking the BaseRequest class it's easy to replace the original with the mock app = Application.builder().token("TOKEN").request(MockRequest).updater(None).build().

Meanwhile, I'm going to work on the entity parser

@elebur FYI telegram supports now MARKDOWN_V2, and i guess there is more MessageEntity's now.

yehuda100 avatar Mar 26 '25 19:03 yehuda100

I've looked through the code of the telegram.Bot and all methods that interacts with the API do it via the _post method (and then it uses the _do_post method). We could rewrite the _post method to intercept all requests. @AlexPHorta , it is similar to your idea.

I'm afraid of using a method marked as private as our main entry point. Isn't it better if we keep the approach of Mockbot and only mock the public methods?

AlexPHorta avatar Mar 27 '25 01:03 AlexPHorta

we need to see if it easy to overwrite just these method, in case of mocking the BaseRequest class it's easy to replace the original with the mock app = Application.builder().token("TOKEN").request(MockRequest).updater(None).build().

If we use this option, I believe the adequate way would be to create a MockRequest(BaseRequest) class. We could use the skeleton of HttpxRequest as inspiration.

But, I'm inclined to just improve on what we already have and expand later.

AlexPHorta avatar Mar 27 '25 01:03 AlexPHorta

I'm afraid of using a method marked as private as our main entry point.

Yes, you are right, it is indeed not the best idea.

@elebur FYI telegram supports now MARKDOWN_V2, and i guess there is more MessageEntity's now.

Thank you! Yes, Markdown V2 has more entities than MD v1. Right now I'm trying to implement a parser for the V1. I want it to be as closer to the original Telegram client as possible (at least I want to try to implement it).

elebur avatar Mar 27 '25 10:03 elebur

So what would be the final decision?

elebur avatar Mar 27 '25 10:03 elebur

Improve what we already have. Throughly testing everything and simplifying things to make sure we can plug other options later. That way we'll have something in a working state soon.

AlexPHorta avatar Mar 27 '25 15:03 AlexPHorta

I'm gonna work on the message generator.

AlexPHorta avatar Mar 27 '25 15:03 AlexPHorta

But, still, I think we need some kind of roadmap. So it will be clear where we are right now and where everything is going to.

elebur avatar Mar 27 '25 16:03 elebur

But, still, I think we need some kind of roadmap. So it will be clear where we are right now and where everything is going to.

yes please 🙏

yehuda100 avatar Mar 27 '25 19:03 yehuda100

https://github.com/ptbtest-dev/ptbtest-echobot

Hi, I created this repository for the first example, echobot. My idea is that we come up with use cases and write code trying to solve those cases. I think we need a MVP.

I'm at work right now, so I can't edit the example code, but feel free to modify it.

PS: I think I didn't explain myself enough. My idea is to make the example a working repository. With working tests, even if basic ones, so we can test the functionality, the deployment, installation process etc.

AlexPHorta avatar Mar 28 '25 12:03 AlexPHorta

As far as I can see almost all network activity happens in the Bot class, so we have to rewrite all methods of the Bot class.

Then ptbtest patches telegram.ext.Application, replaces the original bot with our Mockbot:

building the app

application = Application.builder().token("TOKEN").build() application.add_handler(CommandHandler("start", start)) application.add_handler(CommandHandler("help", help_command)) application.add_handler(MessageHandler(filters.TEXT & ~filters.COMMAND, echo))

mock_app = MockApplication(application) mock_bot = mock_app.bot

Then we do something like mock_app.send_message("/start") and the mock bot responds.

Found this in the tests from PTB. I wonder if we can leverage from it.

python-telegram-bot/tests/ext conftest.py

@pytest.fixture
async def app(bot_info, monkeypatch):
    # We build a new bot each time so that we use `app` in a context manager without problems
    application = (
        ApplicationBuilder()
        .bot(make_bot(bot_info, offline=True)) # This is what I'm interested in.
        .application_class(PytestApplication)
        .build()
    )
    monkeypatch.setattr(application.bot, "delete_webhook", return_true)
    monkeypatch.setattr(application.bot, "set_webhook", return_true)
    yield application
    if application.running:
        await application.stop()
        await application.shutdown()

python-telegram-bot/tests/auxil pytest_classes.py

def make_bot(bot_info=None, offline: bool = True, **kwargs):
    """
    Tests are executed on tg.ext.ExtBot, as that class only extends the functionality of tg.bot
    """
    token = kwargs.pop("token", (bot_info or {}).get("token"))
    private_key = kwargs.pop("private_key", PRIVATE_KEY)
    kwargs.pop("token", None)

    request_class = OfflineRequest if offline else NonchalantHttpxRequest

    return PytestExtBot(
        token=token,
        private_key=private_key if TEST_WITH_OPT_DEPS else None,
        request=request_class(8),
        get_updates_request=request_class(1),
        **kwargs,
    )

And finally, this. python-telegram-bot/tests conftest.py

@pytest.fixture(scope="session")
async def offline_bot(bot_info):
    """Makes an offline Bot instance with the given bot_info
    Note that in tests/ext we also override the `bot` fixture to return the offline bot instead.
    """
    async with make_bot(bot_info, offline=True) as _bot:
        yield _bot

AlexPHorta avatar Mar 30 '25 20:03 AlexPHorta

Hey.

I've been working on the EntityParser for the last few days. Right now I'm working on parser_markdown V1. I want the parser to be as similar to the original one as possible.

The method itself is ready but I'm stuck with tests. While writing tests more and more edge cases appear, yesterday I spent all day on tests.

I want to finish the EntityParser and only then switch on other parts of the PTBtest.

But I've made Markdown V1 only so far and this is the easiest part.

elebur avatar Mar 31 '25 12:03 elebur

@elebur it's a very busy week for me but i'll try to help tomorrow, is the code on your local machine or on the develop branch?

yehuda100 avatar Apr 01 '25 14:04 yehuda100

is the code on your local machine or on the develop branch?

@yehuda100 I sent the PR. It hasn't been merged yet, I have to add more tests.

elebur avatar Apr 01 '25 15:04 elebur