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

(cherry picked from commit 79fe204)

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

@@ -1,7 +1,7 @@
 import asyncio
 import hashlib
 import io
-import os.path
+import pathlib
 import urllib.parse
 import zlib
 from http.cookies import BaseCookie, Morsel, SimpleCookie
@@ -921,12 +921,11 @@ 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()
 
 
@@ -947,19 +946,17 @@ 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)
+        assert req.headers["CONTENT-LENGTH"] == 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, chunked=True, loop=loop
         )

tests/test_cookiejar.py

@@ -1,9 +1,8 @@
 import asyncio
 import datetime
 import itertools
-import os
+import pathlib
 import pickle
-import tempfile
 import unittest
 from http.cookies import BaseCookie, Morsel, SimpleCookie
 from unittest import mock
@@ -178,8 +177,10 @@ async def test_constructor_with_expired(
     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(loop=loop)
@@ -193,7 +194,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

@@ -4,7 +4,6 @@
 import gc
 import platform
 import sys
-import tempfile
 import weakref
 from math import ceil, modf
 from pathlib import Path
@@ -73,9 +72,19 @@ 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_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 -----------------------------------

tests/test_multipart.py

@@ -1,6 +1,7 @@
 import asyncio
 import io
 import json
+import pathlib
 import zlib
 from unittest import mock
 
@@ -1270,7 +1271,7 @@ async def test_write_preserves_content_disposition(self, buf, stream) -> None:
 
     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_preserve_content_disposition_header(self, buf, stream):
 
     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,
@@ -1324,7 +1325,7 @@ async def test_set_content_disposition_override(self, buf, stream):
 
     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

@@ -731,7 +731,7 @@ async def test_proxy_from_env_http_with_auth_from_netrc(
         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)}
@@ -757,7 +757,7 @@ async def test_proxy_from_env_http_without_auth_from_netrc(
         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)}
@@ -780,7 +780,7 @@ async def test_proxy_from_env_http_without_auth_from_wrong_netrc(
     auth = aiohttp.BasicAuth("user", "pass")
     netrc_file = tmp_path / "test_netrc"
     invalid_data = f"machine 127.0.0.1 {auth.login} pass {auth.password}"
-    with open(str(netrc_file), "w") as f:
+    with netrc_file.open("w") as f:
         f.write(invalid_data)
 
     mocker.patch.dict(