Closed kloczek closed 5 months ago
davidhalter
commented
1 year ago No idea why this happens, but I feel like you could probably also skip this test. Feel free to PR this if you realize what's wrong. I won't be able to reproduce this I guess.
kloczek
commented
1 year ago I;ve added for now that unit to --deselect list. Are you able to reporosuce that issue? 🤔
davidhalter
commented
1 year ago I'm not trying to reproduce it, because I feel like this is such a rare problem, that you have to debug it yourself.
kloczek
commented
1 year ago Updated output after update build env
Found issue:
pkg_resources modulepython -m <foo> - pytest and venv should not be used like that because when python executes python -m <foo> it adds by default current path to sys.pathkloczek
commented
1 year ago I found that test/test_api/test_environment.py units have been ailing with error because I had installed locally .whl archoives of pip and setuptools.
After that I found still those units have been failing because it is hardcoded python executable name.
Here is proposition of the fix for that issue
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -118,13 +118,8 @@
"print(sys.real_prefix if hasattr(sys, 'real_prefix') else sys.base_prefix)"
])
prefix = output.rstrip().decode('utf8')
- if os.name == 'nt':
- executable_path = os.path.join(prefix, 'python')
- else:
- executable_name = os.path.basename(environment.executable)
- executable_path = os.path.join(prefix, 'bin', executable_name)
- return_code = subprocess.call([executable_path, '-m', 'venv', dirname])
+ return_code = subprocess.call([sys.executable, '-m', 'venv', dirname])
assert return_code == 0, return_code
return dirnamePlease let me know if you want this as PR.
Still needs to be fixed issue with use deprecated pkg_resources module.
kloczek
commented
1 year ago Still I have one unit failing
========================================================================================= FAILURES ==========================================================================================
____________________________________________________________________________________ test_venv_and_pths _____________________________________________________________________________________
venv_path = '/tmp/pytest-of-tkloczko/pytest-32/venv_path0/venv'
def test_venv_and_pths(venv_path):
pjoin = os.path.join
site_pkg_path = pjoin(venv_path, 'lib')
if os.name == 'nt':
site_pkg_path = pjoin(site_pkg_path, 'site-packages')
else:
site_pkg_path = glob(pjoin(site_pkg_path, 'python*', 'site-packages'))[0]
shutil.rmtree(site_pkg_path)
shutil.copytree(get_example_dir('sample_venvs', 'pth_directory'), site_pkg_path)
virtualenv = create_environment(venv_path)
venv_paths = virtualenv.get_sys_path()
ETALON = [
# For now disable egg-links. I have no idea how they work... ~ dave
#pjoin('/path', 'from', 'egg-link'),
#pjoin(site_pkg_path, '.', 'relative', 'egg-link', 'path'),
site_pkg_path,
pjoin(site_pkg_path, 'dir-from-foo-pth'),
'/foo/smth.py:module',
# Not sure why it's added twice. It has to do with site.py which is not
# something we can change. However this obviously also doesn't matter.
'/foo/smth.py:from_func',
'/foo/smth.py:from_func',
]
# Ensure that pth and egg-link paths were added.
> assert venv_paths[-len(ETALON):] == ETALON
E AssertionError: assert ['/tmp/pytest...py:from_func'] == ['/tmp/pytest...py:from_func']
E At index 2 diff: '/foo/smth.py:from_func' != '/foo/smth.py:module'
E Use -v to get more diff
test/test_inference/test_sys_path.py:61: AssertionError
===================================================================================== warnings summary ======================================================================================
test/test_integration.py::test_completion[conftest:27]
/home/tkloczko/rpmbuild/BUILD/jedi-0.18.2/jedi/plugins/pytest.py:140: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
from pkg_resources import iter_entry_points
test/test_integration.py::test_completion[conftest:27]
test/test_integration.py::test_completion[conftest:27]
test/test_integration.py::test_completion[conftest:27]
test/test_integration.py::test_completion[conftest:27]
test/test_integration.py::test_completion[conftest:27]
/usr/lib/python3.8/site-packages/pkg_resources/__init__.py:2871: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('sphinxcontrib')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
declare_namespace(pkg)
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================================================================== short test summary info ==================================================================================
SKIPPED [86] test/test_integration.py:45: Needs django to be installed to run this test.
SKIPPED [3] test/test_api/test_environment.py:38: Skipped
SKIPPED [5] test/test_inference/test_signature.py:380: attrs needed in target environment to run this test
SKIPPED [2] test/test_inference/test_sys_path.py:70: condition: sys.platform!='win32'
XFAIL test/test_inference/test_imports.py::test_flask_ext[import flask.ext.foo; flask.ext.foo.-Foo]
XFAIL test/test_inference/test_imports.py::test_flask_ext[import flask.ext.bar; flask.ext.bar.-Foo]
XFAIL test/test_inference/test_imports.py::test_flask_ext[import flask.ext.baz; flask.ext.baz.-Foo]
XFAIL test/test_inference/test_imports.py::test_flask_ext[import flask.ext.moo; flask.ext.moo.-Foo]
XFAIL test/test_inference/test_precedence.py::test_equals[... == ...]
FAILED test/test_inference/test_sys_path.py::test_venv_and_pths - AssertionError: assert ['/tmp/pytest...py:from_func'] == ['/tmp/pytest...py:from_func']
======================================================= 1 failed, 3761 passed, 96 skipped, 5 xfailed, 6 warnings in 99.41s (0:01:39) ========================================================
/usr/lib/python3.8/site-packages/_pytest/pathlib.py:95: PytestWarning: (rm_rf) error removing /tmp/pytest-of-tkloczko/garbage-653ff4bc-9549-451f-905c-5eaa6497e007/test_rmtree_errorhandler_rerai0
<class 'OSError'>: [Errno 39] Directory not empty: 'test_rmtree_errorhandler_rerai0'
warnings.warn(
/usr/lib/python3.8/site-packages/_pytest/pathlib.py:95: PytestWarning: (rm_rf) error removing /tmp/pytest-of-tkloczko/garbage-653ff4bc-9549-451f-905c-5eaa6497e007/test_rmtree_errorhandler_reado0
<class 'OSError'>: [Errno 39] Directory not empty: 'test_rmtree_errorhandler_reado0'
warnings.warn(
/usr/lib/python3.8/site-packages/_pytest/pathlib.py:95: PytestWarning: (rm_rf) error removing /tmp/pytest-of-tkloczko/garbage-653ff4bc-9549-451f-905c-5eaa6497e007/test_safe_delete_no_perms0
<class 'OSError'>: [Errno 39] Directory not empty: 'test_safe_delete_no_perms0'
warnings.warn(
/usr/lib/python3.8/site-packages/_pytest/pathlib.py:95: PytestWarning: (rm_rf) error removing /tmp/pytest-of-tkloczko/garbage-653ff4bc-9549-451f-905c-5eaa6497e007/test_safe_set_no_perms0
<class 'OSError'>: [Errno 39] Directory not empty: 'test_safe_set_no_perms0'
warnings.warn(
/usr/lib/python3.8/site-packages/_pytest/pathlib.py:95: PytestWarning: (rm_rf) error removing /tmp/pytest-of-tkloczko/garbage-653ff4bc-9549-451f-905c-5eaa6497e007/test_safe_get_no_perms0
<class 'OSError'>: [Errno 39] Directory not empty: 'test_safe_get_no_perms0'
warnings.warn(
/usr/lib/python3.8/site-packages/_pytest/pathlib.py:95: PytestWarning: (rm_rf) error removing /tmp/pytest-of-tkloczko/garbage-653ff4bc-9549-451f-905c-5eaa6497e007
<class 'OSError'>: [Errno 39] Directory not empty: '/tmp/pytest-of-tkloczko/garbage-653ff4bc-9549-451f-905c-5eaa6497e007'
warnings.warn(I'm not sure but in this case instead using special procedure for nt it shoudl be used directly what provides:
[tkloczko@pers-jacek SPECS]$ python3 -Ic "import sysconfig; print(sysconfig.get_path('platlib'))"
/usr/lib64/python3.8/site-packages
[tkloczko@pers-jacek SPECS]$ python3 -Ic "import sysconfig; print(sysconfig.get_path('purelib'))"
/usr/lib/python3.8/site-packagesSorry, but I don't understand at all what you're referring to.
kloczek
commented
1 year ago I\m talking about code fragment which pytest dumped is in fail log
def test_venv_and_pths(venv_path):
pjoin = os.path.join
site_pkg_path = pjoin(venv_path, 'lib')
if os.name == 'nt':
site_pkg_path = pjoin(site_pkg_path, 'site-packages')
else:
site_pkg_path = glob(pjoin(site_pkg_path, 'python*', 'site-packages'))[0]
shutil.rmtree(site_pkg_path)
shutil.copytree(get_example_dir('sample_venvs', 'pth_directory'), site_pkg_path)
virtualenv = create_environment(venv_path)
venv_paths = virtualenv.get_sys_path()I still have no idea what you mean.
kloczek
commented
1 year ago In quoted fragment of the code are assembled paths which could be taken directly from sysconfig.
davidhalter
commented
1 year ago If that helps, I guess that's fine. Feel free to send a pull request and I will check if CI passes (and if it works on my machine).
kloczek
commented
5 months ago I just tested 0.19.1 and looks like now most of the units are failing because it is hardcoded python interpreter name as python3.9.
In such cases it should be used rather sys.executable.
Almost all units are failing the same way 🤔
kloczek
commented
5 months ago With below patch:
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -118,13 +118,8 @@
"print(sys.real_prefix if hasattr(sys, 'real_prefix') else sys.base_prefix)"
])
prefix = output.rstrip().decode('utf8')
- if os.name == 'nt':
- executable_path = os.path.join(prefix, 'python')
- else:
- executable_name = os.path.basename(environment.executable)
- executable_path = os.path.join(prefix, 'bin', executable_name)
- return_code = subprocess.call([executable_path, '-m', 'venv', dirname])
+ return_code = subprocess.call([sys.executable, '-m', 'venv', dirname])
assert return_code == 0, return_code
return dirname
--- a/jedi/api/environment.py
+++ b/jedi/api/environment.py
@@ -338,7 +338,7 @@
:raises: :exc:`.InvalidPythonEnvironment`
:returns: :class:`.Environment`
"""
- exe = which('python' + version)
+ exe = sys.executable
if exe:
if exe == sys.executable:
return SameEnvironment()I was able to pass almost all units
davidhalter
commented
5 months ago This patch is definitely wrong, because it disables multi Python version Jedi.
I'm very confused what you want to do and where you have issues. It's also not better that there are now a lot of different pytest outputs for different versions that point to different potential problems.
kloczek
commented
5 months ago This patch is definitely wrong, because it disables multi Python version Jedi.
Which is a bit useless in typical OS distribution which tries provide proper exact python major version support.
If there is no during testing anything else than executable name taken from sys.executable probably better would be skip those units.
Issue in this case is that you assumed that python executable name will be versioned which is not typical case. The same is in case of of other pyton versions executables. Current procedure blindly assumes that those executables are available (there is no checking are they available).
I'm not trying to tell that this patch which I've copied is correct but it is OK typical distro packages build procedure conditions. In other words that patch is typical JFDI solution and I think that it would be good to have some modification of current testing procedure which may be working OOTB in such conditions. Dropping that patch I've only presented in which direction those conditions are going.
I'm very confused what you want to do and where you have issues. It's also not better that there are now a lot of different pytest outputs for different versions that point to different potential problems.
Yes, because in mean time I've moved to python 3.9 and latest pytest 8.1.1.
davidhalter
commented
5 months ago The point is that everything currently works in CI. It works on Windows, Ubuntu and MacOS and probably most other Linux derivates (since I haven't had any complaints except yours). So I really don't know what to do here and I still don't know what the problem is. I'm closing.
kloczek
commented
5 months ago The point is that everything currently works in CI.
Point is that I'm not been reporting something what happens in this repo CI and if you will look closer on fedora spec files you can find few units excluded because the yare failing https://src.fedoraproject.org/rpms/python-jedi/blob/rawhide/f/python-jedi.spec
I'm packaging your module as an rpm package so I'm using the typical PEP517 based build, install and test cycle used on building packages from non-root account.
python3 -sBm build -w --no-isolationbuildwith--no-isolationI'm using during all processes only locally installed modulesHere is pytest output:
Here is list of installed modules in build env