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

Fix context handling, stale test dirs, and resource leak #37

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jmgurney
Copy link

Sorry, this combines a few fixes all in one, mainly I was dealing w/ a number of them and did a little bit of clean up at the same time.

  1. it drops py2 compatibility.
  2. it splits _defer_close to not do double duty as closing the file handle, and makes the later a _doclose attribute.
  3. adds a test for context management to make sure things are closed and fix it so they work.
  4. Fixes SeekableArchive to not leak an open file, this silences a bunch of ResourceWarning: unclosed file when running the tests.
  5. Adds support for passing in pathlib.Path.
  6. Fixes tests so they clean up the temp directory after they run.
  7. Adds test to verify that context doesn't close a passed in file that was opened.

FILENAMES = [
'test1.txt',
'foo',
# TODO: test non-ASCII chars.
#'álért.txt',
]

class MakeTempMixIn:
def setUp(self):
self.TMPDIR = tempfile.mkdtemp(suffix='.python-libarchive')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use TemporaryDirectory and its member cleanup() instead to avoid importing shutil and manually deleting it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants