Skip to content

Conversation

Bibo-Joshi
Copy link
Member

addresses part sof #4324

Example console output before and after renaming:

(venv311) ~\PycharmProjects\python-telegram-bot git:[rename-test-classes]
pytest -v -k NoMatchTest
======================================================================= test session starts =======================================================================
platform win32 -- Python 3.11.9, pytest-8.2.2, pluggy-1.5.0 -- C:\Users\hinri\PycharmProjects\python-telegram-bot\venv311\Scripts\python.exe
cachedir: .pytest_cache
rootdir: C:\Users\hinri\PycharmProjects\python-telegram-bot
configfile: pyproject.toml
testpaths: tests
plugins: anyio-4.4.0, flaky-3.8.1, asyncio-0.21.2, socket-0.7.0, xdist-3.6.1, web3-6.18.0
asyncio: mode=Mode.AUTO
collected 6200 items / 6200 deselected / 0 selected                                                                                                                

==================================================================== 6200 deselected in 17.72s ====================================================================
(venv311) ~\PycharmProjects\python-telegram-bot git:[rename-test-classes]
pytest -v -k NoMatchTest
======================================================================= test session starts =======================================================================
platform win32 -- Python 3.11.9, pytest-8.2.2, pluggy-1.5.0 -- C:\Users\hinri\PycharmProjects\python-telegram-bot\venv311\Scripts\python.exe
cachedir: .pytest_cache
rootdir: C:\Users\hinri\PycharmProjects\python-telegram-bot
configfile: pyproject.toml
testpaths: tests
plugins: anyio-4.4.0, flaky-3.8.1, asyncio-0.21.2, socket-0.7.0, xdist-3.6.1, web3-6.18.0
asyncio: mode=Mode.AUTO
collected 6200 items / 6200 deselected / 0 selected                                                                                                                

==================================================================== 6200 deselected in 12.71s ====================================================================

Note that the number of collected items did not change. What did change is the elapsed time, however I'm not sure if this is really reproducable. In any case, it doesn't hurt to rename these classes and if on average it does speed up the collection that's a good thing :)

@harshil21
Copy link
Member

harshil21 commented Sep 3, 2024

Thanks! I ran hyperfine with pytest's --collect-only, and the results were not conclusive (compared with master):

(python-telegram-bot) ➜  python-telegram-bot git:(master) ✗ hyperfine "pytest --collect-only"
Benchmark 1: pytest --collect-only
  Time (mean ± σ):      2.997 s ±  0.118 s    [User: 2.897 s, System: 0.094 s]
  Range (min … max):    2.928 s …  3.310 s    10 runs
 
  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
 
(python-telegram-bot) ➜  python-telegram-bot git:(master) ✗ git checkout rename-test-classes 
Switched to branch 'rename-test-classes'
Your branch is up to date with 'origin/rename-test-classes'.
(python-telegram-bot) ➜  python-telegram-bot git:(rename-test-classes) ✗ hyperfine "pytest --collect-only"
Benchmark 1: pytest --collect-only
  Time (mean ± σ):      3.207 s ±  0.933 s    [User: 3.107 s, System: 0.095 s]
  Range (min … max):    2.888 s …  5.861 s    10 runs
 
  Warning: The first benchmarking run for this command was significantly slower than the rest (5.861 s). This could be caused by (filesystem) caches that were not filled until after the first run. You should consider using the '--warmup' option to fill those caches before the actual benchmark. Alternatively, use the '--prepare' option to clear the caches before each timing run.

My guess is that those classes are collected because they are being subclassed..

@Bibo-Joshi
Copy link
Member Author

That means that you would prefer not to merge or that you agree with

In any case, it doesn't hurt to rename these classes and if on average it does speed up the collection that's a good thing :)

? :)

@harshil21
Copy link
Member

I agree we should we merge anyway, but was kinda hoping that there was a little speedup for collection.

@harshil21 harshil21 added the ⚙️ tests affected functionality: tests label Sep 3, 2024
@Bibo-Joshi Bibo-Joshi merged commit b9d2efd into master Sep 3, 2024
26 of 27 checks passed
@Bibo-Joshi Bibo-Joshi deleted the rename-test-classes branch September 3, 2024 03:24
@github-actions github-actions bot locked and limited conversation to collaborators Sep 10, 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