Skip to content

path.common: handle FileNotFoundError when trying to import pathlib - #207

Merged
nicoddemus merged 3 commits into
pytest-dev:masterfrom
blueyed:py34-fspath-filenotfound
Apr 5, 2019
Merged

path.common: handle FileNotFoundError when trying to import pathlib#207
nicoddemus merged 3 commits into
pytest-dev:masterfrom
blueyed:py34-fspath-filenotfound

Conversation

@blueyed

@blueyed blueyed commented Feb 13, 2019

Copy link
Copy Markdown
Contributor

Python 3.4 might raise FileNotFoundError due to os.getcwd() failing on
a non-existing cwd. This is fixed in Python 3.5.

Ref: pytest-dev/pytest#4787 (comment)

@blueyed blueyed mentioned this pull request Feb 13, 2019
Merged
@blueyed

blueyed commented Feb 13, 2019

Copy link
Copy Markdown
Contributor Author

Code in py34: https://github.com/python/cpython/blob/fa7193286099f8e4164f4a795afd5ad4a8e229df/Lib/importlib/_bootstrap.py#L1878-L1879

@RonnyPfannschmidt

RonnyPfannschmidt commented Feb 13, 2019

Copy link
Copy Markdown
Member

did this practically happen?

@blueyed

blueyed commented Feb 13, 2019

Copy link
Copy Markdown
Contributor Author

Yes, with py34-xdist on Travis, see the original ref.

@blueyed

blueyed commented Feb 13, 2019

Copy link
Copy Markdown
Contributor Author

@RonnyPfannschmidt
https://github.com/pytest-dev/pytest/pull/4787/files#diff-606d01f030119020f3e820547882b6b0

@RonnyPfannschmidt

RonnyPfannschmidt commented Feb 14, 2019

Copy link
Copy Markdown
Member

Yikes, thanks

@blueyed
blueyed force-pushed the py34-fspath-filenotfound branch from 3b86205 to 1a5f9b9 Compare February 14, 2019 18:13
@blueyed

blueyed commented Feb 14, 2019

Copy link
Copy Markdown
Contributor Author

Rebased.

@blueyed

blueyed commented Feb 14, 2019

Copy link
Copy Markdown
Contributor Author

Pushed a fixup.

I also wonder if caching the ImportError wouldn't make sense?!

@nicoddemus

nicoddemus commented Feb 18, 2019

Copy link
Copy Markdown
Member

Can you please check the travis failure @blueyed?

@blueyed

blueyed commented Feb 18, 2019

Copy link
Copy Markdown
Contributor Author

I think it is related to pytest itself failing with the test.
Will have to investigate.

@blueyed

blueyed commented Feb 18, 2019

Copy link
Copy Markdown
Contributor Author

IIRC AppVeyor skips the test.

nicoddemus
nicoddemus reviewed Feb 19, 2019
View reviewed changes
Comment thread testing/path/test_local.py Outdated
Comment thread testing/path/test_local.py
@nicoddemus nicoddemus mentioned this pull request Feb 19, 2019
Merged
@blueyed
blueyed force-pushed the py34-fspath-filenotfound branch from 427c7b6 to 820aa08 Compare February 22, 2019 00:19
@nicoddemus

nicoddemus commented Feb 22, 2019

Copy link
Copy Markdown
Member

Any clues about the failures on Travis?

@blueyed

blueyed commented Feb 22, 2019

Copy link
Copy Markdown
Contributor Author

Still the same as with #207 (comment) I guess.
No energy to look into this now.

@nicoddemus

nicoddemus commented Feb 22, 2019

Copy link
Copy Markdown
Member

OK. Another option is to drop support for those old pytest versions.

With the "importlib" option getting used in pytest, we will need to require a higher version of py anyway.

@blueyed
blueyed force-pushed the py34-fspath-filenotfound branch from 820aa08 to 74e8a71 Compare February 22, 2019 04:42
@blueyed
blueyed force-pushed the py34-fspath-filenotfound branch from 74e8a71 to d714f41 Compare March 1, 2019 21:19
blueyed added 2 commits April 5, 2019 15:42
@blueyed
path.common: handle FileNotFoundError when trying to import pathlib
33afe63 
Python 3.4 might raise FileNotFoundError due to `os.getcwd()` failing on
a non-existing cwd.  This is fixed in Python 3.5.

Ref: pytest-dev/pytest#4787 (comment)
@blueyed
Handle missing FileNotFoundError
a64cb3f 
Might not really be necessary, since py27 triggers the ImportError
always already, but better to be safe.
@blueyed
blueyed force-pushed the py34-fspath-filenotfound branch from d714f41 to a64cb3f Compare April 5, 2019 13:42
@blueyed

blueyed commented Apr 5, 2019

Copy link
Copy Markdown
Contributor Author

Fixed the new test to cd back (using monkeypatch).

@codecov-io

codecov-io commented Apr 5, 2019

Copy link
Copy Markdown

Codecov Report

Merging #207 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #207      +/-   ##
==========================================
+ Coverage   82.04%   82.07%   +0.03%     
==========================================
  Files          55       55              
  Lines       10136    10153      +17     
  Branches     1141     1141              
==========================================
+ Hits         8316     8333      +17     
  Misses       1556     1556              
  Partials      264      264

@nicoddemus

nicoddemus commented Apr 5, 2019

Copy link
Copy Markdown
Member

Please add a CHANGELOG then it is good to go I think

@blueyed

blueyed commented Apr 5, 2019

Copy link
Copy Markdown
Contributor Author

Pushed with [ci skip] - hopefully that will still trigger it on the merge.

@nicoddemus
nicoddemus merged commit 2802b18 into pytest-dev:master Apr 5, 2019
@blueyed
blueyed deleted the py34-fspath-filenotfound branch April 8, 2019 03:03
@blueyed blueyed mentioned this pull request Apr 26, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nicoddemus nicoddemus nicoddemus left review comments
@RonnyPfannschmidt RonnyPfannschmidt RonnyPfannschmidt approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@blueyed @RonnyPfannschmidt @nicoddemus @codecov-io