slackapi / bolt-python

A framework to build Slack apps using Python
https://slack.dev/bolt-python/
MIT License
1.03k stars 237 forks source link

feat: improve test speed #1017

Closed WilliamBergamin closed 5 months ago

WilliamBergamin commented 6 months ago

Flow up to the discussion started in #1015

This PR aims to improvement on the mock_web_api_server by making it more robust and attempts to reduce the time spent sleeping in test ("Run tests without aiohttp")

This is achieved by incorporating a solution that allows for waiting up to a specified duration (X amount of time) to confirm the receipt of requests, instead of waiting for a fixed period and then confirming the reception of requests.

Previously, the test located in "Run tests without aiohttp" required 93 seconds for execution. With these changes, the tests can now be completed in just 24 seconds, resulting in a nearly fourfold reduction in wait time.

Similar changes can be applied to asynchronous test and similar reduction in wait time can also be expected for this as well

NOTE: adding content length header to the mock_web_api_server drastically improved request reliability

Before

Screenshot 2024-02-07 at 11 17 07 AM

After

Screenshot 2024-02-07 at 11 17 28 AM

Feedback

The code can be made clearer and there may be some changes that are not necessary, please point out anything that seems relevant

Is this approach worth implementing?

Category (place an x in each of the [ ])

Requirements (place an x in each [ ])

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (69e033f) 91.76% compared to head (7af76f9) 91.76%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1017 +/- ## ======================================= Coverage 91.76% 91.76% ======================================= Files 181 181 Lines 6312 6312 ======================================= Hits 5792 5792 Misses 520 520 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

WilliamBergamin commented 5 months ago

@seratch I understood why some assert_auth_test_count need to be asserted as being called twice instead of once

  1. The auth test is performed for the bot token and the user token in InstallationStoreAuthorize
  2. Many tests use a variation of this fake MemoryInstallationStore implementation, where a user_token is hard coded into the installation returned by find_installation

I'm assuming the changes regarding the auth_test assertion count now reflect the behavior of bolt python