From 34ced213f7b7ecd5d82e5ad31a47c3c82ce6c132 Mon Sep 17 00:00:00 2001
From: david-streamlio <35466513+david-streamlio@users.noreply.github.com>
Date: Thu, 20 Aug 2026 17:24:49 -0700
Subject: [PATCH] [improve][ci] Run the Python function instance tests in CI

### Motivation

`pulsar-functions/instance/src/scripts/run_python_instance_tests.sh` exists but
nothing invokes it: no workflow, no Gradle task. A repository-wide search for the
script name returns only the script itself. The unit tests for the Python function
instance (`contextimpl.py`, `python_instance.py`, `secretsprovider.py`) have
therefore never run in CI, so a change to the Python runtime can go green on a
full CI run without any of its tests being executed.

The Go function runtime has had this coverage since `ci-go-functions.yaml` was
added. This gives the Python runtime the equivalent.

### Modifications

Add `.github/workflows/ci-python-functions.yaml`, modelled on
`ci-go-functions.yaml`: the same `preconditions` job, triggered on changes to the
Python instance sources, tests or scripts, running the tests on Python 3.12 and
3.13.

`run_python_instance_tests.sh` needed three fixes before it could be wired up:

1. **Dependencies were incomplete.** It installed `mock`, `protobuf==6.31.1` and
   `fastavro`, but the instance also imports `grpc`, `ratelimit`,
   `prometheus_client` and `bookkeeper`, so collection failed. Installing
   `pulsar-client[all]` brings in all four, and the pinned versions now match
   `docker/pulsar/Dockerfile`, so the tests run against the versions the instance
   actually runs with. The versions are overridable by environment variable.

2. **The modules cannot share an interpreter.** `test_python_instance` installs a
   `Mock` over `prometheus_client` in `sys.modules`, which breaks
   `test_python_instance_main` when it later imports the real
   `prometheus_client.exposition`; removing the mock instead trips a
   `DuplicateTimeseries` error, because both modules register the same Prometheus
   metrics. `unittest discover` therefore always failed on one module. Each module
   now runs in its own interpreter. The loop runs every module before reporting,
   so one failure does not hide the others.

3. **`pip install --user` fails inside a virtualenv**, which is how the script is
   most likely to be run locally. The flag is dropped; `SKIP_PYTHON_DEPS=true`
   skips installation entirely for a pre-prepared environment.

No test assertions are changed, and no test file is touched.

### Verifying this change

The script passes on Python 3.12 and 3.13, running all three modules (7 tests) on
an unmodified `master`, including a clean-virtualenv run that exercises the
dependency installation. Injecting a deliberately failing assertion makes the
script exit 1 and name the failing module, confirming a failure is not swallowed.
---
 .github/workflows/ci-python-functions.yaml    | 90 +++++++++++++++++++
 .../src/scripts/run_python_instance_tests.sh  | 54 +++++++++--
 2 files changed, 137 insertions(+), 7 deletions(-)
 create mode 100644 .github/workflows/ci-python-functions.yaml

diff --git a/.github/workflows/ci-python-functions.yaml b/.github/workflows/ci-python-functions.yaml
new file mode 100644
index 0000000000000..d02ed5303ebde
--- /dev/null
+++ b/.github/workflows/ci-python-functions.yaml
@@ -0,0 +1,90 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+name: CI - Python Functions
+on:
+  pull_request:
+    branches:
+      - master
+    paths:
+      - '.github/workflows/**'
+      - 'pulsar-functions/instance/src/main/python/**'
+      - 'pulsar-functions/instance/src/test/python/**'
+      - 'pulsar-functions/instance/src/scripts/**'
+  workflow_dispatch:
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  preconditions:
+    name: Preconditions
+    runs-on: ubuntu-24.04
+    outputs:
+      docs_only: ${{ steps.check_changes.outputs.docs_only }}
+    steps:
+      - name: checkout
+        uses: actions/checkout@v6
+
+      - name: Detect changed files
+        id: changes
+        uses: apache/pulsar-test-infra/paths-filter@master
+        with:
+          filters: .github/changes-filter.yaml
+          list-files: csv
+
+      - name: Check changed files
+        id: check_changes
+        run: |
+          if [[ "${GITHUB_EVENT_NAME}" != "schedule" ]]; then
+            echo "docs_only=${{ fromJSON(steps.changes.outputs.all_count) == fromJSON(steps.changes.outputs.docs_count) && fromJSON(steps.changes.outputs.docs_count) > 0 }}" >> $GITHUB_OUTPUT
+          else
+            echo docs_only=false >> $GITHUB_OUTPUT
+          fi
+
+      - name: Check if the PR is ready for running CI
+        if: ${{ steps.check_changes.outputs.docs_only != 'true' && github.repository == 'apache/pulsar' && github.event_name == 'pull_request' }}
+        uses: ./.github/actions/check-pr-ready-to-test
+
+  instance-tests:
+    needs: preconditions
+    if: ${{ needs.preconditions.outputs.docs_only != 'true' }}
+    name: Python ${{ matrix.python-version }} Functions instance tests
+    runs-on: ubuntu-24.04
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version: ['3.12', '3.13']
+
+    steps:
+      - name: checkout
+        uses: actions/checkout@v6
+
+      - name: Tune Runner VM
+        uses: ./.github/actions/tune-runner-vm
+
+      - name: Set up Python
+        uses: actions/setup-python@v6
+        with:
+          python-version: ${{ matrix.python-version }}
+
+      - name: Run Python instance tests
+        run: |
+          ./pulsar-functions/instance/src/scripts/run_python_instance_tests.sh
diff --git a/pulsar-functions/instance/src/scripts/run_python_instance_tests.sh b/pulsar-functions/instance/src/scripts/run_python_instance_tests.sh
index 11cec15e14c09..b0a0850de2044 100755
--- a/pulsar-functions/instance/src/scripts/run_python_instance_tests.sh
+++ b/pulsar-functions/instance/src/scripts/run_python_instance_tests.sh
@@ -18,14 +18,54 @@
 # under the License.
 #
 
