[improve][ci] Run the Python function instance tests in CI - #26399
Merged
Conversation
### 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.
merlimat
approved these changes
Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
pulsar-functions/instance/src/scripts/run_python_instance_tests.shexists 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.yamlwas added. This gives the Python runtime the equivalent.Modifications
Add
.github/workflows/ci-python-functions.yaml, modelled onci-go-functions.yaml: the samepreconditionsjob, 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.1andfastavro, but the instance also importsgrpc,ratelimit,prometheus_clientandbookkeeper, so collection failed before any test ran. Installingpulsar-client[all]brings in all four (itsfunctionsandavroextras), and the pinned versions now matchdocker/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_instanceinstalls aMockoverprometheus_clientinsys.modulesat import time, which breakstest_python_instance_mainwhen it later imports the realprometheus_client.exposition. Removing the mock is not the fix either — then both modules register the same Prometheus metrics and the shared registry raisesDuplicateTimeseries. Sounittest discoverfails 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 --userfails 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=trueskips 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
The workflow triggers on
.github/workflows/**, so this PR runs the new job on itself.Locally, against an unmodified
master: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
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/Dockerfilealready installs.Documentation
doc-not-neededCI-only change with no user-facing behaviour.