Skip to content
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

Merged
merged 1 commit into from
Aug 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/3551.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Replace all tmpdir fixtures with tmp_path in test suite.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ Martin Melka
Martin Richard
Mathias Fröjdman
Mathieu Dugré
Matt VanEseltine
Matthieu Hauglustaine
Matthieu Rigal
Michael Ihnatenko
Expand Down
4 changes: 2 additions & 2 deletions tests/test_client_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -655,8 +655,8 @@ async def test_pass_falsy_data(loop) -> None:
await req.close()


async def test_pass_falsy_data_file(loop, tmpdir) -> None:
testfile = tmpdir.join('tmpfile').open('w+b')
async def test_pass_falsy_data_file(loop, tmp_path) -> None:
testfile = (tmp_path / 'tmpfile').open('w+b')
testfile.write(b'data')
testfile.seek(0)
skip = frozenset([hdrs.CONTENT_TYPE])
Expand Down
12 changes: 6 additions & 6 deletions tests/test_proxy_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,11 +530,11 @@ async def test_proxy_from_env_http_with_auth(proxy_test_server,


async def test_proxy_from_env_http_with_auth_from_netrc(
proxy_test_server, get_request, tmpdir, mocker):
proxy_test_server, get_request, tmp_path, mocker):
url = 'http://aiohttp.io/path'
proxy = await proxy_test_server()
auth = aiohttp.BasicAuth('user', 'pass')
netrc_file = tmpdir.join('test_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:
Expand All @@ -552,11 +552,11 @@ async def test_proxy_from_env_http_with_auth_from_netrc(


async def test_proxy_from_env_http_without_auth_from_netrc(
proxy_test_server, get_request, tmpdir, mocker):
proxy_test_server, get_request, tmp_path, mocker):
url = 'http://aiohttp.io/path'
proxy = await proxy_test_server()
auth = aiohttp.BasicAuth('user', 'pass')
netrc_file = tmpdir.join('test_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:
Expand All @@ -574,11 +574,11 @@ async def test_proxy_from_env_http_without_auth_from_netrc(


async def test_proxy_from_env_http_without_auth_from_wrong_netrc(
proxy_test_server, get_request, tmpdir, mocker):
proxy_test_server, get_request, tmp_path, mocker):
url = 'http://aiohttp.io/path'
proxy = await proxy_test_server()
auth = aiohttp.BasicAuth('user', 'pass')
netrc_file = tmpdir.join('test_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:
Expand Down
8 changes: 4 additions & 4 deletions tests/test_run_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,10 @@ def test_run_app_custom_backlog_unix(patched_loop) -> None:


@skip_if_no_unix_socks
def test_run_app_http_unix_socket(patched_loop, tmpdir) -> None:
def test_run_app_http_unix_socket(patched_loop, tmp_path) -> None:
app = web.Application()

sock_path = str(tmpdir / 'socket.sock')
sock_path = str(tmp_path / 'socket.sock')
printer = mock.Mock(wraps=stopper(patched_loop))
web.run_app(app, path=sock_path, print=printer)

Expand All @@ -366,10 +366,10 @@ def test_run_app_http_unix_socket(patched_loop, tmpdir) -> None:


@skip_if_no_unix_socks
def test_run_app_https_unix_socket(patched_loop, tmpdir) -> None:
def test_run_app_https_unix_socket(patched_loop, tmp_path) -> None:
app = web.Application()

sock_path = str(tmpdir / 'socket.sock')
sock_path = str(tmp_path / 'socket.sock')
ssl_context = ssl.create_default_context()
printer = mock.Mock(wraps=stopper(patched_loop))
web.run_app(app, path=sock_path, ssl_context=ssl_context, print=printer)
Expand Down
4 changes: 2 additions & 2 deletions tests/test_web_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -1560,7 +1560,7 @@ async def handler(request):
{'name': 'file', 'filename': 'file', 'filename*': 'file'})


async def test_response_with_bodypart_named(aiohttp_client, tmpdir) -> None:
async def test_response_with_bodypart_named(aiohttp_client, tmp_path) -> None:

async def handler(request):
reader = await request.multipart()
Expand All @@ -1571,7 +1571,7 @@ async def handler(request):
app.router.add_post('/', handler)
client = await aiohttp_client(app)

f = tmpdir.join('foobar.txt')
f = tmp_path / 'foobar.txt'
f.write_text('test', encoding='utf8')
data = {'file': open(str(f), 'rb')}
resp = await client.post('/', data=data)
Expand Down
22 changes: 11 additions & 11 deletions tests/test_web_sendfile_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,18 +319,18 @@ def test_static_route_path_existence_check() -> None:
web.StaticResource("/", nodirectory)


async def test_static_file_huge(aiohttp_client, tmpdir) -> None:
async def test_static_file_huge(aiohttp_client, tmp_path) -> None:
filename = 'huge_data.unknown_mime_type'

# fill 20MB file
with tmpdir.join(filename).open('w') as f:
with (tmp_path / filename).open('w') as f:
for i in range(1024*20):
f.write(chr(i % 64 + 0x20) * 1024)

file_st = os.stat(str(tmpdir.join(filename)))
file_st = os.stat(str((tmp_path / filename)))

app = web.Application()
app.router.add_static('/static', str(tmpdir))
app.router.add_static('/static', str(tmp_path))
client = await aiohttp_client(app)

resp = await client.get('/static/'+filename)
Expand All @@ -340,7 +340,7 @@ async def test_static_file_huge(aiohttp_client, tmpdir) -> None:
assert resp.headers.get('CONTENT-ENCODING') is None
assert int(resp.headers.get('CONTENT-LENGTH')) == file_st.st_size

f = tmpdir.join(filename).open('rb')
f = (tmp_path / filename).open('rb')
off = 0
cnt = 0
while off < file_st.st_size:
Expand Down Expand Up @@ -751,11 +751,11 @@ async def handler(request):
await resp.release()


async def test_static_file_huge_cancel(aiohttp_client, tmpdir) -> None:
async def test_static_file_huge_cancel(aiohttp_client, tmp_path) -> None:
filename = 'huge_data.unknown_mime_type'

# fill 100MB file
with tmpdir.join(filename).open('w') as f:
with (tmp_path / filename).open('w') as f:
for i in range(1024*20):
f.write(chr(i % 64 + 0x20) * 1024)

Expand All @@ -768,7 +768,7 @@ async def handler(request):
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))))
return ret

