Fix tests. Merge pull request #320 from JamesParrott/run_network_tests_on_localhost

JamesParrott · web-flow · commit acd1fe90416b · 2025-07-21T00:57:13.000+01:00
Use localhost instead of remote server for network marked tests.
diff --git 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
+        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 }}
diff --git 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
+    - 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
@@ -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
@@ -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,41 +2799,113 @@ 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(","):
             target = target.strip()
             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
