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`.

Note on `test_static_route_user_home`:

* 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.

Note 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.

CHANGES/3955.misc

@@ -0,0 +1 @@
+Improve test suite handling of paths and temp files to consistently use pathlib and pytest fixtures.

tests/test_client_request.py

@@ -3,7 +3,7 @@
 import asyncio
 import hashlib
 import io
-import os.path
+import pathlib
 import urllib.parse
 import zlib
 from http.cookies import SimpleCookie
@@ -789,15 +789,14 @@ async def test_chunked_transfer_encoding(loop, conn) -> None:
 
 
 async def test_file_upload_not_chunked(loop) -> None:
-    here = os.path.dirname(__file__)
-    fname = os.path.join(here, 'aiohttp.png')
-    with open(fname, 'rb') as f:
+    file_path = pathlib.Path(__file__).parent / 'aiohttp.png'
+    with file_path.open('rb') as f:
         req = ClientRequest(
             'post', URL('http://python.org/'),
             data=f,
             loop=loop)
         assert not req.chunked
-        assert req.headers['CONTENT-LENGTH'] == str(os.path.getsize(fname))
+        assert req.headers['CONTENT-LENGTH'] == str(file_path.stat().st_size)
         await req.close()
 
 
@@ -816,23 +815,21 @@ async def test_precompressed_data_stays_intact(loop) -> None:
 
 
 async def test_file_upload_not_chunked_seek(loop) -> None:
-    here = os.path.dirname(__file__)
-    fname = os.path.join(here, 'aiohttp.png')
-    with open(fname, 'rb') as f:
+    file_path = pathlib.Path(__file__).parent / 'aiohttp.png'
+    with file_path.open('rb') as f:
         f.seek(100)
         req = ClientRequest(
             'post', URL('http://python.org/'),
             data=f,
             loop=loop)
         assert req.headers['CONTENT-LENGTH'] == \
-            str(os.path.getsize(fname) - 100)
+            str(file_path.stat().st_size - 100)
         await req.close()
 
 
 async def test_file_upload_force_chunked(loop) -> None:
-    here = os.path.dirname(__file__)
-    fname = os.path.join(here, 'aiohttp.png')
-    with open(fname, 'rb') as f:
+    file_path = pathlib.Path(__file__).parent / 'aiohttp.png'
+    with file_path.open('rb') as f:
         req = ClientRequest(
             'post', URL('http://python.org/'),
             data=f,

tests/test_cookiejar.py

@@ -1,8 +1,6 @@
 import asyncio
 import datetime
 import itertools
-import os
-import tempfile
 import unittest
 from http.cookies import SimpleCookie
 from unittest import mock
@@ -144,8 +142,8 @@ async def test_constructor(loop, cookies_to_send, cookies_to_receive) -> None:
     assert jar._loop is loop
 
 
-async def test_save_load(loop, cookies_to_send, cookies_to_receive) -> None:
-    file_path = tempfile.mkdtemp() + '/aiohttp.test.cookie'
+async def test_save_load(tmp_path, loop, cookies_to_send, cookies_to_receive) -> None:
+    file_path = tmp_path / 'aiohttp.test.cookie'
 
     # export cookie jar
     jar_save = CookieJar()
@@ -159,7 +157,6 @@ async def test_save_load(loop, cookies_to_send, cookies_to_receive) -> None:
     for cookie in jar_load:
         jar_test[cookie.key] = cookie
 
-    os.unlink(file_path)
     assert jar_test == cookies_to_receive
 
 

tests/test_helpers.py

@@ -3,7 +3,6 @@
 import gc
 import os
 import platform
-import tempfile
 from unittest import mock
 
 import pytest
@@ -46,9 +45,9 @@ def test_parse_mimetype(mimetype, expected) -> None:
 
 # ------------------- guess_filename ----------------------------------
 
-def test_guess_filename_with_tempfile() -> None:
-    with tempfile.TemporaryFile() as fp:
-        assert (helpers.guess_filename(fp, 'no-throw') is not None)
+def test_guess_filename_with_tempfile(tmp_path) -> None:
+    file_path = tmp_path / 'test_guess_filename'
+    assert (helpers.guess_filename(file_path, 'no-throw') is not None)
 
 
 # ------------------- BasicAuth -----------------------------------

tests/test_multipart.py

@@ -1,6 +1,7 @@
 import asyncio
 import io
 import json
+import pathlib
 import zlib
 from unittest import mock
 
@@ -1266,7 +1267,7 @@ async def test_preserve_content_disposition_header(self, buf, stream):
         """
         https://github.com/aio-libs/aiohttp/pull/3475#issuecomment-451072381
         """
-        with open(__file__, 'rb') as fobj:
+        with pathlib.Path(__file__).open('rb') as fobj:
             with aiohttp.MultipartWriter('form-data', boundary=':') as writer:
                 part = writer.append(
                     fobj,
@@ -1297,7 +1298,7 @@ async def test_set_content_disposition_override(self, buf, stream):
         """
         https://github.com/aio-libs/aiohttp/pull/3475#issuecomment-451072381
         """
-        with open(__file__, 'rb') as fobj:
+        with pathlib.Path(__file__).open('rb') as fobj:
             with aiohttp.MultipartWriter('form-data', boundary=':') as writer:
                 part = writer.append(
                     fobj,
@@ -1328,7 +1329,7 @@ async def test_reset_content_disposition_header(self, buf, stream):
         """
         https://github.com/aio-libs/aiohttp/pull/3475#issuecomment-451072381
         """
-        with open(__file__, 'rb') as fobj:
+        with pathlib.Path(__file__).open('rb') as fobj:
             with aiohttp.MultipartWriter('form-data', boundary=':') as writer:
                 part = writer.append(
                     fobj,

tests/test_proxy_functional.py

@@ -537,7 +537,7 @@ async def test_proxy_from_env_http_with_auth_from_netrc(
     netrc_file = tmp_path / 'test_netrc'
     netrc_file_data = 'machine 127.0.0.1 login %s password %s' % (
         auth.login, auth.password)
-    with open(str(netrc_file), 'w') as f:
+    with netrc_file.open('w') as f:
         f.write(netrc_file_data)
     mocker.patch.dict(os.environ, {'http_proxy': str(proxy.url),
                                    'NETRC': str(netrc_file)})
@@ -559,7 +559,7 @@ async def test_proxy_from_env_http_without_auth_from_netrc(
     netrc_file = tmp_path / 'test_netrc'
     netrc_file_data = 'machine 127.0.0.2 login %s password %s' % (
         auth.login, auth.password)
-    with open(str(netrc_file), 'w') as f:
+    with netrc_file.open('w') as f:
         f.write(netrc_file_data)
     mocker.patch.dict(os.environ, {'http_proxy': str(proxy.url),
                                    'NETRC': str(netrc_file)})
@@ -581,7 +581,7 @@ async def test_proxy_from_env_http_without_auth_from_wrong_netrc(
     netrc_file = tmp_path / 'test_netrc'
     invalid_data = 'machine 127.0.0.1 %s pass %s' % (
         auth.login, auth.password)
-    with open(str(netrc_file), 'w') as f:
+    with netrc_file.open('w') as f:
         f.write(invalid_data)
 
     mocker.patch.dict(os.environ, {'http_proxy': str(proxy.url),

tests/test_urldispatch.py

@@ -51,7 +51,7 @@ def go():
         route2 = router.add_route('GET', '/variable/{name}',
                                   make_handler())
         resource = router.add_static('/static',
-                                     os.path.dirname(aiohttp.__file__))
+                                     pathlib.Path(aiohttp.__file__).parent)
         return [route1, route2] + list(resource)
     return go
 
@@ -352,7 +352,7 @@ def test_route_dynamic(router) -> None:
 
 def test_add_static(router) -> None:
     resource = router.add_static('/st',
-                                 os.path.dirname(aiohttp.__file__),
+                                 pathlib.Path(aiohttp.__file__).parent,
                                  name='static')
     assert router['static'] is resource
     url = resource.url_for(filename='/dir/a.txt')
@@ -362,7 +362,7 @@ def test_add_static(router) -> None:
 
 def test_add_static_append_version(router) -> None:
     resource = router.add_static('/st',
-                                 os.path.dirname(__file__),
+                                 pathlib.Path(__file__).parent,
                                  name='static')
     url = resource.url_for(filename='/data.unknown_mime_type',
                            append_version=True)
@@ -373,7 +373,7 @@ def test_add_static_append_version(router) -> None:
 
 def test_add_static_append_version_set_from_constructor(router) -> None:
     resource = router.add_static('/st',
-                                 os.path.dirname(__file__),
+                                 pathlib.Path(__file__).parent,
                                  append_version=True,
                                  name='static')
     url = resource.url_for(filename='/data.unknown_mime_type')
@@ -384,7 +384,7 @@ def test_add_static_append_version_set_from_constructor(router) -> None:
 
 def test_add_static_append_version_override_constructor(router) -> None:
     resource = router.add_static('/st',
-                                 os.path.dirname(__file__),
+                                 pathlib.Path(__file__).parent,
                                  append_version=True,
                                  name='static')
     url = resource.url_for(filename='/data.unknown_mime_type',
@@ -395,7 +395,7 @@ def test_add_static_append_version_override_constructor(router) -> None:
 
 def test_add_static_append_version_filename_without_slash(router) -> None:
     resource = router.add_static('/st',
-                                 os.path.dirname(__file__),
+                                 pathlib.Path(__file__).parent,
                                  name='static')
     url = resource.url_for(filename='data.unknown_mime_type',
                            append_version=True)
@@ -406,7 +406,7 @@ def test_add_static_append_version_filename_without_slash(router) -> None:
 
 def test_add_static_append_version_non_exists_file(router) -> None:
     resource = router.add_static('/st',
-                                 os.path.dirname(__file__),
+                                 pathlib.Path(__file__).parent,
                                  name='static')
     url = resource.url_for(filename='/non_exists_file', append_version=True)
     assert '/st/non_exists_file' == str(url)
@@ -415,23 +415,22 @@ def test_add_static_append_version_non_exists_file(router) -> None:
 def test_add_static_append_version_non_exists_file_without_slash(
         router) -> None:
     resource = router.add_static('/st',
-                                 os.path.dirname(__file__),
+                                 pathlib.Path(__file__).parent,
                                  name='static')
     url = resource.url_for(filename='non_exists_file', append_version=True)
     assert '/st/non_exists_file' == str(url)
 
 
-def test_add_static_append_version_follow_symlink(router, tmpdir) -> None:
+def test_add_static_append_version_follow_symlink(router, tmp_path) -> None:
     """
     Tests the access to a symlink, in static folder with apeend_version
     """
-    tmp_dir_path = str(tmpdir)
-    symlink_path = os.path.join(tmp_dir_path, 'append_version_symlink')
-    symlink_target_path = os.path.dirname(__file__)
+    symlink_path = tmp_path / 'append_version_symlink'
+    symlink_target_path = pathlib.Path(__file__).parent
     os.symlink(symlink_target_path, symlink_path, True)
 
     # Register global static route:
-    resource = router.add_static('/st', tmp_dir_path, follow_symlinks=True,
+    resource = router.add_static('/st', tmp_path, follow_symlinks=True,
                                  append_version=True)
 
     url = resource.url_for(
@@ -442,17 +441,17 @@ def test_add_static_append_version_follow_symlink(router, tmpdir) -> None:
     assert expect_url == str(url)
 
 
-def test_add_static_append_version_not_follow_symlink(router, tmpdir) -> None:
+def test_add_static_append_version_not_follow_symlink(router, tmp_path) -> None:
     """
     Tests the access to a symlink, in static folder with apeend_version
     """
-    tmp_dir_path = str(tmpdir)
-    symlink_path = os.path.join(tmp_dir_path, 'append_version_symlink')
-    symlink_target_path = os.path.dirname(__file__)
+
+    symlink_path = tmp_path / 'append_version_symlink'
+    symlink_target_path = pathlib.Path(__file__).parent
     os.symlink(symlink_target_path, symlink_path, True)
 
     # Register global static route:
-    resource = router.add_static('/st', tmp_dir_path, follow_symlinks=False,
+    resource = router.add_static('/st', tmp_path, follow_symlinks=False,
                                  append_version=True)
 
     filename = '/append_version_symlink/data.unknown_mime_type'
@@ -475,7 +474,7 @@ def test_dynamic_not_match(router) -> None:
 
 
 async def test_static_not_match(router) -> None:
-    router.add_static('/pre', os.path.dirname(aiohttp.__file__),
+    router.add_static('/pre', pathlib.Path(aiohttp.__file__).parent,
                       name='name')
     resource = router['name']
     ret = await resource.resolve(
@@ -513,20 +512,20 @@ def test_contains(router) -> None:
 
 
 def test_static_repr(router) -> None:
-    router.add_static('/get', os.path.dirname(aiohttp.__file__),
+    router.add_static('/get', pathlib.Path(aiohttp.__file__).parent,
                       name='name')
     assert re.match(r"<StaticResource 'name' /get", repr(router['name']))
 
 
 def test_static_adds_slash(router) -> None:
     route = router.add_static('/prefix',
-                              os.path.dirname(aiohttp.__file__))
+                              pathlib.Path(aiohttp.__file__).parent)
     assert '/prefix' == route._prefix
 
 
 def test_static_remove_trailing_slash(router) -> None:
     route = router.add_static('/prefix/',
-                              os.path.dirname(aiohttp.__file__))
+                              pathlib.Path(aiohttp.__file__).parent)
     assert '/prefix' == route._prefix
 
 
@@ -797,7 +796,7 @@ def test_named_resources(router) -> None:
     route2 = router.add_route('GET', '/variable/{name}',
                               make_handler(), name='route2')
     route3 = router.add_static('/static',
-                               os.path.dirname(aiohttp.__file__),
+                               pathlib.Path(aiohttp.__file__).parent,
                                name='route3')
     names = {route1.name, route2.name, route3.name}
 
@@ -945,11 +944,11 @@ def test_resources_abc(router) -> None:
 
 def test_static_route_user_home(router) -> None:
     here = pathlib.Path(aiohttp.__file__).parent
-    home = pathlib.Path(os.path.expanduser('~'))
-    if not str(here).startswith(str(home)):  # pragma: no cover
+    try:
+        static_dir = pathlib.Path('~') / here.relative_to(pathlib.Path.home())
+    except ValueError:
         pytest.skip("aiohttp folder is not placed in user's HOME")
-    static_dir = '~/' + str(here.relative_to(home))
-    route = router.add_static('/st', static_dir)
+    route = router.add_static('/st', str(static_dir))
     assert here == route.get_info()['directory']
 
 
@@ -961,15 +960,15 @@ def test_static_route_points_to_file(router) -> None:
 
 async def test_404_for_static_resource(router) -> None:
     resource = router.add_static('/st',
-                                 os.path.dirname(aiohttp.__file__))
+                                 pathlib.Path(aiohttp.__file__).parent)
     ret = await resource.resolve(
         make_mocked_request('GET', '/unknown/path'))
     assert (None, set()) == ret
 
 
 async def test_405_for_resource_adapter(router) -> None:
     resource = router.add_static('/st',
-                                 os.path.dirname(aiohttp.__file__))
+                                 pathlib.Path(aiohttp.__file__).parent)
     ret = await resource.resolve(
         make_mocked_request('POST', '/st/abc.py'))
     assert (None, {'HEAD', 'GET'}) == ret
@@ -986,13 +985,13 @@ async def test_check_allowed_method_for_found_resource(router) -> None:
 
 def test_url_for_in_static_resource(router) -> None:
     resource = router.add_static('/static',
-                                 os.path.dirname(aiohttp.__file__))
+                                 pathlib.Path(aiohttp.__file__).parent)
     assert URL('/static/file.txt') == resource.url_for(filename='file.txt')
 
 
 def test_url_for_in_static_resource_pathlib(router) -> None:
     resource = router.add_static('/static',
-                                 os.path.dirname(aiohttp.__file__))
+                                 pathlib.Path(aiohttp.__file__).parent)
     assert URL('/static/file.txt') == resource.url_for(
         filename=pathlib.Path('file.txt'))
 
@@ -1162,7 +1161,7 @@ def test_frozen_app_on_subapp(app) -> None:
 
 def test_set_options_route(router) -> None:
     resource = router.add_static('/static',
-                                 os.path.dirname(aiohttp.__file__))
+                                 pathlib.Path(aiohttp.__file__).parent)
     options = None
     for route in resource:
         if route.method == 'OPTIONS':
@@ -1224,7 +1223,7 @@ def test_dynamic_resource_canonical() -> None:
 
 def test_static_resource_canonical() -> None:
     prefix = '/prefix'
-    directory = str(os.path.dirname(aiohttp.__file__))
+    directory = str(pathlib.Path(aiohttp.__file__).parent)
     canonical = prefix
     res = StaticResource(prefix=prefix, directory=directory)
     assert res.canonical == canonical