-
-# Make sure dependencies are installed
-pip3 install mock --user
-pip3 install protobuf==6.31.1 --user
-pip3 install fastavro --user
+set -o errexit
+set -o nounset
+set -o pipefail
 
 CUR_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"
 PULSAR_HOME="$( cd "$CUR_DIR/../../../../" >/dev/null && pwd )"
 
-# run instance tests
-PULSAR_HOME=${PULSAR_HOME} PYTHONPATH=${PULSAR_HOME}/pulsar-functions/instance/src/main/python python3 -m unittest discover -v -s ${PULSAR_HOME}/pulsar-functions/instance/src/test/python
+PYTHON_BIN=${PYTHON_BIN:-python3}
+
+# Keep these aligned with the Python dependencies installed in docker/pulsar/Dockerfile: the tests
+# should run against the same versions the Python instance runs with in production. pulsar-client's
+# "all" extra brings in apache-bookkeeper-client, fastavro, prometheus_client and ratelimit, which
+# the instance imports.
+PULSAR_CLIENT_PYTHON_VERSION=${PULSAR_CLIENT_PYTHON_VERSION:-3.13.0}
+PYTHON_GRPCIO_VERSION=${PYTHON_GRPCIO_VERSION:-1.78.0}
+PYTHON_PROTOBUF_VERSION=${PYTHON_PROTOBUF_VERSION:-6.33.6}
+
+# Set SKIP_PYTHON_DEPS=true to run against an environment you have prepared yourself. Otherwise the
+# dependencies are installed into whatever ${PYTHON_BIN} resolves to, so run this inside a
+# virtualenv unless you want them on your system interpreter.
+if [[ "${SKIP_PYTHON_DEPS:-false}" != "true" ]]; then
+  ${PYTHON_BIN} -m pip install \
+    mock \
+    "pulsar-client[all]==${PULSAR_CLIENT_PYTHON_VERSION}" \
+    "grpcio==${PYTHON_GRPCIO_VERSION}" \
+    "protobuf==${PYTHON_PROTOBUF_VERSION}"
+fi
+
+TEST_DIR="${PULSAR_HOME}/pulsar-functions/instance/src/test/python"
+export PULSAR_HOME
+export PYTHONPATH="${PULSAR_HOME}/pulsar-functions/instance/src/main/python"
+
+# Each test module runs in its own interpreter. They cannot share one: test_python_instance replaces
+# prometheus_client with a mock in sys.modules, which breaks test_python_instance_main when it later
+# imports the real one, and the two modules also register the same Prometheus metrics, so a shared
+# registry rejects the duplicates.
+failed_modules=()
+for test_file in "${TEST_DIR}"/test_*.py; do
+  module_name="$(basename "${test_file}" .py)"
+  echo "=== Running ${module_name} ==="
+  if ! (cd "${TEST_DIR}" && ${PYTHON_BIN} -m unittest -v "${module_name}"); then
+    failed_modules+=("${module_name}")
+  fi
+done
+
+if [[ ${#failed_modules[@]} -gt 0 ]]; then
+  echo "Failed test modules: ${failed_modules[*]}" >&2
+  exit 1
+fi
+
+echo "All Python instance test modules passed"