-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Replace all tmpdir fixtures with tmp_path (#3551) #3955
Conversation
tmp_path is the replacement fixture in pytest for tmpdir; tmp_path uses the builtin pathlib.Path class. As it says on the tin, this commit replaces every instance of tmpdir in the test suite with tmp_path. Aside from s/tmpdir/tmp_path/ this also required changing instances of `tmpdir.join(foo)` to `tmp_path / foo`. This is intended to comprehensively address #3551 and should have no side effects.
Codecov Report
@@ Coverage Diff @@
## master #3955 +/- ##
==========================================
- Coverage 97.77% 97.75% -0.03%
==========================================
Files 43 43
Lines 8761 8761
Branches 1375 1375
==========================================
- Hits 8566 8564 -2
- Misses 83 84 +1
- Partials 112 113 +1
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #3955 +/- ##
==========================================
- Coverage 97.77% 97.75% -0.03%
==========================================
Files 43 43
Lines 8761 8761
Branches 1375 1375
==========================================
- Hits 8566 8564 -2
- Misses 83 84 +1
- Partials 112 113 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@@ -802,7 +802,7 @@ def test_static_route_path_existence_check() -> None: | |||
tr = request.transport | |||
sock = tr.get_extra_info('socket') | |||
sock.setsockopt(socket.SOL_SOCKET, socket.SO_SNDBUF, 1024) | |||
ret = web.FileResponse(pathlib.Path(str(tmpdir.join(filename)))) | |||
ret = web.FileResponse(pathlib.Path(str((tmp_path / filename)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here and in a few other places now the path is turned into string and than back to Path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I've missed this.
Please feel free to make a PR for the fix.
The problem is minor though if all lines belong to tests only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'm working on a PR now to more comprehensively address path handling across the test suite.
Mostly that's replacing os.path
calls with their appropriate pathlib.Path
counterparts.
There are also a couple of uses of ad-hoc temporary directories that I'm updating to tmp_path
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ironically, to support 3.5 some pathlib.Path(str(tmp_path)))
turns out to be necessary.
pytest uses pathlib2
to provide tmp_path
for Python 3.5, which is mostly fine but it does create a couple of corner cases of incompatibility where pathlib2.Path
fails isinstance checks that demand str
or PurePath
. See pytest-dev/pytest/issues/5017
* Improve test suite handling of paths, temp files This updates most uses of `os.path` to instead use `pathlib.Path`. Relatedly, and following up from #3955 (which replaced pytest's `tmpdir` fixture with `tmp_path`), this removes most ad-hoc tempfile creation in favor of the `tmp_path` fixture. Following conversion, unnecessary `os` and `tempfile` imports were removed. Most pathlib changes involve straightforward changes from `os` functions such as `os.mkdir` or `os.path.abspath` to their equivalent methods in `pathlib.Path`. Changing ad-hoc temporary path to `tmp_path` involved removing the `tmp_dir_path` fixture and replacing its functionality with `tmp_path` in `test_save_load` and `test_guess_filename_with_tempfile`. On `test_static_route_user_home` function: * I think that the intention of this test is to ensure that aiohttp correctly expands the home path if passed in a string. I refactored it to `pathlib.Path` and cut out duplication of `relative_to()` calls. But if it's not doing anything but expanding `~`, then it's testing the functionality of `pathlib.Path`, not aiohttp. On `unix_sockname` fixture: This fixture uses `tempfile.TemporaryDirectory`. Because it's a somewhat complicated fixture used across multiple test modules, I left it as-is for now. On `str(tmp_path)` and even `pathlib.Path(str(tmp_path))`: pytest uses `pathlib2` to provide `tmp_path` for Python 3.5 (only). This is mostly fine but it fails on a couple of corner cases, such as `os.symlink()` which blocks all but `str` and `PurePath` via isinstance type checking. In several cases, this requires conversion to string or conversion to string and then into `pathlib.Path` to maintain code compatibility. See: pytest-dev/pytest/issues/5017 * Correct test_guess_filename to use file object * Update symlink in tests; more guess_filename tests
Backport to 3.10: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 8960063 on top of patchback/backports/3.10/8960063ef4137d6c547a687a45ed55b943e9b8d1/pr-3955 Backporting merged PR #3955 into master
🤖 @patchback |
Backport to 3.9: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 8960063 on top of patchback/backports/3.9/8960063ef4137d6c547a687a45ed55b943e9b8d1/pr-3955 Backporting merged PR #3955 into master
🤖 @patchback |
) tmp_path is the replacement fixture in pytest for tmpdir; tmp_path uses the builtin pathlib.Path class. As it says on the tin, this commit replaces every instance of tmpdir in the test suite with tmp_path. Aside from s/tmpdir/tmp_path/ this also required changing instances of `tmpdir.join(foo)` to `tmp_path / foo`. This is intended to comprehensively address aio-libs#3551 and should have no side effects. (cherry picked from commit 8960063)
) tmp_path is the replacement fixture in pytest for tmpdir; tmp_path uses the builtin pathlib.Path class. As it says on the tin, this commit replaces every instance of tmpdir in the test suite with tmp_path. Aside from s/tmpdir/tmp_path/ this also required changing instances of `tmpdir.join(foo)` to `tmp_path / foo`. This is intended to comprehensively address aio-libs#3551 and should have no side effects. (cherry picked from commit 8960063)
) tmp_path is the replacement fixture in pytest for tmpdir; tmp_path uses the builtin pathlib.Path class. As it says on the tin, this commit replaces every instance of tmpdir in the test suite with tmp_path. Aside from s/tmpdir/tmp_path/ this also required changing instances of `tmpdir.join(foo)` to `tmp_path / foo`. This is intended to comprehensively address aio-libs#3551 and should have no side effects. (cherry picked from commit 8960063)
) tmp_path is the replacement fixture in pytest for tmpdir; tmp_path uses the builtin pathlib.Path class. As it says on the tin, this commit replaces every instance of tmpdir in the test suite with tmp_path. Aside from s/tmpdir/tmp_path/ this also required changing instances of `tmpdir.join(foo)` to `tmp_path / foo`. This is intended to comprehensively address aio-libs#3551 and should have no side effects. (cherry picked from commit 8960063)
…mp_path (#3551) (#8075) **This is a backport of PR #3955 as merged into master (8960063).** tmp_path is the replacement fixture in pytest for tmpdir; tmp_path uses the builtin pathlib.Path class. As it says on the tin, this commit replaces every instance of tmpdir in the test suite with tmp_path. Aside from s/tmpdir/tmp_path/ this also required changing instances of `tmpdir.join(foo)` to `tmp_path / foo`. This is intended to comprehensively address and close #3551, and should have no side effects. This does not affect end users. Co-authored-by: Matt VanEseltine <[email protected]>
…p_path (#3551) (#8076) **This is a backport of PR #3955 as merged into master (8960063).** tmp_path is the replacement fixture in pytest for tmpdir; tmp_path uses the builtin pathlib.Path class. As it says on the tin, this commit replaces every instance of tmpdir in the test suite with tmp_path. Aside from s/tmpdir/tmp_path/ this also required changing instances of `tmpdir.join(foo)` to `tmp_path / foo`. This is intended to comprehensively address and close #3551, and should have no side effects. This does not affect end users. Co-authored-by: Matt VanEseltine <[email protected]>
* Improve test suite handling of paths, temp files This updates most uses of `os.path` to instead use `pathlib.Path`. Relatedly, and following up from aio-libs#3955 (which replaced pytest's `tmpdir` fixture with `tmp_path`), this removes most ad-hoc tempfile creation in favor of the `tmp_path` fixture. Following conversion, unnecessary `os` and `tempfile` imports were removed. Most pathlib changes involve straightforward changes from `os` functions such as `os.mkdir` or `os.path.abspath` to their equivalent methods in `pathlib.Path`. Changing ad-hoc temporary path to `tmp_path` involved removing the `tmp_dir_path` fixture and replacing its functionality with `tmp_path` in `test_save_load` and `test_guess_filename_with_tempfile`. On `test_static_route_user_home` function: * I think that the intention of this test is to ensure that aiohttp correctly expands the home path if passed in a string. I refactored it to `pathlib.Path` and cut out duplication of `relative_to()` calls. But if it's not doing anything but expanding `~`, then it's testing the functionality of `pathlib.Path`, not aiohttp. On `unix_sockname` fixture: This fixture uses `tempfile.TemporaryDirectory`. Because it's a somewhat complicated fixture used across multiple test modules, I left it as-is for now. On `str(tmp_path)` and even `pathlib.Path(str(tmp_path))`: pytest uses `pathlib2` to provide `tmp_path` for Python 3.5 (only). This is mostly fine but it fails on a couple of corner cases, such as `os.symlink()` which blocks all but `str` and `PurePath` via isinstance type checking. In several cases, this requires conversion to string or conversion to string and then into `pathlib.Path` to maintain code compatibility. See: pytest-dev/pytest/issues/5017 * Correct test_guess_filename to use file object * Update symlink in tests; more guess_filename tests (cherry picked from commit 79fe204)
* Improve test suite handling of paths, temp files This updates most uses of `os.path` to instead use `pathlib.Path`. Relatedly, and following up from aio-libs#3955 (which replaced pytest's `tmpdir` fixture with `tmp_path`), this removes most ad-hoc tempfile creation in favor of the `tmp_path` fixture. Following conversion, unnecessary `os` and `tempfile` imports were removed. Most pathlib changes involve straightforward changes from `os` functions such as `os.mkdir` or `os.path.abspath` to their equivalent methods in `pathlib.Path`. Changing ad-hoc temporary path to `tmp_path` involved removing the `tmp_dir_path` fixture and replacing its functionality with `tmp_path` in `test_save_load` and `test_guess_filename_with_tempfile`. On `test_static_route_user_home` function: * I think that the intention of this test is to ensure that aiohttp correctly expands the home path if passed in a string. I refactored it to `pathlib.Path` and cut out duplication of `relative_to()` calls. But if it's not doing anything but expanding `~`, then it's testing the functionality of `pathlib.Path`, not aiohttp. On `unix_sockname` fixture: This fixture uses `tempfile.TemporaryDirectory`. Because it's a somewhat complicated fixture used across multiple test modules, I left it as-is for now. On `str(tmp_path)` and even `pathlib.Path(str(tmp_path))`: pytest uses `pathlib2` to provide `tmp_path` for Python 3.5 (only). This is mostly fine but it fails on a couple of corner cases, such as `os.symlink()` which blocks all but `str` and `PurePath` via isinstance type checking. In several cases, this requires conversion to string or conversion to string and then into `pathlib.Path` to maintain code compatibility. See: pytest-dev/pytest/issues/5017 * Correct test_guess_filename to use file object * Update symlink in tests; more guess_filename tests (cherry picked from commit 79fe204)
* Improve test suite handling of paths, temp files This updates most uses of `os.path` to instead use `pathlib.Path`. Relatedly, and following up from aio-libs#3955 (which replaced pytest's `tmpdir` fixture with `tmp_path`), this removes most ad-hoc tempfile creation in favor of the `tmp_path` fixture. Following conversion, unnecessary `os` and `tempfile` imports were removed. Most pathlib changes involve straightforward changes from `os` functions such as `os.mkdir` or `os.path.abspath` to their equivalent methods in `pathlib.Path`. Changing ad-hoc temporary path to `tmp_path` involved removing the `tmp_dir_path` fixture and replacing its functionality with `tmp_path` in `test_save_load` and `test_guess_filename_with_tempfile`. On `test_static_route_user_home` function: * I think that the intention of this test is to ensure that aiohttp correctly expands the home path if passed in a string. I refactored it to `pathlib.Path` and cut out duplication of `relative_to()` calls. But if it's not doing anything but expanding `~`, then it's testing the functionality of `pathlib.Path`, not aiohttp. On `unix_sockname` fixture: This fixture uses `tempfile.TemporaryDirectory`. Because it's a somewhat complicated fixture used across multiple test modules, I left it as-is for now. On `str(tmp_path)` and even `pathlib.Path(str(tmp_path))`: pytest uses `pathlib2` to provide `tmp_path` for Python 3.5 (only). This is mostly fine but it fails on a couple of corner cases, such as `os.symlink()` which blocks all but `str` and `PurePath` via isinstance type checking. In several cases, this requires conversion to string or conversion to string and then into `pathlib.Path` to maintain code compatibility. See: pytest-dev/pytest/issues/5017 * Correct test_guess_filename to use file object * Update symlink in tests; more guess_filename tests (cherry picked from commit 79fe204)
…hs, temp files (#8083) **This is a backport of PR #3957 as merged into master (79fe204).** * Improve test suite handling of paths, temp files This updates most uses of `os.path` to instead use `pathlib.Path`. Relatedly, and following up from #3955 (which replaced pytest's `tmpdir` fixture with `tmp_path`), this removes most ad-hoc tempfile creation in favor of the `tmp_path` fixture. Following conversion, unnecessary `os` and `tempfile` imports were removed. Most pathlib changes involve straightforward changes from `os` functions such as `os.mkdir` or `os.path.abspath` to their equivalent methods in `pathlib.Path`. Changing ad-hoc temporary path to `tmp_path` involved removing the `tmp_dir_path` fixture and replacing its functionality with `tmp_path` in `test_save_load` and `test_guess_filename_with_tempfile`. On `test_static_route_user_home` function: * I think that the intention of this test is to ensure that aiohttp correctly expands the home path if passed in a string. I refactored it to `pathlib.Path` and cut out duplication of `relative_to()` calls. But if it's not doing anything but expanding `~`, then it's testing the functionality of `pathlib.Path`, not aiohttp. On `unix_sockname` fixture: This fixture uses `tempfile.TemporaryDirectory`. Because it's a somewhat complicated fixture used across multiple test modules, I left it as-is for now. On `str(tmp_path)` and even `pathlib.Path(str(tmp_path))`: pytest uses `pathlib2` to provide `tmp_path` for Python 3.5 (only). This is mostly fine but it fails on a couple of corner cases, such as `os.symlink()` which blocks all but `str` and `PurePath` via isinstance type checking. In several cases, this requires conversion to string or conversion to string and then into `pathlib.Path` to maintain code compatibility. See: pytest-dev/pytest/issues/5017 * Correct test_guess_filename to use file object * Update symlink in tests; more guess_filename tests (cherry picked from commit 79fe204) <!-- Thank you for your contribution! --> ## What do these changes do? This updates most uses of `os.path` to instead use `pathlib.Path`. Relatedly, and following up from #3955 (which replaced pytest's `tmpdir` fixture with `tmp_path`), this removes most ad-hoc tempfile creation in favor of the `tmp_path` fixture. Following conversion, unnecessary `os` and `tempfile` imports were removed. Most pathlib changes involve straightforward changes from `os` functions such as `os.mkdir` or `os.path.abspath` to their equivalent methods in `pathlib.Path`. Changing ad-hoc temporary path to `tmp_path` involved removing the `tmp_dir_path` fixture and replacing its functionality with `tmp_path` in `test_save_load` and `test_guess_filename_with_tempfile`. On `test_static_route_user_home` function: * I think that the intention of this test is to ensure that aiohttp correctly expands the home path if passed in a string. I refactored it to `pathlib.Path` and cut out duplication of `relative_to()` calls. But if it's not doing anything but expanding `~`, then it's testing the functionality of `pathlib.Path`, not aiohttp. On `unix_sockname` fixture: This fixture uses `tempfile.TemporaryDirectory`. Because it's a somewhat complicated fixture used across multiple test modules, I left it as-is for now. On `str(tmp_path)` and even `pathlib.Path(str(tmp_path))`: pytest uses `pathlib2` to provide `tmp_path` for Python 3.5 (only). This is mostly fine but it fails on a couple of corner cases, such as `os.symlink()` which blocks all but `str` and `PurePath` via isinstance type checking. In several cases, this requires conversion to string or conversion to string and then into `pathlib.Path` to maintain code compatibility. See: pytest-dev/pytest/issues/5017 ## Are there changes in behavior for the user? These changes only affect the test suite and have no impact on the end user. ## Related issue number This is intended to address discussion following the simplistic changes from tmpdir to tmp_path of #3955. ## Checklist - [X] I think the code is well written - [X] Unit tests for the changes exist - [X] Documentation reflects the changes - [X] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [X] Add a new news fragment into the `CHANGES` folder * name it `<issue_id>.<type>` for example (588.bugfix) * if you don't have an `issue_id` change it to the pr id after creating the pr * ensure type is one of the following: * `.feature`: Signifying a new feature. * `.bugfix`: Signifying a bug fix. * `.doc`: Signifying a documentation improvement. * `.removal`: Signifying a deprecation or removal of public API. * `.misc`: A ticket has been closed, but it is not of interest to users. * Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files." Co-authored-by: Matt VanEseltine <[email protected]>
…s, temp files (#8084) **This is a backport of PR #3957 as merged into master (79fe204).** * Improve test suite handling of paths, temp files This updates most uses of `os.path` to instead use `pathlib.Path`. Relatedly, and following up from #3955 (which replaced pytest's `tmpdir` fixture with `tmp_path`), this removes most ad-hoc tempfile creation in favor of the `tmp_path` fixture. Following conversion, unnecessary `os` and `tempfile` imports were removed. Most pathlib changes involve straightforward changes from `os` functions such as `os.mkdir` or `os.path.abspath` to their equivalent methods in `pathlib.Path`. Changing ad-hoc temporary path to `tmp_path` involved removing the `tmp_dir_path` fixture and replacing its functionality with `tmp_path` in `test_save_load` and `test_guess_filename_with_tempfile`. On `test_static_route_user_home` function: * I think that the intention of this test is to ensure that aiohttp correctly expands the home path if passed in a string. I refactored it to `pathlib.Path` and cut out duplication of `relative_to()` calls. But if it's not doing anything but expanding `~`, then it's testing the functionality of `pathlib.Path`, not aiohttp. On `unix_sockname` fixture: This fixture uses `tempfile.TemporaryDirectory`. Because it's a somewhat complicated fixture used across multiple test modules, I left it as-is for now. On `str(tmp_path)` and even `pathlib.Path(str(tmp_path))`: pytest uses `pathlib2` to provide `tmp_path` for Python 3.5 (only). This is mostly fine but it fails on a couple of corner cases, such as `os.symlink()` which blocks all but `str` and `PurePath` via isinstance type checking. In several cases, this requires conversion to string or conversion to string and then into `pathlib.Path` to maintain code compatibility. See: pytest-dev/pytest/issues/5017 * Correct test_guess_filename to use file object * Update symlink in tests; more guess_filename tests (cherry picked from commit 79fe204) <!-- Thank you for your contribution! --> ## What do these changes do? This updates most uses of `os.path` to instead use `pathlib.Path`. Relatedly, and following up from #3955 (which replaced pytest's `tmpdir` fixture with `tmp_path`), this removes most ad-hoc tempfile creation in favor of the `tmp_path` fixture. Following conversion, unnecessary `os` and `tempfile` imports were removed. Most pathlib changes involve straightforward changes from `os` functions such as `os.mkdir` or `os.path.abspath` to their equivalent methods in `pathlib.Path`. Changing ad-hoc temporary path to `tmp_path` involved removing the `tmp_dir_path` fixture and replacing its functionality with `tmp_path` in `test_save_load` and `test_guess_filename_with_tempfile`. On `test_static_route_user_home` function: * I think that the intention of this test is to ensure that aiohttp correctly expands the home path if passed in a string. I refactored it to `pathlib.Path` and cut out duplication of `relative_to()` calls. But if it's not doing anything but expanding `~`, then it's testing the functionality of `pathlib.Path`, not aiohttp. On `unix_sockname` fixture: This fixture uses `tempfile.TemporaryDirectory`. Because it's a somewhat complicated fixture used across multiple test modules, I left it as-is for now. On `str(tmp_path)` and even `pathlib.Path(str(tmp_path))`: pytest uses `pathlib2` to provide `tmp_path` for Python 3.5 (only). This is mostly fine but it fails on a couple of corner cases, such as `os.symlink()` which blocks all but `str` and `PurePath` via isinstance type checking. In several cases, this requires conversion to string or conversion to string and then into `pathlib.Path` to maintain code compatibility. See: pytest-dev/pytest/issues/5017 ## Are there changes in behavior for the user? These changes only affect the test suite and have no impact on the end user. ## Related issue number This is intended to address discussion following the simplistic changes from tmpdir to tmp_path of #3955. ## Checklist - [X] I think the code is well written - [X] Unit tests for the changes exist - [X] Documentation reflects the changes - [X] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [X] Add a new news fragment into the `CHANGES` folder * name it `<issue_id>.<type>` for example (588.bugfix) * if you don't have an `issue_id` change it to the pr id after creating the pr * ensure type is one of the following: * `.feature`: Signifying a new feature. * `.bugfix`: Signifying a bug fix. * `.doc`: Signifying a documentation improvement. * `.removal`: Signifying a deprecation or removal of public API. * `.misc`: A ticket has been closed, but it is not of interest to users. * Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files." --------- Co-authored-by: Matt VanEseltine <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
tmp_path is the replacement fixture in pytest for tmpdir; tmp_path
uses the builtin pathlib.Path class. As it says on the tin, this
commit replaces every instance of tmpdir in the test suite with
tmp_path. Aside from s/tmpdir/tmp_path/ this also required changing
instances of
tmpdir.join(foo)
totmp_path / foo
.This is intended to comprehensively address and close #3551, and
should have no side effects. This does not affect end users.