Closed BeyondEvil closed 4 years ago
i just took a look, from my pov #9 is a solid solution that changes the import time to a point that reduces cost, i don't think there is another reasonable approach to avoid that import when its not needed
There's an extremely tiny performance hit on doing an import inside a function, as every time you enter the function the import actually runs and basically checks a dict if the module is present and then bails out.
This performance difference doesn't seem to be relevant as it relates to this ticket as the _verify_url
function is only called once per session, so it's the exact same as if it was in the top of the file.
Now, if you just have the plugin installed in the virtualenv, pytest will load it, causing the import to be hit if it's on the top level. This costs ~200ms.
Here's a little demonstration of what I'm talking about:
◰² venv ~/P/scientist virtualenv -p (which python3) venv 303ms Tue May 19 11:31:15 2020
Running virtualenv with interpreter /usr/local/bin/python3
Using base prefix '/usr/local/Cellar/python/3.7.7/Frameworks/Python.framework/Versions/3.7'
New python executable in /Users/andersh/Projects/scientist/venv/bin/python3.7
Also creating executable in /Users/andersh/Projects/scientist/venv/bin/python
Installing setuptools, pip, wheel...
sourcedone.
◰³ venv ~/P/scientist source venv/bin/activate.fish 17.5s Tue May 19 11:31:36 2020
◰³ venv ~/P/scientist pip install hammett Tue May 19 11:32:19 2020
Processing /Users/andersh/Library/Caches/pip/wheels/39/88/68/060e6095f2d869855bfcb8ba17cd88828cfb8fe4ef954c7a20/hammett-0.6.0-cp37-none-any.whl
Collecting astunparse
Using cached astunparse-1.6.3-py2.py3-none-any.whl (12 kB)
Collecting colorama
Using cached colorama-0.4.3-py2.py3-none-any.whl (15 kB)
Requirement already satisfied: wheel<1.0,>=0.23.0 in ./venv/lib/python3.7/site-packages (from astunparse->hammett) (0.34.2)
Collecting six<2.0,>=1.6.1
Using cached six-1.14.0-py2.py3-none-any.whl (10 kB)
Installing collected packages: six, astunparse, colorama, hammett
Successfully installed astunparse-1.6.3 colorama-0.4.3 hammett-0.6.0 six-1.14.0
◰³ venv ~/P/scientist python -m hammett 1754ms Tue May 19 11:32:31 2020
.......
7 succeeded, 0 failed, 0 skipped
◰³ venv ~/P/scientist python -m pytest 101ms Tue May 19 11:32:33 2020
/Users/andersh/Projects/scientist/venv/bin/python: No module named pytest
! ◰³ venv ~/P/scientist pip install pytest Tue May 19 11:32:40 2020
Collecting pytest
Using cached pytest-5.4.2-py3-none-any.whl (247 kB)
Collecting packaging
Downloading packaging-20.4-py2.py3-none-any.whl (37 kB)
Collecting wcwidth
Using cached wcwidth-0.1.9-py2.py3-none-any.whl (19 kB)
Collecting attrs>=17.4.0
Using cached attrs-19.3.0-py2.py3-none-any.whl (39 kB)
Collecting py>=1.5.0
Using cached py-1.8.1-py2.py3-none-any.whl (83 kB)
Collecting importlib-metadata>=0.12; python_version < "3.8"
Using cached importlib_metadata-1.6.0-py2.py3-none-any.whl (30 kB)
Collecting more-itertools>=4.0.0
Using cached more_itertools-8.3.0-py3-none-any.whl (44 kB)
Collecting pluggy<1.0,>=0.12
Using cached pluggy-0.13.1-py2.py3-none-any.whl (18 kB)
Collecting pyparsing>=2.0.2
Using cached pyparsing-2.4.7-py2.py3-none-any.whl (67 kB)
Requirement already satisfied: six in ./venv/lib/python3.7/site-packages (from packaging->pytest) (1.14.0)
Collecting zipp>=0.5
Using cached zipp-3.1.0-py3-none-any.whl (4.9 kB)
Installing collected packages: pyparsing, packaging, wcwidth, attrs, py, zipp, importlib-metadata, more-itertools, pluggy, pytest
Successfully installed attrs-19.3.0 importlib-metadata-1.6.0 more-itertools-8.3.0 packaging-20.4 pluggy-0.13.1 py-1.8.1 pyparsing-2.4.7 pytest-5.4.2 wcwidth-0.1.9 zipp-3.1.0
◰³ venv ~/P/scientist python -m pytest 4448ms Tue May 19 11:32:49 2020
====================================================== test session starts ======================================================
platform darwin -- Python 3.7.7, pytest-5.4.2, py-1.8.1, pluggy-0.13.1
rootdir: /Users/andersh/Projects/scientist
collected 7 items
tests/test_scientist.py ....... [100%]
======================================================= 7 passed in 0.06s =======================================================
◰³ venv ~/P/scientist pip install pytest-base-url 356ms Tue May 19 11:32:51 2020
Collecting pytest-base-url
Downloading pytest_base_url-1.4.1-py2.py3-none-any.whl (7.1 kB)
Collecting requests>=2.9
Downloading requests-2.23.0-py2.py3-none-any.whl (58 kB)
|████████████████████████████████| 58 kB 3.7 MB/s
Requirement already satisfied: pytest>=2.7.3 in ./venv/lib/python3.7/site-packages (from pytest-base-url) (5.4.2)
Collecting certifi>=2017.4.17
Downloading certifi-2020.4.5.1-py2.py3-none-any.whl (157 kB)
|████████████████████████████████| 157 kB 8.6 MB/s
Collecting chardet<4,>=3.0.2
Downloading chardet-3.0.4-py2.py3-none-any.whl (133 kB)
|████████████████████████████████| 133 kB 9.4 MB/s
Collecting idna<3,>=2.5
Downloading idna-2.9-py2.py3-none-any.whl (58 kB)
|████████████████████████████████| 58 kB 11.2 MB/s
Collecting urllib3!=1.25.0,!=1.25.1,<1.26,>=1.21.1
Downloading urllib3-1.25.9-py2.py3-none-any.whl (126 kB)
|████████████████████████████████| 126 kB 8.0 MB/s
Requirement already satisfied: wcwidth in ./venv/lib/python3.7/site-packages (from pytest>=2.7.3->pytest-base-url) (0.1.9)
Requirement already satisfied: pluggy<1.0,>=0.12 in ./venv/lib/python3.7/site-packages (from pytest>=2.7.3->pytest-base-url) (0.13.1)
Requirement already satisfied: packaging in ./venv/lib/python3.7/site-packages (from pytest>=2.7.3->pytest-base-url) (20.4)
Requirement already satisfied: py>=1.5.0 in ./venv/lib/python3.7/site-packages (from pytest>=2.7.3->pytest-base-url) (1.8.1)
Requirement already satisfied: importlib-metadata>=0.12; python_version < "3.8" in ./venv/lib/python3.7/site-packages (from pytest>=2.7.3->pytest-base-url) (1.6.0)
Requirement already satisfied: attrs>=17.4.0 in ./venv/lib/python3.7/site-packages (from pytest>=2.7.3->pytest-base-url) (19.3.0)
Requirement already satisfied: more-itertools>=4.0.0 in ./venv/lib/python3.7/site-packages (from pytest>=2.7.3->pytest-base-url) (8.3.0)
Requirement already satisfied: pyparsing>=2.0.2 in ./venv/lib/python3.7/site-packages (from packaging->pytest>=2.7.3->pytest-base-url) (2.4.7)
Requirement already satisfied: six in ./venv/lib/python3.7/site-packages (from packaging->pytest>=2.7.3->pytest-base-url) (1.14.0)
Requirement already satisfied: zipp>=0.5 in ./venv/lib/python3.7/site-packages (from importlib-metadata>=0.12; python_version < "3.8"->pytest>=2.7.3->pytest-base-url) (3.1.0)
Installing collected packages: certifi, chardet, idna, urllib3, requests, pytest-base-url
Successfully installed certifi-2020.4.5.1 chardet-3.0.4 idna-2.9 pytest-base-url-1.4.1 requests-2.23.0 urllib3-1.25.9
◰³ venv ~/P/scientist python -m pytest 2703ms Tue May 19 11:33:03 2020
====================================================== test session starts ======================================================
platform darwin -- Python 3.7.7, pytest-5.4.2, py-1.8.1, pluggy-0.13.1
rootdir: /Users/andersh/Projects/scientist
plugins: base-url-1.4.1
collected 7 items
tests/test_scientist.py ....... [100%]
======================================================= 7 passed in 0.08s =======================================================
◰³ venv ~/P/scientist 450ms Tue May 19 11:33:05 2020```
If I sound frustrated, I do apologize! I am having this long discussion with many people and if I fail to convince just one person then all my work is for nothing. It's not an ideal situation! :)
I concur with @RonnyPfannschmidt... I think plugins are recommended to avoid costly high-level imports, specially if they might not be used in runtime at all for all tests or even for a few tests, and seems the case for pytest-base-url
which is activated by an option.
So 👍 from me. 😁
Looks like we're lazy loading the requests lib. @boxed 😂
Fantastic! And if you have other similar projects, do keep an eye out for similar things. We're all in this together.
Sooo..... do you reopen the PR?
Sooo..... do you reopen the PR?
NEJ!
🤷♂️
Eh... well that's weird. Ok, well, the diff is really trivial anyway. I don't care about attribution in the git log.
Closed PR (and start of discussion): https://github.com/pytest-dev/pytest-base-url/pull/9
Before I consider implementing the lazy load, I need to understand the time/speed issue 100% and have exhausted all other (resonable) approaches.
@boxed