Skip to content

[improve][ci] Run the Python function instance tests in CI - #26399

Merged
merlimat merged 1 commit into
apache:masterfrom
david-streamlio:ci-python-instance-tests
Aug 24, 2026
Merged

[improve][ci] Run the Python function instance tests in CI#26399
merlimat merged 1 commit into
apache:masterfrom
david-streamlio:ci-python-instance-tests

Conversation

@david-streamlio

Copy link
Copy Markdown
Contributor

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. A change to the Python runtime can pass a full CI run — 40+ green checks — without a single one of its tests being executed. I hit this on #26392: every check passed, and none of them ran the tests that PR added.

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.

Wiring the script up first required fixing three things that stopped it working at all:

1. The dependency list was incomplete. It installed mock, protobuf==6.31.1 and fastavro, but the instance also imports grpc, ratelimit, prometheus_client and bookkeeper, so collection failed before any test ran. Installing pulsar-client[all] brings in all four (its functions and avro extras), and the pinned versions now match docker/pulsar/Dockerfile, so the tests run against the versions the instance actually runs with in production. All versions are overridable by environment variable.

2. The test modules cannot share an interpreter. test_python_instance installs a Mock over prometheus_client in sys.modules at import time, which breaks test_python_instance_main when it later imports the real prometheus_client.exposition. Removing the mock is not the fix either — then both modules register the same Prometheus metrics and the shared registry raises DuplicateTimeseries. So unittest discover fails on one module or the other no matter what. Each module now runs in its own interpreter, which is what these tests have always implicitly required. The loop runs every module before reporting, so one failure does not hide the others.

3. pip install --user fails inside a virtualenv (Can not perform a '--user' install. User site-packages are not visible in this 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 — this PR does not overlap #26392.

Verifying this change

  • Make sure that the change passes the CI checks.

The workflow triggers on .github/workflows/**, so this PR runs the new job on itself.

Locally, against an unmodified master:

  • The script passes on Python 3.12 and 3.13, running all three modules (7 tests).
  • A clean-virtualenv run exercises the dependency installation end to end, not just the test execution.
  • Injecting a deliberately failing assertion makes the script exit 1 and name the failing module, confirming failures are not swallowed by the loop.

I also merged #26392 into this branch locally: it merges cleanly, and the script then runs 26 tests across the three modules — the 7 existing plus the 19 that PR adds. That is the state this is meant to protect, and it is green.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

No box checked. The dependency changes are confined to a test script's own pip install and do not affect anything shipped; they are aligned with the versions docker/pulsar/Dockerfile already installs.

Documentation

  • doc-not-needed

CI-only change with no user-facing behaviour.

### 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.
@david-streamlio david-streamlio added doc-not-needed Your PR changes do not impact docs area/function labels Aug 21, 2026
@merlimat
merlimat merged commit 9890872 into apache:master Aug 24, 2026
48 checks passed
@lhotari lhotari added this to the 5.0.0-M2 milestone Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/function doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants