Skip to content

Conversation

Bibo-Joshi
Copy link
Member

Apparently the function load_ssl_context_verify that's called upon the creation of httpx.AsyncClient takes quite a bit of time so that creating new HTTPXRequest objects (indirectly via Bot) is a performance bottleneck in the tests.

Hence, this changing make_bot to use offline=True by default and allowing online only in the tests that make requests.

This is just one small step on test performance improvement, but hey it's something :)

@Bibo-Joshi Bibo-Joshi added the ⚙️ tests affected functionality: tests label Sep 21, 2024
Copy link

codecov bot commented Sep 21, 2024

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
5991 1 5990 271
View the top 1 failed tests by shortest run time
tests.test_bot.TestBotWithRequest test_copy_messages
Stack Traces | 0.48s run time
self = <tests.test_bot.TestBotWithRequest object at 0x000001E49E2BA340>
bot = PytestExtBot[token=695104088:AAHfzylIOjSIIS-eOnI20y2E20HodHsfz-0]
chat_id = '675666224'

    async def test_copy_messages(self, bot, chat_id):
        tasks = asyncio.gather(
            bot.send_message(chat_id, text="will be copied 1"),
            bot.send_message(chat_id, text="will be copied 2"),
        )
        msg1, msg2 = await tasks
    
        copy_messages = await bot.copy_messages(
            chat_id, from_chat_id=chat_id, message_ids=sorted((msg1.message_id, msg2.message_id))
        )
        assert isinstance(copy_messages, tuple)
    
        tasks = asyncio.gather(
            bot.send_message(chat_id, "temp 1", reply_to_message_id=copy_messages[0].message_id),
            bot.send_message(chat_id, "temp 2", reply_to_message_id=copy_messages[1].message_id),
        )
        temp_msg1, temp_msg2 = await tasks
    
        forward_msg1 = temp_msg1.reply_to_message
        forward_msg2 = temp_msg2.reply_to_message
    
>       assert forward_msg1.text == msg1.text
E       AssertionError: assert 'will be copied 2' == 'will be copied 1'
E         
E         - will be copied 1
E         ?                ^
E         + will be copied 2
E         ?                ^

tests\test_bot.py:3962: AssertionError

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know what % of time was the calling of load_ssl_context_verify taking?

@Bibo-Joshi Bibo-Joshi merged commit 79e589b into master Sep 25, 2024
25 checks passed
@Bibo-Joshi Bibo-Joshi deleted the test-performance branch September 25, 2024 15:17
@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⚙️ tests affected functionality: tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants