Skip to content
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

Fix assertion rewrite to match module names correctly #2940

Merged
merged 1 commit into from
Nov 22, 2017

Conversation

nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Nov 22, 2017

Fix #2939

@hroncok, I managed use your reproducer to verify that the fix works:

{env36} X:\setuptools [master]>python -m pytest -k test_unicode_filename_in_sdist -v --tb=line
============================= test session starts =============================
platform win32 -- Python 3.6.0, pytest-3.2.5, py-1.5.2, pluggy-0.4.0 -- X:\setuptools\.env36\Scripts\python.exe
cachedir: .cache
rootdir: X:\setuptools, inifile: pytest.ini
plugins: virtualenv-1.2.11, shutil-1.2.11, flake8-0.9.1
collected 391 items

setuptools/tests/test_easy_install.py::TestEasyInstallTest::test_unicode_filename_in_sdist FAILED

================================== FAILURES ===================================
<frozen importlib._bootstrap>:960: RecursionError: maximum recursion depth exceeded
============================ 390 tests deselected =============================
================== 1 failed, 390 deselected in 2.01 seconds ===================

Using the code from this branch fixes the issue:

{env36} X:\setuptools [master]>pip install -e c:\pytest
Obtaining file:///C:/pytest
Requirement already satisfied: py>=1.4.33 in x:\setuptools\.env36\lib\site-packages (from pytest==3.2.6.dev5+gd13086a)
Requirement already satisfied: setuptools in x:\setuptools\.env36\lib\site-packages (from pytest==3.2.6.dev5+gd13086a)
Requirement already satisfied: colorama in x:\setuptools\.env36\lib\site-packages (from pytest==3.2.6.dev5+gd13086a)
Installing collected packages: pytest
  Found existing installation: pytest 3.2.5
    Uninstalling pytest-3.2.5:
      Successfully uninstalled pytest-3.2.5
  Running setup.py develop for pytest
Successfully installed pytest

{env36} X:\setuptools [master]>python -m pytest -k test_unicode_filename_in_sdist -v --tb=line
============================= test session starts =============================
platform win32 -- Python 3.6.0, pytest-3.2.6.dev5+gd13086a, py-1.5.2, pluggy-0.4.0 -- X:\setuptools\.env36\Scripts\python.exe
cachedir: .cache
rootdir: X:\setuptools, inifile: pytest.ini
plugins: virtualenv-1.2.11, shutil-1.2.11, flake8-0.9.1
collected 391 items

setuptools/tests/test_easy_install.py::TestEasyInstallTest::test_unicode_filename_in_sdist PASSED

============================ 390 tests deselected =============================
================== 1 passed, 390 deselected in 7.82 seconds ===================

Thanks again for taking the time to analyze the issue!

pytest_plugins = "ham"
""",
'ham.py': "",
'hambug.py': "",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love this to be hamster 🐹

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh done!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.258% when pulling d13086a on nicoddemus:rewrite-bug-2939 into 77bd0aa on pytest-dev:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.258% when pulling c8d52b6 on nicoddemus:rewrite-bug-2939 into 77bd0aa on pytest-dev:master.

@The-Compiler
Copy link
Member

The-Compiler commented Nov 22, 2017

LGTM - thanks! 🐹

@The-Compiler The-Compiler merged commit 8df7ed1 into pytest-dev:master Nov 22, 2017
@nicoddemus nicoddemus deleted the rewrite-bug-2939 branch November 22, 2017 08:03
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

Successfully merging this pull request may close these issues.

Assertion rewrite should match files completely and avoid rewriting setup.py
4 participants