Skip to content

Commit

Permalink
Issue pypa#7280 - Clean-up python2.7, isort, and flake8 errors
Browse files Browse the repository at this point in the history
  • Loading branch information
Dan Davis committed Jan 2, 2020
1 parent b6c0991 commit eb73752
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 28 deletions.
38 changes: 21 additions & 17 deletions tests/lib/filesystem.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Helpers for filesystem-dependent tests.
"""
import os
import multiprocessing
import os
import socket
import subprocess
import sys
Expand Down Expand Up @@ -47,7 +47,8 @@ def lock_action(f):
def external_file_opener(conn):
"""
This external process is run with multiprocessing.
It waits for a path from the parent, opens it, and then wait for another message before closing it.
It waits for a path from the parent, opens it, and then wait for another
message before closing it.
:param conn: bi-directional pipe
:return: nothing
Expand All @@ -69,7 +70,7 @@ def external_file_opener(conn):
lock_action(f)
elif action == 'noread':
make_unreadable_file(path)
except OSError as e:
except OSError:
traceback.print_exc(None, sys.stderr)

# Indicate the file is opened
Expand All @@ -84,23 +85,26 @@ def external_file_opener(conn):

class FileOpener(object):
"""
Test class acts as a context manager which can open a file from another process, and hold it open
to assure that this does not interfere with pip's operations.
Test class acts as a context manager which can open a file from a
subprocess, and hold it open to assure that this does not interfere with
pip's operations.
If a path is passed to the FileOpener, it immediately sends a message to the other process to
open that path. An action of "lock" or "noread" can also be sent to the subprocess, resulting
in various additional monkey wrenches now and in the future.
If a path is passed to the FileOpener, it immediately sends a message to
the other process to open that path. An action of "lock" or "noread" can
also be sent to the subprocess, resulting in various additional monkey
wrenches now and in the future.
Opening the path and taking the action can be deferred however, so that the FileOpener may
function as a pytest fixture if so desired.
Opening the path and taking the action can be deferred however, so that
the FileOpener may function as a pytest fixture if so desired.
"""
def __init__(self,
path=None, # type: Optional(str)
action=None, # type: Optional(str)
):
def __init__(self, path=None, action=None):
self.path = None
self.conn, child_conn = multiprocessing.Pipe()
self.child = multiprocessing.Process(target=external_file_opener, daemon=True, args=(child_conn,))
self.child = multiprocessing.Process(
target=external_file_opener,
args=(child_conn,)
)
self.child.daemon = True
self.child.start()
if path:
self.send(path, action)
Expand All @@ -109,11 +113,11 @@ def send(self, path, action=None):
if self.path is not None:
raise AttributeError('path may only be set once')
self.path = str(path)
self.conn.send( (str(path), action) )
self.conn.send((str(path), action))
return self.conn.recv()

def cleanup(self):
# send a message to the child to exit; it will exit whether the path was sent or not
# send a message to the child to exit
if self.child:
self.conn.send(True)
self.child.join()
Expand Down
21 changes: 14 additions & 7 deletions tests/unit/test_utils_filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
import pytest

from pip._internal.utils.filesystem import copy2_fixed, is_socket
from tests.lib.filesystem import make_socket_file, make_unreadable_file, FileOpener
from tests.lib.filesystem import (
FileOpener,
make_socket_file,
make_unreadable_file,
)
from tests.lib.path import Path


Expand Down Expand Up @@ -69,36 +73,39 @@ def test_copy2_fixed_raises_appropriate_errors(create, error_type, tmpdir):


def test_file_opener_no_file(process):
# The FileOpener cleans up the subprocess even if the parent never sends the path
# FileOpener joins the subprocess even if the parent never sends the path
with FileOpener():
pass
assert len(process.children()) == 0


def test_file_opener_not_found(tmpdir, process):
# The FileOpener cleans up the subprocess even if the file cannot be opened
# The FileOpener cleans up the subprocess when the file cannot be opened
path = tmpdir.joinpath('foo.txt')
with FileOpener(path):
pass
assert len(process.children()) == 0


def test_file_opener_normal(tmpdir, process):
# The FileOpener cleans up the subprocess when the file exists; also tests that path may be deferred
# The FileOpener cleans up the subprocess when the file exists
path = tmpdir.joinpath('foo.txt')
with open(path, 'w') as f:
f.write('Hello\n')
with FileOpener() as opener:
opener.send(path)
with FileOpener(path):
pass
assert len(process.children()) == 0


@skip_unless_windows
def test_file_opener_produces_unlink_error(tmpdir, process):
# FileOpener forces an error on Windows when we attempt to remove a file
# The initial path may be deferred; which must be tested with an error
path = tmpdir.joinpath('foo.txt')
with open(path, 'w') as f:
f.write('Hello\n')
with FileOpener(path):
with FileOpener() as opener:
opener.send(path)
with pytest.raises(OSError):
os.unlink(path)

Expand Down
7 changes: 3 additions & 4 deletions tests/unit/test_utils_temp_dir.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
TempDirectory,
global_tempdir_manager,
)

from tests.lib.filesystem import FileOpener


Expand Down Expand Up @@ -223,7 +222,7 @@ def test_temp_dir_warns_if_cannot_clean(caplog):

# Capture only at WARNING level and up
with caplog.at_level(logging.WARNING, 'pip._internal.utils.temp_dir'):
# open a file within the temporary directory in a sub-process, so it cannot be cleaned up
# open a file within the temporary directory in a sub-process
with FileOpener() as opener:
subpath = os.path.join(temp_dir_path, 'foo.txt')
with open(subpath, 'w') as f:
Expand All @@ -232,11 +231,11 @@ def test_temp_dir_warns_if_cannot_clean(caplog):
# with the file open, attempt to remove the log directory
temp_dir.cleanup()

# assert that a WARNING was logged and that it contains a note about a potential virus scanner
# assert that a WARNING was logged about virus scanner
assert 'WARNING' in caplog.text
assert 'virus scanner' in caplog.text

# Assure that the cleanup was properly retried; this fix did not change retries
# Assure that the cleanup was properly retried
duration = time.time() - stime
assert duration >= 2.0

Expand Down

0 comments on commit eb73752

Please sign in to comment.