fix: TarFile import #5666
fix: TarFile import #5666
Conversation
Now the test fails
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
Wrap the TarFile and TarInfo classes to adhere to the interface
There was a problem hiding this comment.
Pull Request Overview
This PR aims to fix a failing test for handling tar files by introducing a custom TarWrapper and TarInfo in the importer as well as adding a new test case for tar file imports.
- Introduce a custom TarInfo and TarWrapper in beets/importer.py to better handle tar files
- Add a new test_import_tar in test/test_importer.py to validate tar file import functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| beets/importer.py | Replace existing tarfile.open handler with a custom wrapper class |
| test/test_importer.py | Update test cases and class inheritance to include a tar file import |
Comments suppressed due to low confidence (1)
test/test_importer.py:292
- [nitpick] The variable name 'zip_path' can be misleading in a test intended for tar files; consider renaming it to 'tar_path'.
zip_path = self.create_archive()
| self.gid = member.gid | ||
| self.uname = member.uname | ||
| self.gname = member.gname | ||
| self.date_time = time.gmtime(member.mtime)[:6] |
There was a problem hiding this comment.
The use of 'time.gmtime' requires an import for the time module. Please add 'import time' at the beginning of the file.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
There was a problem hiding this comment.
Seems to be passing without it. 🤷
| @@ -1225,7 +1225,33 @@ def handlers(cls): | |||
| cls._handlers.append((is_zipfile, ZipFile)) | |||
| import tarfile | |||
|
|
|||
| cls._handlers.append((tarfile.is_tarfile, tarfile.open)) | |||
| class TarInfo(tarfile.TarInfo): | |||
There was a problem hiding this comment.
I am not a python dev. Not sure this is correct, but is working on my install.
document bugfix
bcotton
changed the title
Description
Failing test for #5664. Proposed solution to failing test
To Do
docs/to describe it.)docs/changelog.rstto the bottom of one of the lists near the top of the document.)