app = web.Application()
Expand All @@ -789,11 +789,11 @@ async def handler(request):
assert len(data) < 1024 * 1024 * 20


async def test_static_file_huge_error(aiohttp_client, tmpdir) -> None:
async def test_static_file_huge_error(aiohttp_client, tmp_path) -> None:
filename = 'huge_data.unknown_mime_type'

# fill 20MB file
with tmpdir.join(filename).open('wb') as f:
with (tmp_path / filename).open('wb') as f:
f.seek(20*1024*1024)
f.write(b'1')

Expand All @@ -802,7 +802,7 @@ async def handler(request):
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))))
Copy link
Contributor

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.

Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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

return ret

app = web.Application()
Expand Down
4 changes: 2 additions & 2 deletions tests/test_web_urldispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,12 +438,12 @@ async def post(self):
await r.release()


async def test_static_absolute_url(aiohttp_client, tmpdir) -> None:
async def test_static_absolute_url(aiohttp_client, tmp_path) -> None:
# requested url is an absolute name like
# /static/\\machine_name\c$ or /static/D:\path
# where the static dir is totally different
app = web.Application()
fname = tmpdir / 'file.txt'
fname = tmp_path / 'file.txt'
fname.write_text('sample text', 'ascii')
here = pathlib.Path(__file__).parent
app.router.add_static('/static', here)
Expand Down