You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Port the unit and end-to-end tests to pytest - #482
#473 asks how we should write e2e tests, and the bake-off pointed at pytest. This PR moves the e2e tests and the function unit tests to pytest, so the repo has one test runner, and rewrites every unit test to one consistent pattern, which CONTRIBUTING's Tests section now describes.
To judge the pattern, read compose-model-cache's tests. No unit test's input or expectation changed: every distinct request and response is the same as before. Mutation testing ran about 18,000 injected bugs in the function code against main's tests and this branch's. The branch's tests catch about 1,400 more than main's, and miss one main's catch. A function-test-style flake check now enforces the pattern's mechanical rules.
nix run .#test runs every function's unit tests. Name a function to run only its tests, and pass pytest arguments after it. Each case is a test of its own, with a short name:
$ nix run .#test -- compose-model-cachefunctions/compose-model-cache/tests/test_fn.py::test_compose[GKEFirstPass] PASSEDfunctions/compose-model-cache/tests/test_fn.py::test_compose[RevisionAndAuthSecret] PASSEDfunctions/compose-model-cache/tests/test_fn.py::test_compose[EKSFirstPass] PASSEDfunctions/compose-model-cache/tests/test_fn.py::test_compose[JobComplete] PASSED...14 passed in 0.12s
When a case fails, pytest prints a sentence saying what the case checks, then the whole response, with the expected lines as - and the actual lines as +, the same way round as Go's cmp.Diff(want, got). Here's one case with an expected value broken on purpose:
$ nix run .#test -- compose-model-cache -k JobCompletefunctions/compose-model-cache/tests/test_fn.py::test_compose[JobComplete] FAILED...E AssertionError: With its PVC bound and its hydration Job complete, a ModelCache drops the Job and reports Ready....E - 'phase': 'Hydrating',E ? ^ -------E + 'phase': 'Ready',E ? ^^^^...
nix run .#e2e -- --verify brings up both kind clusters as before, then runs the e2e tests, passing any further arguments to pytest:
Run nix flake check (or ./nix.sh flake check) and made sure it passes.
Added or updated tests covering any composition function changes. This PR changes only tests.
Signed off every commit with git commit -s.
negz
changed the title
Port the function unit tests to pytest and write their cases out in full
Port the unit and end-to-end tests to pytest
Oct 1, 2026
The checks build their virtualenvs from a uv2nix package set that checks.nix
defines for itself. The apps that run the tests need virtualenvs from the same
set, so this commit moves it to nix/python.nix, and flake.nix passes it to the
checks.
Signed-off-by: Nic Cope <nicc@rk0n.org>
Every polling attempt in test_usage_record downloads and reparses the complete Envoy log from every proxy. Because --test is designed for reuse against long-running clusters, these logs grow without bound and can make the one-minute poll increasingly expensive or time out. Keep a bounded tail here, as the previous e2e check did.
On Copilot's finding that e2e/gateway.py parses the whole Envoy log on every poll: a bounded tail would break this test. Every request logs an identical record, so the test proves its own request was metered by counting matching records before and after sending it. Once a tail fills with identical records, a new record pushes an old one out and the count never grows. run.sh's tail is also why it could pass on a record left over from an earlier run.
The reason will be displayed to describe this comment to others. Learn more.
@negz on board with the pattern. compose-model-cache's tests read well, and the thing that makes it stick is check_function_tests.py, because the rules are enforced rather than aspirational. Approving.
Two directional notes.
The LOC cost is worth writing into CONTRIBUTING rather than leaving for someone to discover. 22k lines of test code to 46.6k, with compose-serving-stack at 9.5k and compose-model-replica at 7.9k. That's the trade "write each case out in full" buys and I think it's the right one, but the next person adding a case to a 9.5k-line file should know it's deliberate.
"No unit test's input or expectation changed" isn't quite true for compose-model-replica. I checked this mechanically: ran both branches under a plugin recording every call into the function package with its args and result, and 18 of 19 functions match exactly. compose-model-replica drops 39 distinct calls and adds none. 29 are old tests computing expectations by calling the function's own helpers, which is the point of the rule. The other 10 are build() inputs that set phase or spec.serving, and build() reads neither, so no coverage is lost. Worth a clause in the description so the next reviewer who checks doesn't stop there.
FYI, impl things, take or leave:
nix run .#test -- -k Foo always exits 1. pytest returns 5 when -k selects nothing and the loop treats non-zero as failure, so every function reports failed while the matching test passed.
.#test has to run from the repo root. The pytest path is relative and nothing anchors it, so from a subdir every function exits 4.
logging.configure(DISABLED) went with setUpModule and nothing replaced it, so structlog is unconfigured and every RunFunction logs. Shows up in -s and in captured stdout ahead of the diff.
e2e/tests/test_telemetry.py:104 catches AssertionError, but kube.RETRY is four types now including TimeoutError, and wait.until re-raises the last one. Three escape the fallback, so all eight cases error opaquely instead of printing the per-line diagnostic.
e2e/tests/conftest.py:209 gives the first 200 a hard 120s where run.sh looped ten attempts. Downstream tests send one request with no retry of their own.
check_function_tests.py only checks helpers named _*, and doesn't run over e2e/tests, which uses the same Case shape.
"Tests are plain functions, with no classes, fixtures, or conftest.py" sits in the function-tests section but reads repo-wide, and e2e deliberately uses all three.
The e2e tests are moving to pytest, and one test runner for the whole repo is
simpler than two. pytest 9 runs the existing unittest suites unchanged and
reports each subTest on its own, so this commit switches the runner without
touching a test. It configures pytest in strict mode, to list each test, and to
print whole assertion diffs, since the tests compare whole responses.
nix run .#test runs every function's unit tests outside the sandbox, against
the virtualenvs the checks use, or one function's with pytest arguments after
its name.
Towards modelplaneai#473.
Signed-off-by: Nic Cope <nicc@rk0n.org>
The previous commit runs the unittest suites under pytest unchanged. This
commit ports them to pytest's own style, so each case in a table is a test of
its own that -k can select. Async tests call RunFunction with asyncio.run
rather than needing a plugin. pytest captures a test's output and shows it only
when the test fails, so the tests no longer disable the functions' logging.
pytest diffs two dicts in insertion order, where unittest sorted their keys.
The Structs a function returns rarely list their keys in the same order as the
test's expected ones, so the tests compare dicts with sorted keys, which keeps
a diff to the fields that differ. No case's request or expected response
changes.
Towards modelplaneai#473.
Signed-off-by: Nic Cope <nicc@rk0n.org>
The tests built their cases in several ways. Some derived a case from another
by copying and mutating it, or patched a request after building it. Others
built whole requests and responses with helpers, asserted on individual fields,
or computed expected values with the code under test or the SDK. A reader often
couldn't tell what a case checked without tracing code. Some case names also
claimed conditions their input didn't set up, and as sentences they made long
test IDs.
This commit rewrites the tests to the rules CONTRIBUTING now states, and says
why in a comment wherever a test departs from them. Helpers take everything
that varies as a required keyword argument, because a defaulted argument had
hidden that a case named for an unpinned replica was pinned. A field-level test
became a case in its entry point's table, or was deleted where a case already
sent the same request. Each case has a short CamelCase name, which is its test
ID, and a one-sentence reason, which pytest prints when it fails.
Every distinct RunFunction request and response is unchanged. Writing cases
out in full takes the tests from about 22,000 lines to 46,000.
Towards modelplaneai#473.
Signed-off-by: Nic Cope <nicc@rk0n.org>
CONTRIBUTING's Tests section sets out how a function's unit tests are
written, but only a reviewer checked that a test followed it.
This commit adds a checker that turns the rules an AST can decide into
a pass or fail: a table and the test that runs it, a case's fields,
name and reason, the assertion message, copies, merges and computed
child names, module-level objects other than tables, a helper's
parameters, and test names. It lives in hack/ and uses only the
standard library. A function-test-style flake check type-checks and
runs it, and nix run .#fix now lints its directory.
A test with a reason to depart from a rule escapes it on the reported
line, as in "# noqa: MPT401 # why". That's the form ruff already
reads, so ruff now treats MPT codes as external rather than unknown. A
case name counts an acronym such as GKE as one word, because many
cases name a cloud or protocol that way.
The new check fails on five violations in the tests as they stand.
Towards modelplaneai#473.
Signed-off-by: Nic Cope <nicc@rk0n.org>
The local e2e was a shell script, e2e/run.sh. Most of it checked the running
environment, with polling loops written out by hand, JSON and logs matched as
text, and an exit at the first failed check, so one fault hid every check after
it.
This commit replaces run.sh with a pytest suite that makes the checks run.sh's
--verify made, one test each, and brings the clusters up the way run.sh did.
Each test waits only for what it needs, so a gateway that refuses every caller
fails only the tests that need it to serve. nix run .#e2e keeps its flags, and
gains --test, to rerun the tests without bringing the clusters up again.
Four checks are stricter. The response must name the served model in its
top-level model field, where a model key at any depth passed. An unclaimed model
must get a 404, where any status but 200 passed. /v1/models must list the model
by its exact name, where a substring of another name passed. And the usage
record must carry the full endpoint name, where a prefix of it passed.
Fixesmodelplaneai#473.
Signed-off-by: Nic Cope <nicc@rk0n.org>
ty ran in 21 checks: a ty-<function> check per function, ty-e2e, and
function-test-style, which type-checked hack/check_function_tests.py
before running it.
This commit replaces them with one ty check. It still runs ty once per
function, against that function's venv, because every function's
package is named function and one run could resolve one function's
import to another's package. Then it checks e2e/ and hack/. A failed
run doesn't stop the rest, and the build ends by naming every directory
that failed. The runs are serial where the old checks built in
parallel, which costs about a second once the venvs are built.
Towards modelplaneai#473.
Signed-off-by: Nic Cope <nicc@rk0n.org>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
It broadly replaces both unit-test infrastructure and live two-cluster orchestration, warranting final human validation despite no confirmed blocking defect.
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
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.
negz
changed the title
dennis-upbound
left a comment
Description of your changes
Fixes #473.
#473 asks how we should write e2e tests, and the bake-off pointed at pytest. This PR moves the e2e tests and the function unit tests to pytest, so the repo has one test runner, and rewrites every unit test to one consistent pattern, which CONTRIBUTING's Tests section now describes.
To judge the pattern, read compose-model-cache's tests. No unit test's input or expectation changed: every distinct request and response is the same as before. Mutation testing ran about 18,000 injected bugs in the function code against main's tests and this branch's. The branch's tests catch about 1,400 more than main's, and miss one main's catch. A
function-test-styleflake check now enforces the pattern's mechanical rules.nix run .#testruns every function's unit tests. Name a function to run only its tests, and pass pytest arguments after it. Each case is a test of its own, with a short name:When a case fails, pytest prints a sentence saying what the case checks, then the whole response, with the expected lines as
-and the actual lines as+, the same way round as Go'scmp.Diff(want, got). Here's one case with an expected value broken on purpose:nix run .#e2e -- --verifybrings up both kind clusters as before, then runs the e2e tests, passing any further arguments to pytest:--testreruns the e2e tests against clusters that are already up, and--cleantears them down:I have:
nix flake check(or./nix.sh flake check) and made sure it passes.Added or updated tests covering any composition function changes.This PR changes only tests.git commit -s.