From 19a9238d530cfccd5cd3f84fbaa8e0254413e9a0 Mon Sep 17 00:00:00 2001 From: James Parrott <80779630+JamesParrott@users.noreply.github.com> Date: Sat, 19 Jul 2025 12:09:50 +0100 Subject: [PATCH] Test Network tests on localhost Run on this branch Run Network tests in container jobs Remove type field from Action input Add description and required fields to Action input. Add option to Clone repo of copies of files to serve locally in mock remote url tests Specify shell in action job step Use actions/checkout instead of git (unavailable in Python slim docker images) Checkout artefacts repo to .. Add pyshp_repo_directory input Serve artefacts on localhost:8000 Specify shell: bash in Github action Don't curl from localhost - not available in Python slim images (neither is wget) TRy requesting on localhost in Python non-slim images Correct path to custom test file Test local server Sleep for twice as long after starting simple Python server Don't output PyTest version Swap out remote URLs with stripped path localhost version Rename input variable Import re where needed and reformat Reformat Change env var to be yes / no, not True / False Reformat Pass list to urlunparse on Python 2 Specify port 8000 Use double quotes Separate network tests and non-network tests into different steps Trim whitespace Try Network tests on all platforms Print simplified localhost urls during Pytest network tests Reorder pre-commit hooks and add blank line Try curling from simplified url on Python2 Test curl from server only Remove errant ' Run doctests against Python 2 SimpleHTTPServer Special case "import shapefile" in doctests filter Update shapefile.py Always include first example doctest Run Pytest tests in Python 2 non-slim container Update action.yml Update test_shapefile.py Update test_shapefile.py Explicitly export env var Don't need to explicitly set env var. Test without patching localhost. Reformat Update shapefile.py Update shapefile.py Use Caddy instead of SimpleHTTPServer Run caddy in backgorund Run all tests in all containers, all platforms, all Python versions Revert to python -m http.server to avoid overloading Caddy releases page --- .github/actions/test/action.yml | 91 ++++++++++++++++- .../workflows/run_tests_hooks_and_tools.yml | 43 ++++++-- .pre-commit-config.yaml | 14 +-- shapefile.py | 99 ++++++++++++++++--- test_shapefile.py | 23 ++++- 5 files changed, 234 insertions(+), 36 deletions(-) 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