diff --git a/.github/actions/test/action.yml b/.github/actions/test/action.yml index 1020606..fd4fee7 100644 --- a/.github/actions/test/action.yml +++ b/.github/actions/test/action.yml @@ -1,38 +1,119 @@ name: - Test + Run Doctests and Pytest description: Run pytest, and run the doctest runner (shapefile.py as a script). inputs: extra_args: - type: string + description: Extra command line args for Pytest and python shapefile.py default: '-m "not network"' + required: false + replace_remote_urls_with_localhost: + description: yes or no. Test loading shapefiles from a url, without overloading an external server from 30 parallel workflows. + default: 'no' + required: false + pyshp_repo_directory: + description: Path to where the PyShp repo was checked out to (to keep separate from Shapefiles & artefacts repo). + required: false + default: '.' + python-version: + description: Set to "2.7" to use caddy instead of python -m SimpleHTTPServer + required: true + + runs: using: "composite" steps: - # The Repo is required to already be checked out, e.g. by the calling workflow + # The PyShp repo is required to already be checked out into pyshp_repo_directory, + # e.g. by the calling workflow using: + # steps: + # - uses: actions/checkout@v4 + # with: + # path: ./Pyshp + # and then calling this Action with: + # - name: Run tests + # uses: ./Pyshp/.github/actions/test + # with: + # extra_args: "" + # replace_remote_urls_with_localhost: 'yes' + # pyshp_repo_directory: ./Pyshp # The Python to be tested with is required to already be setup, with "python" and "pip" on the system Path + - name: Checkout shapefiles and zip file artefacts repo + if: ${{ inputs.replace_remote_urls_with_localhost == 'yes' }} + uses: actions/checkout@v4 + with: + repository: JamesParrott/PyShp_test_shapefile + path: ./PyShp_test_shapefile + + - name: Serve shapefiles and zip file artefacts on localhost + if: ${{ inputs.replace_remote_urls_with_localhost == 'yes' && inputs.python-version != '2.7'}} + shell: bash + working-directory: ./PyShp_test_shapefile + run: | + python -m http.server 8000 & + echo "HTTP_SERVER_PID=$!" >> $GITHUB_ENV + sleep 4 # give server time to start + + - name: Download and unzip Caddy binary + if: ${{ inputs.replace_remote_urls_with_localhost == 'yes' && inputs.python-version == '2.7'}} + working-directory: . + shell: bash + run: | + curl -L https://github.com/caddyserver/caddy/releases/download/v2.10.0/caddy_2.10.0_linux_amd64.tar.gz --output caddy.tar.gz + tar -xzf caddy.tar.gz + + - name: Serve shapefiles and zip file artefacts on localhost using Caddy + if: ${{ inputs.replace_remote_urls_with_localhost == 'yes' && inputs.python-version == '2.7'}} + shell: bash + working-directory: . + run: | + ./caddy file-server --root ./PyShp_test_shapefile --listen :8000 & + echo "HTTP_SERVER_PID=$!" >> $GITHUB_ENV + sleep 2 # give server time to start + - name: Doctests shell: bash + working-directory: ${{ inputs.pyshp_repo_directory }} + env: + REPLACE_REMOTE_URLS_WITH_LOCALHOST: ${{ inputs.replace_remote_urls_with_localhost }} run: python shapefile.py ${{ inputs.extra_args }} - name: Install test dependencies. shell: bash + working-directory: ${{ inputs.pyshp_repo_directory }} run: | python -m pip install --upgrade pip pip install -r requirements.test.txt - name: Pytest shell: bash + working-directory: ${{ inputs.pyshp_repo_directory }} + env: + REPLACE_REMOTE_URLS_WITH_LOCALHOST: ${{ inputs.replace_remote_urls_with_localhost }} run: | - pytest ${{ inputs.extra_args }} + pytest -rA --tb=short ${{ inputs.extra_args }} - name: Show versions for logs. shell: bash run: | python --version - python -m pytest --version \ No newline at end of file + python -m pytest --version + + + # - name: Test http server + # # (needs a full Github Actions runner or a Python non-slim Docker image, + # # as the slim Debian images don't have curl or wget). + # if: ${{ inputs.replace_remote_urls_with_localhost == 'yes' }} + # shell: bash + # run: curl http://localhost:8000/ne_110m_admin_0_tiny_countries.shp + + - name: Stop http server + if: ${{ inputs.replace_remote_urls_with_localhost == 'yes' }} + shell: bash + run: | + echo Killing http server process ID: ${{ env.HTTP_SERVER_PID }} + kill ${{ env.HTTP_SERVER_PID }} \ No newline at end of file diff --git a/.github/workflows/run_tests_hooks_and_tools.yml b/.github/workflows/run_tests_hooks_and_tools.yml index 94995eb..468b2e2 100644 --- a/.github/workflows/run_tests_hooks_and_tools.yml +++ b/.github/workflows/run_tests_hooks_and_tools.yml @@ -5,7 +5,7 @@ name: Run pre-commit hooks and tests on: push: pull_request: - branches: [ master ] + branches: [ master, ] workflow_call: workflow_dispatch: @@ -30,7 +30,7 @@ jobs: run: | pylint --disable=R,C test_shapefile.py - test_on_old_Pythons: + test_on_EOL_Pythons: strategy: fail-fast: false matrix: @@ -44,16 +44,28 @@ jobs: runs-on: ubuntu-latest container: - image: python:${{ matrix.python-version }}-slim + image: python:${{ matrix.python-version }} steps: - uses: actions/checkout@v4 + with: + path: ./Pyshp - - name: Run tests - uses: ./.github/actions/test + - name: Non-network tests + uses: ./Pyshp/.github/actions/test + with: + pyshp_repo_directory: ./Pyshp + python-version: ${{ matrix.python-version }} + - name: Network tests + uses: ./Pyshp/.github/actions/test + with: + extra_args: '-m network' + replace_remote_urls_with_localhost: 'yes' + pyshp_repo_directory: ./Pyshp + python-version: ${{ matrix.python-version }} - run_tests: + test_on_supported_Pythons: strategy: fail-fast: false matrix: @@ -74,11 +86,24 @@ jobs: runs-on: ${{ matrix.os }} steps: + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + - uses: actions/checkout@v4 + with: + path: ./Pyshp - - uses: actions/setup-python@v5 + - name: Non-network tests + uses: ./Pyshp/.github/actions/test with: + pyshp_repo_directory: ./Pyshp python-version: ${{ matrix.python-version }} - - name: Run tests - uses: ./.github/actions/test \ No newline at end of file + - name: Network tests + uses: ./Pyshp/.github/actions/test + with: + extra_args: '-m network' + replace_remote_urls_with_localhost: 'yes' + pyshp_repo_directory: ./Pyshp + python-version: ${{ matrix.python-version }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f065f59..ffe59bf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,15 +1,15 @@ repos: -- repo: https://github.com/pre-commit/pre-commit-hooks - rev: v2.3.0 +- repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.6.4 hooks: - - id: check-yaml - - id: trailing-whitespace + - id: ruff-format - repo: https://github.com/pycqa/isort rev: 5.13.2 hooks: - id: isort name: isort (python) -- repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.6.4 +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v2.3.0 hooks: - - id: ruff-format + - id: check-yaml + - id: trailing-whitespace diff --git a/shapefile.py b/shapefile.py index be3650d..31c2f7f 100644 --- a/shapefile.py +++ b/shapefile.py @@ -25,6 +25,11 @@ # Module settings VERBOSE = True +# Test config (for the Doctest runner and test_shapefile.py) +REPLACE_REMOTE_URLS_WITH_LOCALHOST = ( + os.getenv("REPLACE_REMOTE_URLS_WITH_LOCALHOST", "").lower() == "yes" +) + # Constants for shape types NULL = 0 POINT = 1 @@ -2794,12 +2799,27 @@ def _get_doctests(): return tests -def _get_no_network_doctests(examples): +def _filter_network_doctests(examples, include_network=False, include_non_network=True): globals_from_network_doctests = set() - for example in examples: + + if not (include_network or include_non_network): + return + + examples_it = iter(examples) + + yield next(examples_it) + + for example in examples_it: + # Track variables in doctest shell sessions defined from commands + # that poll remote URLs, to skip subsequent commands until all + # such dependent variables are reassigned. + if 'sf = shapefile.Reader("https://' in example.source: globals_from_network_doctests.add("sf") + if include_network: + yield example continue + lhs = example.source.partition("=")[0] for target in lhs.split(","): @@ -2807,28 +2827,85 @@ def _get_no_network_doctests(examples): if target in globals_from_network_doctests: globals_from_network_doctests.remove(target) + # Non-network tests dependent on the network tests. if globals_from_network_doctests: + if include_network: + yield example + continue + + if not include_non_network: continue yield example -def _test(verbosity=0): +def _replace_remote_url( + old_url, + # Default port of Python http.server and Python 2's SimpleHttpServer + port=8000, + scheme="http", + netloc="localhost", + path=None, + params="", + query="", + fragment="", +): + old_parsed = urlparse(old_url) + + # Strip subpaths, so an artefacts + # repo or file tree can be simpler and flat + if path is None: + path = old_parsed.path.rpartition("/")[2] + + if port not in (None, ""): + netloc = "%s:%s" % (netloc, port) + + new_parsed = old_parsed._replace( + scheme=scheme, + netloc=netloc, + path=path, + params=params, + query=query, + fragment=fragment, + ) + + new_url = urlunparse(new_parsed) if PYTHON3 else urlunparse(list(new_parsed)) + return new_url + + +def _test(args=sys.argv[1:], verbosity=0): if verbosity == 0: print("Getting doctests...") - tests = _get_doctests() - - if len(sys.argv) >= 3 and sys.argv[1:3] == ["-m", "not network"]: - if verbosity == 0: - print("Removing doctests requiring internet access...") - tests.examples = list(_get_no_network_doctests(tests.examples)) import doctest + import re doctest.NORMALIZE_WHITESPACE = 1 - # ignore py2-3 unicode differences - import re + tests = _get_doctests() + + if len(args) >= 2 and args[0] == "-m": + if verbosity == 0: + print("Filtering doctests...") + tests.examples = list( + _filter_network_doctests( + tests.examples, + include_network=args[1] == "network", + include_non_network=args[1] == "not network", + ) + ) + + if REPLACE_REMOTE_URLS_WITH_LOCALHOST: + if verbosity == 0: + print("Replacing remote urls with http://localhost in doctests...") + + for example in tests.examples: + match_url_str_literal = re.search(r'"(https://.*)"', example.source) + if not match_url_str_literal: + continue + old_url = match_url_str_literal.group(1) + new_url = _replace_remote_url(old_url) + example.source = example.source.replace(old_url, new_url) class Py23DocChecker(doctest.OutputChecker): def check_output(self, want, got, optionflags): diff --git a/test_shapefile.py b/test_shapefile.py index 7984e91..1b7182f 100644 --- a/test_shapefile.py +++ b/test_shapefile.py @@ -487,16 +487,31 @@ def test_reader_url(): """ Assert that Reader can open shapefiles from a url. """ + + # Allow testing loading of shapefiles from a url on localhost (to avoid + # overloading external servers, and associated spurious test failures). + # A suitable repo of test files, and a localhost server setup is + # defined in ./.github/actions/test/actions.yml + if shapefile.REPLACE_REMOTE_URLS_WITH_LOCALHOST: + + def Reader(url): + new_url = shapefile._replace_remote_url(url) + print("repr(new_url): %s" % repr(new_url)) + return shapefile.Reader(new_url) + else: + print("Using plain Reader") + Reader = shapefile.Reader + # test with extension url = "https://github.com/nvkelso/natural-earth-vector/blob/master/110m_cultural/ne_110m_admin_0_tiny_countries.shp?raw=true" - with shapefile.Reader(url) as sf: + with Reader(url) as sf: for __recShape in sf.iterShapeRecords(): pass assert sf.shp.closed is sf.shx.closed is sf.dbf.closed is True # test without extension url = "https://github.com/nvkelso/natural-earth-vector/blob/master/110m_cultural/ne_110m_admin_0_tiny_countries?raw=true" - with shapefile.Reader(url) as sf: + with Reader(url) as sf: for __recShape in sf.iterShapeRecords(): pass assert len(sf) > 0 @@ -505,12 +520,12 @@ def test_reader_url(): # test no files found url = "https://raw.githubusercontent.com/nvkelso/natural-earth-vector/master/README.md" with pytest.raises(shapefile.ShapefileException): - with shapefile.Reader(url) as sf: + with Reader(url) as sf: pass # test reading zipfile from url url = "https://github.com/JamesParrott/PyShp_test_shapefile/raw/main/gis_osm_natural_a_free_1.zip" - with shapefile.Reader(url) as sf: + with Reader(url) as sf: for __recShape in sf.iterShapeRecords(): pass assert len(sf) > 0