Skip to content

xdist v1.26 breaks PyInstaller's test suite for Python 2.7 #414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
htgoebel opened this issue Feb 12, 2019 · 10 comments
Closed

xdist v1.26 breaks PyInstaller's test suite for Python 2.7 #414

htgoebel opened this issue Feb 12, 2019 · 10 comments

Comments

@htgoebel
Copy link

htgoebel commented Feb 12, 2019

PyInstaller's test-suite fails with an import error when xdist 1.26 is used, but only on Python 2.7.

xdist Python 2.7 Python 3.x
1.25.0 pass pass
1.26.0 fail pass
>   ???
E   ImportError: No module named makespec
PyInstaller/__main__.py:78: ImportError
  • As you can see, the trace back does not show the source
  • PyInstaller/__main__.py:78 reads: import PyInstaller.building.makespec.
    • Of course this module exists, otherwise test with other versions would fail, too.
    • Diagnostic output I added shows modules like PyInstaller.building.PyInstaller, PyInstaller.lib.modulegraph.sys, PyInstaller.utils.hooks.PyInstaller and even PyInstaller.utils.pytest - none of which exists.
  • In PyInstaller's main repo failure are different: Most tests pass, but some fail when trying to import PyInstaller.hooks.

I thought, this might be related to #397, but setting PYTHONPATH does not solve the issue.

@nicoddemus
Copy link
Member

Hi @htgoebel,

First of all thanks for pyinstaller, we use it at work extensively and is a great piece of software.

I see htgoebel/pyinstaller@4f8f624#diff-354f30a63fb0907d4ad57269548329e3

That's pretty close to what was changed in #397...

  1. At that point was pip install -e . called so PyInstaller is importable? At a glance it seems like it, but it would be great if you can confirm this.

  2. If you use python -m pytest instead of py.test, does that change the outcome at all?

@htgoebel
Copy link
Author

Hi @nicoddemus

First of all thanks for pyinstaller, we use it at work extensively and is a great piece of software.

May I ask you to consider funding a few days of development?

I see htgoebel/pyinstaller@4f8f624#diff-354f30a63fb0907d4ad57269548329e3
That's pretty close to what was changed in #397...

Yes, I tried to "revert" #397.

re 1) Yes PyInstaller is installed using pip install -e . and the line just below shows that importing works

re 2) This does not solve the issue, see https://travis-ci.org/htgoebel/pyinstaller/jobs/492320342#L1023

@nicoddemus
Copy link
Member

Hi @htgoebel,

May I ask you to consider funding a few days of development?

Will talk with the people that might make it happen. 👍

Yes, I tried to "revert" #397.

By setting PYTHONPATH manually right?

Does this problem also happen on Windows? If it does I can take a look.

@htgoebel
Copy link
Author

Will talk with the people that might make it happen. +1

Great!

By setting PYTHONPATH manually right?

Right, see the change you mentioned in #414 (comment) and can see it here and heere in the travis log.

Does this problem also happen on Windows? If it does I can take a look.

Yes, this also happens on windows, see https://ci.appveyor.com/project/htgoebel/pyinstaller/builds/22317708/job/nl84rdbnwlk74org The symptoms are a bit different (there are other tests which fail), but the problem is the same.

For reproducing please note:

  • When running the whole test-suite,
    tests/functional/test_import.py::test_pkg_without_hook_for_pkg[onefile] and
    tests/functional/test_import.py::test_pkg_without_hook_for_pkg[onedir]
    fail (as you can see in the appveyor log)
  • When only running these tests they pass.
  • When running tests/functional/test_import.py other tests fail, but the symptoms are the same (see the travis logs)

So for speeding up testing I suggest using

  py.test -n 3 --maxfail 3 tests/functional/test_import.py

@nicoddemus
Copy link
Member

Thanks, I will give it a go ASAP. 👍

@nicoddemus
Copy link
Member

Hi @htgoebel,

Managed to reproduce the issue and debug a little.

