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

monkeypatch.syspath_prepend: invalidate import cache #5098

Merged
merged 1 commit into from Apr 13, 2019

Conversation

Projects
None yet
3 participants
@blueyed
Copy link
Contributor

blueyed commented Apr 12, 2019

This was done with testdir only, and uses the fixed monkeypatch method
there now.

Fixes pallets/flask#3151.

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Apr 12, 2019

Tried to come up with a test, which would basically have to trigger an invalid cache I guess, but could not..

For reference (trying to mimic Flask's tests):

def test_syspath_prepend_invalidates_cache(testdir):
    p1 = testdir.makepyfile(
        """
        import pytest
        from _pytest import monkeypatch

        @pytest.fixture(scope='session', autouse=True)
        def _standard_os_environ():
            mp = monkeypatch.MonkeyPatch()
            yield
            mp.undo()

        @pytest.fixture
        def purge_module(request):
            def inner(name):
                request.addfinalizer(lambda: sys.modules.pop(name, None))

            return inner

        @pytest.fixture
        def modules_tmpdir(tmpdir, monkeypatch):
            rv = tmpdir.mkdir('modules_tmpdir')
            print("tmpdir", rv)
            monkeypatch.syspath_prepend(str(rv))
            return rv

        def test1(modules_tmpdir):
            import sys
            import main_app
            sys.modules.pop("main_app")
            # with pytest.raises(ImportError):
            #     import main_app

        def test2(modules_tmpdir):
            import os
            print(os.getcwd())
            app = modules_tmpdir.join('main_app.py')
            app.write('print("main_app_imported"); app = True')

            from main_app import app
        """,
        main_app="""
        """
    )
    result = testdir.runpytest("-s", str(p1))
    assert result.ret == 0
    result.stdout.fnmatch_lines([
        "*main_app_imported",
@codecov

This comment has been minimized.

Copy link

codecov bot commented Apr 12, 2019

Codecov Report

Merging #5098 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5098      +/-   ##
==========================================
- Coverage   96.06%   96.06%   -0.01%     
==========================================
  Files         114      114              
  Lines       25786    25784       -2     
  Branches     2549     2550       +1     
==========================================
- Hits        24772    24770       -2     
  Misses        704      704              
  Partials      310      310
Impacted Files Coverage Δ
src/_pytest/pytester.py 91.1% <100%> (-0.13%) ⬇️
src/_pytest/monkeypatch.py 94.23% <100%> (+0.11%) ⬆️
testing/test_monkeypatch.py 98.98% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3b7efc...8fd5a65. Read the comment docs.

@blueyed blueyed force-pushed the blueyed:fix-syspath_prepend branch from 3dcfdcc to 92834e0 Apr 12, 2019

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Apr 12, 2019

Got a test.. \o/
(it requires namespace packages to be in use already)

@nicoddemus
Copy link
Member

nicoddemus left a comment

Great work finding this one!

Show resolved Hide resolved src/_pytest/monkeypatch.py Outdated
@asottile
Copy link
Member

asottile left a comment

please make the change I've suggested before merging -- it is not a functional change

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Apr 12, 2019

please make the change I've suggested before merging -- it is not a functional change

Well, like you said, with that change backported modules might behave different then, no?

@asottile

This comment has been minimized.

Copy link
Member

asottile commented Apr 12, 2019

please make the change I've suggested before merging -- it is not a functional change

Well, like you said, with that change backported modules might behave different then, no?

yeah, your current code is much more likely to crash

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Apr 12, 2019

yeah, your current code is much more likely to crash

This is moved only.

@asottile

This comment has been minimized.

Copy link
Member

asottile commented Apr 12, 2019

yeah, your current code is much more likely to crash

This is moved only.

All the more reason to improve it at the same time :)

@blueyed

This comment has been minimized.

Copy link
Contributor Author

blueyed commented Apr 12, 2019

But not necessarily for a bugfix.
(I've thought using the existing monkeypatch in the first place should be avoided, but skipped that then)

@nicoddemus

This comment has been minimized.

Copy link
Member

nicoddemus commented Apr 13, 2019

IMHO: this code is fine to go into master as it is a bug fix, and the change requested by @asottile is reasonable as well. 😁

monkeypatch.syspath_prepend: invalidate import cache
This was done with testdir only, and uses the fixed monkeypatch method
there now.

@blueyed blueyed force-pushed the blueyed:fix-syspath_prepend branch from a0e9616 to 8fd5a65 Apr 13, 2019

@asottile
Copy link
Member

asottile left a comment

@blueyed blueyed merged commit 533e610 into pytest-dev:master Apr 13, 2019

4 checks passed

codecov/patch 100% of diff hit (target 96.06%)
Details
codecov/project Absolute coverage decreased by -<.01% but relative coverage increased by +3.93% compared to c3b7efc
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
pytest-CI #20190413.2 succeeded
Details

@blueyed blueyed deleted the blueyed:fix-syspath_prepend branch Apr 13, 2019

M157q added a commit to zdict/zdict that referenced this pull request Apr 15, 2019

Update pytest to 4.4.1 (#337)
This PR updates [pytest](https://pypi.org/project/pytest) from **4.4.0** to **4.4.1**.



<details>
  <summary>Changelog</summary>
  
  
   ### 4.4.1
   ```
   =========================

Bug Fixes
---------

- `5031 &lt;https://github.com/pytest-dev/pytest/issues/5031&gt;`_: Environment variables are properly restored when using pytester&#39;s ``testdir`` fixture.


- `5039 &lt;https://github.com/pytest-dev/pytest/issues/5039&gt;`_: Fix regression with ``--pdbcls``, which stopped working with local modules in 4.0.0.


- `5092 &lt;https://github.com/pytest-dev/pytest/issues/5092&gt;`_: Produce a warning when unknown keywords are passed to ``pytest.param(...)``.


- `5098 &lt;https://github.com/pytest-dev/pytest/issues/5098&gt;`_: Invalidate import caches with ``monkeypatch.syspath_prepend``, which is required with namespace packages being used.
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pytest
  - Changelog: https://pyup.io/changelogs/pytest/
  - Homepage: https://docs.pytest.org/en/latest/
</details>

bors bot added a commit to rehandalal/therapist that referenced this pull request Apr 17, 2019

Merge #67
67: Update pytest to 4.4.1 r=rehandalal a=pyup-bot


This PR updates [pytest](https://pypi.org/project/pytest) from **4.4.0** to **4.4.1**.



<details>
  <summary>Changelog</summary>
  
  
   ### 4.4.1
   ```
   =========================

Bug Fixes
---------

- `5031 &lt;https://github.com/pytest-dev/pytest/issues/5031&gt;`_: Environment variables are properly restored when using pytester&#39;s ``testdir`` fixture.


- `5039 &lt;https://github.com/pytest-dev/pytest/issues/5039&gt;`_: Fix regression with ``--pdbcls``, which stopped working with local modules in 4.0.0.


- `5092 &lt;https://github.com/pytest-dev/pytest/issues/5092&gt;`_: Produce a warning when unknown keywords are passed to ``pytest.param(...)``.


- `5098 &lt;https://github.com/pytest-dev/pytest/issues/5098&gt;`_: Invalidate import caches with ``monkeypatch.syspath_prepend``, which is required with namespace packages being used.
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pytest
  - Changelog: https://pyup.io/changelogs/pytest/
  - Homepage: https://docs.pytest.org/en/latest/
</details>



Co-authored-by: pyup-bot <github-bot@pyup.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.