Consolidate ArchiveOperations tests, remove duplicated code
#431
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
christian-monch
force-pushed
the
enh-consolidate-archiveops-tests
branch
from
7758aa3
to
04368e7
Compare
|
This PR has been in draft mode for a long time. I am closing it now as part of a cleanup operation. Feel free to reopen, once work on it restarts. Sorry. |
This commit adds `ZipArchiveOperations`, a class that implements `ArchiveOperations` for zip files. It makes use of `datalad_next.iter_collections.zipfile.iter_zip` to implement iteration. The commit comes with tests. The test code includes a pytest-fixture called `sample_zip`, which creates a sample zip-file with known content, much like the `sample_tar_xz`-fixture.
This commit fixes a bug in the tests where Unix path-separators were always used to identify directory entries in a zip-archive. That is wrong on Windows, where Windows-path separators have to be used.
This commit ensures that all zip-file names that are used as parameters in methods of ZipArchiveOperations use '/' in names of zip-archive items. That is due to the fact that zip enforces the use '/' in the names of its items (zip-APPNOTE.TXT 6.3.3 from September 1st, 2012). Until PR datalad#409 is merged this will ensure that the proper file names are used to check for items in zip-archives. The commit can be reverted once PR datalad#409 is merged.
This commit removes duplicated code for the sample_zip-fixture. It makes the sample_zip- fixture avaiable through conftest.py and re-uses it on the ZipArchiveOperations tests.
This commit uses PurePosixPath to properly read the path of a zip-archive on Windows in test_open()
This commit renames the file `datalad_next/archive_operations/tests/test_zipfile_archive_operations.py` to `datalad_next/archive_operations/tests/test_zipfile.py`. This is better readable and in sync with the name of the tests for `datalad_next/archive_operations/tarfile.py`
This commit adapts the ZipArchiveOperations code to the current type of `ZipfileItem.name`, i.e. to `PurePosixPath`. The commit also adapts the tests for ZipArchiveOperations to the updated types, and aligns the tests to the test code for TarArchiveOperations. This is in preparation for a test code consolidation
This commit factors out the common logic for tests on archive operations and used them in the tests of tar-archive operations and of zip-archive operations. The code relies on the ZipFileDirPath to properly work. Therefore the commit that adds ZipFileDirPath will be cherry picked into this branch
This commit adds the class `ZipFileDirPath`. The class is a direct subclass of `PurePosixPath`. Instances of the class represent directory names in zip-archives. The difference to `PurePosixPath` is that rendering appends a '/' to the name. This allows us to use the generic way of testing for containment in ZipArchiveOperations, i.e. if item.name in archive: Otherwise we would have to discriminate between directory- and file-names and manually add a '/' to directory names, e.g. something like this: if (item.name + '/' if item.type == FileSystemItemType.directory else item.name) in archive:
|
Reopening and adapting it to the current status of main |
christian-monch
force-pushed
the
enh-consolidate-archiveops-tests
branch
from
04368e7
to
bbee58e
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #431 +/- ##
==========================================
- Coverage 92.04% 91.35% -0.69%
==========================================
Files 122 125 +3
Lines 9100 9442 +342
==========================================
+ Hits 8376 8626 +250
- Misses 724 816 +92
☔ View full report in Codecov by Sentry. |
|
The PR is still in draft mode, hence closing it again after some months have passed. #407 seems to have most of these changes, and is not in draft mode. I will look into merging that on instead. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR factors out common test code for archive operations, i.e. for tar-archive operations and zip-archive operations.
This PR is in draft mode, because:
ZipArchiveOperations, which implements archive operations on zip-files #407, and PR Fix directory name handling in zip-iterator #430, are not merged into main yet, I merged them into this branch to be able to run the tests. This branch will be rebased and fixed, once PR 407, and PR 430 are merged.