Skip to content
This repository has been archived by the owner on May 22, 2021. It is now read-only.

Commit

Permalink
Improve test suite handling of paths, temp files
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vaneseltine committed Aug 2, 2019
1 parent e76bd8a commit fdff4e7
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 147 deletions.
1 change: 1 addition & 0 deletions CHANGES/3955.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve test suite handling of paths and temp files to consistently use pathlib and pytest fixtures.
21 changes: 9 additions & 12 deletions tests/test_client_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()


Expand All @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions tests/test_cookiejar.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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


Expand Down
7 changes: 3 additions & 4 deletions tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import gc
import os
import platform
import tempfile
from unittest import mock

import pytest
Expand Down Expand Up @@ -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 -----------------------------------
Expand Down
7 changes: 4 additions & 3 deletions tests/test_multipart.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import asyncio
import io
import json
import pathlib
import zlib
from unittest import mock

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions tests/test_proxy_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)})
Expand All @@ -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)})
Expand All @@ -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),
Expand Down

0 comments on commit fdff4e7

Please sign in to comment.