First thing is that my suspicions are correct and I can confirm if I revert #397 on top of master the tests PASS, so the reason is definitely #397 (good, that makes sense).

I saw your print statements you added to run() for debugging, so I changed them to call a little debug() function which just added the messages to a list, and after the last print() I would call assert 0, '\n'.join(DEBUG_LINES) (still in run()), this way I can see what's happening in each worker.

Here's the difference in sys.path, only the relevant portions:

# WORKS, master with #397 reverted
E       ['None',
E        'X:\\pyinstaller\\tests\\functional',
E        'X:\\pyinstaller',
E        '',
E        'X:\\pyinstaller',
E        'X:\\pytest-xdist',

(Note that the 3d 'X:\\pyinstaller' item is the one added by pytest-xdist).

# FAILS, master
E   ['None',
E    'X:\\pyinstaller\\tests\\functional',
E    '',
E    'X:\\pyinstaller',
E    'X:\\pytest-xdist',

Another interesting difference is on the imported modules:

# WORKS, master with #397 reverted
E       #~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~
E       X:\pyinstaller\PyInstaller\__init__.pyc
E       ['X:\\pyinstaller\\PyInstaller']
E       #~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~
E       X:\pyinstaller\PyInstaller\building\__init__.pyc
E       ['X:\\pyinstaller\\PyInstaller\\building']
# FAILS, master
E   PyInstaller\__init__.pyc
E   ['PyInstaller']
E   #~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~
E   PyInstaller\building\__init__.pyc
E   ['PyInstaller\\building']
E   #~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~#~

So #397 caused the imported modules to get relative paths, instead of absolute ones, because without pytest-xdist adding the absolute x:\pyinstaller, Python is picking '' and generating relative paths.

I noticed a workaround: if I execute from inside the tests directory instead of the root dir, the tests pass.

$ cd tests
$ pytest -n 3 --maxfail 1 functional/test_import.py

Not clear to me yet why this is causing a problem though.

@nicoddemus
Copy link
Member

nicoddemus commented Feb 14, 2019

(I was about to close PyCharm when I decided to take a look at tests/functional/conftest.py):

I noticed these two lines at the top:

_ROOT_DIR = os.path.normpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), '..', '..'))
sys.path.append(_ROOT_DIR)

If I comment out sys.path.append, the test suite fails:

ImportError while loading conftest 'x:\pyinstaller\tests\functional\conftest.py'.
.env27\lib\site-packages\six.py:709: in exec_
    exec("""exec _code_ in _globs_, _locs_""")
tests\functional\conftest.py:50: in <module>
    from PyInstaller import configure, config
E   ImportError: No module named PyInstaller

This is strange, this type of sys.path manipulation should not be necessary given that PyInstaller is in editable mode in my environment:

λ python -c "import PyInstaller;print(PyInstaller.__file__)"
PyInstaller\__init__.pyc

It is late, I will continue this debugging some other time.

@evgeni
Copy link

evgeni commented Sep 27, 2019

Hey @nicoddemus, did you ever have a chance to further debug this?

One interesting side-effect of this issue is that pkg_resources.resource_filename will also return a relative path under test and that broke loading of some data files in our app.

evgeni added a commit to evgeni/obal that referenced this issue Sep 27, 2019
evgeni added a commit to evgeni/obal that referenced this issue Sep 27, 2019
evgeni added a commit to evgeni/obal that referenced this issue Sep 27, 2019
evgeni added a commit to theforeman/obal that referenced this issue Sep 27, 2019
@htgoebel
Copy link
Author

Given that this affects Python 2.7 only, which will be EOL in 3 moths, I'm closing this issuse.

@nicoddemus
Copy link
Member

Hey @nicoddemus, did you ever have a chance to further debug this?

Sorry for lack of response here, unfortunately I did not manage to find the time to look further into it, sorry.

@htgoebel thanks for the follow up. 👍

ehelms pushed a commit to theforeman/obsah that referenced this issue Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants