Improve test suite handling of paths, temp files (#3957)

* 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

Author: vaneseltine

CHANGES/3957.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,7 @@
 import asyncio
 import datetime
 import itertools
-import os
-import tempfile
+import pathlib
 import unittest
 from http.cookies import SimpleCookie
 from unittest import mock
@@ -144,8 +143,10 @@ 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 = pathlib.Path(str(tmp_path)) / 'aiohttp.test.cookie'
 
     # export cookie jar
     jar_save = CookieJar()
@@ -159,7 +160,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,11 +45,21 @@ def test_parse_mimetype(mimetype, expected) -> None:
 
 # ------------------- guess_filename ----------------------------------
 
-def test_guess_filename_with_tempfile() -> None:
-    with tempfile.TemporaryFile() as fp:
+def test_guess_filename_with_file_object(tmp_path) -> None:
+    file_path = tmp_path / 'test_guess_filename'
+    with file_path.open('w+b') as fp:
         assert (helpers.guess_filename(fp, 'no-throw') is not None)
 
 
+def test_guess_filename_with_path(tmp_path) -> None:
+    file_path = tmp_path / 'test_guess_filename'
+    assert (helpers.guess_filename(file_path, 'no-throw') is not None)
+
+
+def test_guess_filename_with_default() -> None:
+    assert (helpers.guess_filename(None, 'no-throw') == 'no-throw')
+
+
 # ------------------- BasicAuth -----------------------------------
 
 def test_basic_auth1() -> None:

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),