From e531abd82e777de170c79d7571985fa15fc1b2be Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 16:50:25 +0100 Subject: [PATCH 01/12] Enable tests on recent versions of Python and on Windows and macOS --- .github/workflows/tests.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e24302e..ddec63a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -4,11 +4,12 @@ on: [push, pull_request] jobs: tests: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os-version }} strategy: fail-fast: false matrix: - python-version: [3.6, 3.7, 3.8, 3.9] + os-version: ['ubuntu-latest', 'windows-latest', 'macos-latest '] + python-version: [3.9, 3.10, 3.11, 3.12, 3.13] steps: - uses: actions/checkout@v2 From 9b7d72dc2c44fca7d1fe99f586d8e7505cbee4a3 Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 16:50:40 +0100 Subject: [PATCH 02/12] Update the script to allow running a specific test. --- run_tests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/run_tests.py b/run_tests.py index ea5d47d..1eb0d53 100644 --- a/run_tests.py +++ b/run_tests.py @@ -12,6 +12,8 @@ 'test.test_zipfile_aes', 'test.test_zipfile2' ]) + else: + names.extend(sys.argv[1:]) else: names.append('test.test_zipfile') From 5c81cbfb3b2ccd3f160652bc3fd73d225f1533a8 Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 16:51:04 +0100 Subject: [PATCH 03/12] Update the tests on Windows. --- test/test_zipfile.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/test/test_zipfile.py b/test/test_zipfile.py index e1a3bae..6dcb91b 100644 --- a/test/test_zipfile.py +++ b/test/test_zipfile.py @@ -1419,12 +1419,15 @@ def test_extract_hackers_arcnames_windows_only(self): (r'C:/foo/bar', 'foo/bar'), (r'C://foo/bar', 'foo/bar'), (r'C:\foo\bar', 'foo/bar'), - (r'//conky/mountpoint/foo/bar', 'foo/bar'), - (r'\\conky\mountpoint\foo\bar', 'foo/bar'), - (r'///conky/mountpoint/foo/bar', 'conky/mountpoint/foo/bar'), - (r'\\\conky\mountpoint\foo\bar', 'conky/mountpoint/foo/bar'), - (r'//conky//mountpoint/foo/bar', 'conky/mountpoint/foo/bar'), - (r'\\conky\\mountpoint\foo\bar', 'conky/mountpoint/foo/bar'), + # The server together with the share name are considered as the drive root. + (r'//server-name/share-name/foo/bar', 'foo/bar'), + (r'\\server-name\share-name\foo\bar', 'foo/bar'), + # Here there is no server name. + (r'///share-name/root-without-server-name/foo/bar', 'root-without-server-name/foo/bar'), + (r'\\\share-name\root-without-server-name\foo\bar', 'root-without-server-name/foo/bar'), + # Here there is no share name. + (r'//server-name//root-dir-without-share-name/foo/bar', 'root-dir-without-share-name/foo/bar'), + (r'\\server-name\\root-dir-without-share-name\foo\bar', 'root-dir-without-share-name/foo/bar'), (r'//?/C:/foo/bar', 'foo/bar'), (r'\\?\C:\foo\bar', 'foo/bar'), (r'C:/../C:/foo/bar', 'C_/foo/bar'), @@ -1459,7 +1462,7 @@ def _test_extract_hackers_arcnames(self, hacknames): with zipfile.ZipFile(TESTFN2, 'r') as zipfp: writtenfile = zipfp.extract(arcname, targetpath) self.assertEqual(writtenfile, correctfile, - msg='extract %r: %r != %r' % + msg='extract %r\nactual : %r\nexpected: %r' % (arcname, writtenfile, correctfile)) self.check_file(correctfile, content) rmtree('target') From f020daad25148275ddabb8d9e8928374b80fd59c Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 16:51:23 +0100 Subject: [PATCH 04/12] Update tox env list. --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 651b112..db9dac0 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{34,35,36,37,38,39}, flake8 +envlist = py{39,310,311,312,313}, flake8 [flake8] max-line-length = 99 From 0d4cf4ed0251d8ba917de0d08dbb4978b17f685e Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 16:54:58 +0100 Subject: [PATCH 05/12] Remove usage of nntplib. --- test/support/__init__.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/support/__init__.py b/test/support/__init__.py index 53aee0f..a767370 100644 --- a/test/support/__init__.py +++ b/test/support/__init__.py @@ -16,7 +16,6 @@ import importlib.util import io import logging.handlers -import nntplib import os import platform import re @@ -1492,10 +1491,6 @@ def filter_error(err): if timeout is not None: socket.setdefaulttimeout(timeout) yield - except nntplib.NNTPTemporaryError as err: - if verbose: - sys.stderr.write(denied.args[0] + "\n") - raise denied from err except OSError as err: # urllib can wrap original socket errors multiple times (!), we must # unwrap to get at the original error. From 6e6ad0be4e9110acd81c1c2b5a6688dc02284ebf Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 17:27:47 +0100 Subject: [PATCH 06/12] Run tests in GitHub Actions directly using pytest. Skip large file tests. --- .github/workflows/tests.yml | 7 +++++-- test/support/__init__.py | 4 +--- test/test_zipfile2.py | 15 +++++++++------ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ddec63a..a5c8a8d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -13,14 +13,17 @@ jobs: steps: - uses: actions/checkout@v2 + - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} + - name: Install dependencies run: | python -m pip install --upgrade pip tox tox-factor - - name: Run tox targets for ${{ matrix.python-version }} + + - name: Run all tests for ${{ matrix.python-version }} run: | PYVERSION=$(python -c "import sys; print(''.join([str(sys.version_info.major), str(sys.version_info.minor)]))") - python -m tox -f "py${PYVERSION}" + python -m pytest diff --git a/test/support/__init__.py b/test/support/__init__.py index a767370..980d620 100644 --- a/test/support/__init__.py +++ b/test/support/__init__.py @@ -6,7 +6,6 @@ import asyncio.events import collections.abc import contextlib -import datetime import errno import faulthandler import fnmatch @@ -295,7 +294,7 @@ def get_attribute(obj, name): return attribute verbose = 1 # Flag set to 0 by regrtest.py -use_resources = None # Flag set to [] by regrtest.py +use_resources = [] # Flag set to [] by regrtest.py max_memuse = 0 # Disable bigmem tests (they will still be run with # small sizes, to make sure they work.) real_max_memuse = 0 @@ -1517,7 +1516,6 @@ def filter_error(err): def captured_output(stream_name): """Return a context manager used by captured_stdout/stdin/stderr that temporarily replaces the sys stream *stream_name* with a StringIO.""" - import io orig_stdout = getattr(sys, stream_name) setattr(sys, stream_name, io.StringIO()) try: diff --git a/test/test_zipfile2.py b/test/test_zipfile2.py index 37c626a..47221fa 100644 --- a/test/test_zipfile2.py +++ b/test/test_zipfile2.py @@ -1172,12 +1172,15 @@ def test_extract_hackers_arcnames_windows_only(self): (r'C:/foo/bar', 'foo/bar'), (r'C://foo/bar', 'foo/bar'), (r'C:\foo\bar', 'foo/bar'), - (r'//conky/mountpoint/foo/bar', 'foo/bar'), - (r'\\conky\mountpoint\foo\bar', 'foo/bar'), - (r'///conky/mountpoint/foo/bar', 'conky/mountpoint/foo/bar'), - (r'\\\conky\mountpoint\foo\bar', 'conky/mountpoint/foo/bar'), - (r'//conky//mountpoint/foo/bar', 'conky/mountpoint/foo/bar'), - (r'\\conky\\mountpoint\foo\bar', 'conky/mountpoint/foo/bar'), + # The server together with the share name are considered as the drive root. + (r'//server-name/share-name/foo/bar', 'foo/bar'), + (r'\\server-name\share-name\foo\bar', 'foo/bar'), + # Here there is no server name. + (r'///share-name/root-without-server-name/foo/bar', 'root-without-server-name/foo/bar'), + (r'\\\share-name\root-without-server-name\foo\bar', 'root-without-server-name/foo/bar'), + # Here there is no share name. + (r'//server-name//root-dir-without-share-name/foo/bar', 'root-dir-without-share-name/foo/bar'), + (r'\\server-name\\root-dir-without-share-name\foo\bar', 'root-dir-without-share-name/foo/bar'), (r'//?/C:/foo/bar', 'foo/bar'), (r'\\?\C:\foo\bar', 'foo/bar'), (r'C:/../C:/foo/bar', 'C_/foo/bar'), From 8a21ce74954fe0b5518282cccfe18672b357257f Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 17:39:15 +0100 Subject: [PATCH 07/12] Update the code to pass with pytest. Make tox optional. --- CONTRIBUTING.rst | 12 ++++++----- run_tests.py | 23 -------------------- test/support/__init__.py | 28 ++++++++---------------- test/test_support.py | 46 +--------------------------------------- 4 files changed, 17 insertions(+), 92 deletions(-) delete mode 100644 run_tests.py diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index a95a8a6..1e744d6 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -4,8 +4,8 @@ Contributing ============ -Contributions are welcome, and they are greatly appreciated! Every little bit -helps, and credit will always be given. +Contributions are welcome, and they are greatly appreciated! +Every little bit helps, and credit will always be given. You can contribute in many ways: @@ -78,10 +78,12 @@ Ready to contribute? Here's how to set up `pyzipper` for local development. Now you can make your changes locally. 5. When you're done making changes, check that your changes pass flake8 and the - tests, including testing other Python versions with tox:: + tests. + You can run tests for other Python versions with tox. + When a PR is created, it will trigger GitHub Action tests for all the supported Python versions:: - - $ tox + $ pytest test + $ flake8 To get flake8 and tox, just pip install them into your virtualenv. diff --git a/run_tests.py b/run_tests.py deleted file mode 100644 index 1eb0d53..0000000 --- a/run_tests.py +++ /dev/null @@ -1,23 +0,0 @@ -import sys -import unittest - - -names = [] -if len(sys.argv) > 1: - test_type = sys.argv[1] - if test_type == 'extralargefile': - names.append('test.test_zipfile64') - elif test_type == 'aes': - names.extend([ - 'test.test_zipfile_aes', - 'test.test_zipfile2' - ]) - else: - names.extend(sys.argv[1:]) -else: - names.append('test.test_zipfile') - -suite = unittest.TestLoader().loadTestsFromNames(names) -runner = unittest.TextTestRunner(verbosity=2).run(suite) -ret = not runner.wasSuccessful() -sys.exit(ret) diff --git a/test/support/__init__.py b/test/support/__init__.py index 980d620..8c76005 100644 --- a/test/support/__init__.py +++ b/test/support/__init__.py @@ -1,4 +1,12 @@ -"""Supporting definitions for the Python regression tests.""" +""" +Supporting definitions for the Python regression tests. + +Copied from a very old stdblib. +Code not supported on latest Python version was removed. + +The goal is to move pyzipper tests to pytest so that we will +no longer need the code from here. +""" if __name__ != 'test.support': raise ImportError('support must be imported from the test package') @@ -1999,24 +2007,6 @@ def match_test_regex(test_id): _match_test_func = func - -def run_unittest(*classes): - """Run tests from unittest.TestCase-derived classes.""" - valid_types = (unittest.TestSuite, unittest.TestCase) - suite = unittest.TestSuite() - for cls in classes: - if isinstance(cls, str): - if cls in sys.modules: - suite.addTest(unittest.findTestCases(sys.modules[cls])) - else: - raise ValueError("str arguments must be keys in sys.modules") - elif isinstance(cls, valid_types): - suite.addTest(cls) - else: - suite.addTest(unittest.makeSuite(cls)) - _filter_suite(suite, match_test) - _run_suite(suite) - #======================================================================= # Check for the presence of docstrings. diff --git a/test/test_support.py b/test/test_support.py index afdc391..14a9676 100644 --- a/test/test_support.py +++ b/test/test_support.py @@ -290,11 +290,6 @@ def test_check_syntax_error(self): with self.assertRaises(AssertionError): support.check_syntax_error(self, "x=1") - def test_CleanImport(self): - import importlib - with support.CleanImport("asyncore"): - importlib.import_module("asyncore") - def test_DirsOnSysPath(self): with support.DirsOnSysPath('foo', 'bar'): self.assertIn("foo", sys.path) @@ -397,7 +392,7 @@ def test_check__all__(self): extra=extra, blacklist=blacklist) - extra = {'TextTestResult', 'installHandler'} + extra = {'TextTestResult', 'installHandler', 'IsolatedAsyncioTestCase'} blacklist = {'load_tests', "TestProgram", "BaseTestSuite"} support.check__all__(self, @@ -469,37 +464,6 @@ def check_options(self, args, func): self.assertEqual(proc.stdout.rstrip(), repr(args)) self.assertEqual(proc.returncode, 0) - def test_args_from_interpreter_flags(self): - # Test test.support.args_from_interpreter_flags() - for opts in ( - # no option - [], - # single option - ['-B'], - ['-s'], - ['-S'], - ['-E'], - ['-v'], - ['-b'], - ['-q'], - # same option multiple times - ['-bb'], - ['-vvv'], - # -W options - ['-Wignore'], - # -X options - ['-X', 'dev'], - ['-Wignore', '-X', 'dev'], - ['-X', 'faulthandler'], - ['-X', 'importtime'], - ['-X', 'showalloccount'], - ['-X', 'showrefcount'], - ['-X', 'tracemalloc'], - ['-X', 'tracemalloc=3'], - ): - with self.subTest(opts=opts): - self.check_options(opts, 'args_from_interpreter_flags') - def test_optim_args_from_interpreter_flags(self): # Test test.support.optim_args_from_interpreter_flags() for opts in ( @@ -605,11 +569,3 @@ def test_fd_count(self): # can_symlink # skip_unless_symlink # SuppressCrashReport - - -def test_main(): - tests = [TestSupport] - support.run_unittest(*tests) - -if __name__ == '__main__': - test_main() From 7e402563cb8ef3e9c27ce3a85f64157c87424f6b Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 17:48:03 +0100 Subject: [PATCH 08/12] Fix GHA deps install. --- .github/workflows/tests.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a5c8a8d..e4bf22f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -8,8 +8,16 @@ jobs: strategy: fail-fast: false matrix: - os-version: ['ubuntu-latest', 'windows-latest', 'macos-latest '] - python-version: [3.9, 3.10, 3.11, 3.12, 3.13] + os-version: + - ubuntu-latest + - windows-latest + - macos-latest + python-version: + - '3.9' + - '3.10' + - '3.11' + - '3.12' + - '3.13' steps: - uses: actions/checkout@v2 @@ -21,7 +29,7 @@ jobs: - name: Install dependencies run: | - python -m pip install --upgrade pip tox tox-factor + python -m pip install -r requirements_dev.txt - name: Run all tests for ${{ matrix.python-version }} run: | From ab31972960094f79b55ca6f265de9eff75b73806 Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 17:50:32 +0100 Subject: [PATCH 09/12] Install pyzipper. --- .github/workflows/tests.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e4bf22f..09e9ce1 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -31,6 +31,11 @@ jobs: run: | python -m pip install -r requirements_dev.txt + - name: Install project + run: | + python -m pip install -e . + + - name: Run all tests for ${{ matrix.python-version }} run: | PYVERSION=$(python -c "import sys; print(''.join([str(sys.version_info.major), str(sys.version_info.minor)]))") From 1bc97ab638ae1d37175b29f878fd3d96124ede00 Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 18:11:52 +0100 Subject: [PATCH 10/12] Fix test support overlap with stdlib. --- .github/workflows/tests.yml | 17 +++++++++++++++-- test/test_support.py | 6 +++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 09e9ce1..aec6f46 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,6 +1,20 @@ --- name: Tests -on: [push, pull_request] +on: + push: + branches: [master] + pull_request: + +# Start with explicit read-only permissions. +permissions: + contents: read + + +# Cancel pending jobs if a new commit was pushed for the same branch. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: tests: @@ -38,5 +52,4 @@ jobs: - name: Run all tests for ${{ matrix.python-version }} run: | - PYVERSION=$(python -c "import sys; print(''.join([str(sys.version_info.major), str(sys.version_info.minor)]))") python -m pytest diff --git a/test/test_support.py b/test/test_support.py index 14a9676..ff2f2e1 100644 --- a/test/test_support.py +++ b/test/test_support.py @@ -174,6 +174,10 @@ def test_temp_dir__forked_child(self): # Run the test as an external script, because it uses fork. script_helper.assert_python_ok("-c", textwrap.dedent(""" import os + import sys + # Make sure we don't load the `test` code from stdlib. + sys.path.insert(0, os.getcwd()) + print(os.getcwd()) from test import support with support.temp_cwd() as temp_path: pid = os.fork() @@ -192,7 +196,7 @@ def test_temp_dir__forked_child(self): # directory. if not os.path.isdir(temp_path): raise AssertionError("Child removed temp_path.") - """)) + """), __cwd=os.getcwd()) # Tests for change_cwd() From 25d6d0cd49a41924dc53373971a613c983eeb438 Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Mon, 7 Jul 2025 21:50:13 +0100 Subject: [PATCH 11/12] Try to implement workaround for py3.9 and 3.10. --- pyzipper/zipfile.py | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/pyzipper/zipfile.py b/pyzipper/zipfile.py index ae46a65..ebb0241 100644 --- a/pyzipper/zipfile.py +++ b/pyzipper/zipfile.py @@ -828,7 +828,7 @@ def from_file(cls, filename, arcname=None, *, strict_timestamps=True): # Create ZipInfo instance to store file information if arcname is None: arcname = filename - arcname = os.path.normpath(os.path.splitdrive(arcname)[1]) + arcname = os.path.normpath(compat_strip_drive(arcname)) while arcname[0] in (os.sep, os.altsep): arcname = arcname[1:] if isdir: @@ -2135,7 +2135,7 @@ def _extract_member(self, member, targetpath, pwd): arcname = arcname.replace(os.path.altsep, os.path.sep) # interpret absolute pathname as relative, remove drive letter or # UNC path, redundant separators, "." and ".." components. - arcname = os.path.splitdrive(arcname)[1] + arcname = compat_strip_drive(arcname) invalid_path_parts = ('', os.path.curdir, os.path.pardir) arcname = os.path.sep.join(x for x in arcname.split(os.path.sep) if x not in invalid_path_parts) @@ -2538,6 +2538,35 @@ def _compile(file, optimize=-1): return (fname, archivename) +def compat_strip_drive(path): + """ + Get os.path.splitdrive to behave the same in any Python version. + + This can be removed once we no longer support Python 3.10. + """ + if os.name != 'nt' or sys.version_info[:2] > (3,10): + return os.path.splitdrive(path)[1] + + # We are on Windows and Python 3.10 or older. + + if not (path.startswith('//') or path.startswith(r'\\')): + # Not a Windows Share path. + return os.path.splitdrive(path)[1] + + # Simplify path handling by using only unix separators. + sane_path = path.replace('\\', '/').rstrip('/') + + if sane_path.startswith('///'): + # Windows Share path without server name. + sane_path = '//server-ignored/' + sane_path[3:] + + parts = sane_path.split('/') + if len(parts) > 3 and parts[3] == '': + return '/'.join(parts[4:]) + + return os.path.splitdrive(sane_path)[1] + + def main(args=None): import argparse From b3bee9f7e418226054a5423ae5818403e315cca3 Mon Sep 17 00:00:00 2001 From: Adi Roiban Date: Tue, 8 Jul 2025 00:22:28 +0100 Subject: [PATCH 12/12] Fix 3.10. --- pyzipper/zipfile.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pyzipper/zipfile.py b/pyzipper/zipfile.py index ebb0241..c38e759 100644 --- a/pyzipper/zipfile.py +++ b/pyzipper/zipfile.py @@ -2554,15 +2554,24 @@ def compat_strip_drive(path): return os.path.splitdrive(path)[1] # Simplify path handling by using only unix separators. - sane_path = path.replace('\\', '/').rstrip('/') + sane_path = path if sane_path.startswith('///'): # Windows Share path without server name. sane_path = '//server-ignored/' + sane_path[3:] + if sane_path.startswith('\\\\\\'): + # Windows Share path without server name. + sane_path = '\\\\server-ignored\\' + sane_path[3:] + + #import pdb; pdb.set_trace() parts = sane_path.split('/') if len(parts) > 3 and parts[3] == '': - return '/'.join(parts[4:]) + return '/' + '/'.join(parts[4:]) + + parts = sane_path.split('\\') + if len(parts) > 3 and parts[3] == '': + return '\\' + '\\'.join(parts[4:]) return os.path.splitdrive(sane_path)[1]