diff --git a/.agents/building-and-testing.md b/.agents/building-and-testing.md index 021d555ec..f82445f7e 100644 --- a/.agents/building-and-testing.md +++ b/.agents/building-and-testing.md @@ -45,3 +45,22 @@ Rules (both gates): - **Don't weaken the gate:** never hand-lower a baseline or widen a tolerance to turn a red gate green. The ratchet only moves up. - If a change drops coverage, **add tests** (sort `coverage-summary.json` by line% ascending to find untested code) rather than editing the baseline. When coverage legitimately rises, commit the regenerated baseline (`make test-coverage-baseline` / `test-ui-coverage-baseline`). - The Go gate is **strict — no tolerance**; `covermode=atomic` keeps it deterministic. The UI gate keeps a small tolerance only because its e2e coverage isn't. + +## Distributed-mode test suites + +Two suites cover distributed mode (frontend replicas, worker nodes, PostgreSQL, NATS), split by a Ginkgo label: + +- `make test-e2e-distributed` runs `Distributed && !VLLMMultinode && !Cluster` over `./tests/e2e/distributed` recursively. Services are wired directly into the test binary. ~240 specs, ~75s. +- `make test-e2e-cluster` runs `Cluster` and spawns real `local-ai` child processes through the `tests/e2e/distributed/cluster` helper package. 6 specs, 8m39s measured over three consecutive runs (509.1s / 509.8s / 512.3s). + +Both jobs live in `.github/workflows/tests-e2e-distributed.yml`, PR-triggered with a `paths-ignore` filter (see [.agents/ci-caching.md](ci-caching.md)) and `timeout-minutes: 45` each. They are advisory only because `master` carries no branch protection, which is a repository setting and not a YAML key: `continue-on-error: true` would flip the run's *conclusion* to success and hide the failure, so it is not used. + +- **Containers are suite-scoped, not spec-scoped.** `SetupInfra` used to start a PostgreSQL (~10s) and a NATS (~3.5s) per spec. Across the 213 specs behind it that was roughly **48 minutes of pure container startup per run**, which is why this suite was never in CI. Containers now start once in `BeforeSuite` and each spec gets its own database via `CREATE DATABASE` (~67ms), which is what the `dbName` argument was always describing. Adding a spec needs no change: call `SetupInfra("some-name")` as before, the name is a prefix and a counter keeps it unique. +- **Consequence for new specs:** the NATS bus is now *shared* within a Ginkgo process, so a wildcard subscriber can observe another spec's traffic. Filter assertions on an identifier your spec owns (a node ID, a job ID) instead of counting everything on `jobs.*.progress`, and verify the spec with `--randomize-all`. +- **`BeforeSuite`, not `SynchronizedBeforeSuite`.** Under `ginkgo -p` each process then gets its own container pair, keeping NATS subjects isolated per process. A single shared NATS across parallel processes would let specs on different processes see each other's messages on the same subject. +- **The label split.** The 8 argument-validation specs under `tests/e2e/distributed/cluster/` carry `Label("Distributed")` only, on purpose: they need no binary, no PostgreSQL and no NATS, so they belong in the fast job. That is why `test-e2e-distributed` keeps `-r` (it must reach the subpackage) and `test-e2e-cluster` deliberately does **not** (the subpackage is out of its scope). +- **`--fail-on-empty` is load-bearing on both targets.** Ginkgo exits 0 when a label filter selects nothing, so without it a refactor that renames or drops `Label("Cluster")` leaves the target reporting "Test Suite Passed" having started no cluster at all. `LOCALAI_E2E_REQUIRE_BINARIES` does not cover this case: it only fires inside a spec that is actually running. +- **The binary gate.** `localAIBinary()` and `mockBackendBinary()` **fail** rather than skip when `CI` is set, or when `LOCALAI_E2E_REQUIRE_BINARIES` is truthy; `LOCALAI_E2E_REQUIRE_BINARIES=0` (also `off`/`no`/`disabled`) forces skipping even under CI. The polarity is deliberate: in CI a skipped cluster spec is indistinguishable from a passing one, because Ginkgo exits 0 on skips. Locally a missing binary still just skips, since `CI` is unset in an ordinary shell. +- **Flake budget:** `DISTRIBUTED_TEST_FLAKES` defaults to **1**, not the repo-wide `TEST_FLAKES=5`, and `test-e2e-cluster` pins `--flake-attempts 1` outright. These suites exist to surface nondeterminism, so a retry converts exactly that signal into a green run. Raise it locally when bisecting something unrelated, not in the Makefile. +- **Coverage:** `tests/e2e/distributed` is excluded from the coverage roots (`COVERAGE_E2E_ROOTS = ./tests/e2e`, run non-recursively), and so is the `cluster` helper package beneath it. Neither suite moves the baseline, so production code that these suites are the only cover for reads as **uncovered**. Unit tests for such code belong under `./core/...` with `testutil.SetupTestDB()`. +- **Do not shorten the cluster suite's waits.** Three of its six specs sit at ~167s each because they wait out a 60s staleness threshold plus a 15s health-check tick. That wait is what stops the assertions from passing before the system could have reacted, which was a real false green earlier on. If the job has to get faster, the levers are CI concurrency or making the thresholds configurable, not shorter waits. diff --git a/.agents/ci-caching.md b/.agents/ci-caching.md index 6742049e6..244bb4549 100644 --- a/.agents/ci-caching.md +++ b/.agents/ci-caching.md @@ -153,7 +153,7 @@ This is worth more than it looks. Measured over the week to 2026-07-30, **97% of The volume is real: 13 gallery-only PRs merged that week with 10 open at once, and 78 of the 137 PRs opened were bot-generated. -`paths-ignore` on the PR trigger of `image-pr.yml` (7 jobs), `build-test.yaml` (3), `lint.yml` (2) and `tests-e2e.yml` (1) drops 13 of those 20. The excluded set: +`paths-ignore` on the PR trigger of `image-pr.yml` (7 jobs), `build-test.yaml` (3), `lint.yml` (2) and `tests-e2e.yml` (1) drops 13 of those 20. `tests-e2e-distributed.yml` (2 jobs) landed after that measurement and carries the same exclusion set for the same reason: its dependency graph is 99 packages, so an allowlist of paths would silently stop guarding the moment code moved, while a diff confined to the paths below provably cannot reach it. The excluded set: | Path | Why no image or Go build can see it | |---|---| @@ -192,7 +192,7 @@ What still runs, and why it has to: Two properties this relies on: - `paths-ignore` skips a run only when **every** changed file matches, so a PR touching the gallery *and* Go code still runs everything. That is what makes the exclusion safe rather than a hole. -- `master` carries no branch protection and no rulesets, so a skipped workflow reports no status and nothing waits on it. If required status checks are ever introduced, these four entries must be excluded from the required set or PRs will hang on "Expected — Waiting for status to be reported". +- `master` carries no branch protection and no rulesets, so a skipped workflow reports no status and nothing waits on it. If required status checks are ever introduced, these five entries must be excluded from the required set or PRs will hang on "Expected — Waiting for status to be reported". ### `image.yml` on master push is gated too, by a job rather than a path filter diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d87db37ea..9c1b176af 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -265,6 +265,37 @@ The e2e tests run LocalAI in a Docker container and exercise the API: make test-e2e ``` +### Running distributed-mode tests + +Distributed mode (several frontend replicas, worker nodes, PostgreSQL and NATS) has two suites. Both bring up their PostgreSQL and NATS with testcontainers, so Docker has to be available: + +```bash +make test-e2e-distributed # in-process: services wired directly into the test binary +make test-e2e-cluster # process-level: real local-ai child processes +``` + +`make test-e2e-distributed` is the fast one (around 240 specs in roughly 75 seconds). It starts one PostgreSQL and one NATS for the whole run and gives each spec its own database. It retries a failing spec once (`DISTRIBUTED_TEST_FLAKES`, default 1, deliberately lower than the repo-wide `TEST_FLAKES=5`), because this suite exists to catch nondeterministic cluster behaviour and retrying hides exactly that. + +`make test-e2e-cluster` runs `local-ai` as real child processes, one per frontend replica and one per worker, so a spec can kill a replica and assert what the survivors do. Budget about 8m40s: three of its six specs wait out real staleness and health-check windows. It needs a built binary and the mock backend: + +```bash +make build build-mock-backend +make test-e2e-cluster +``` + +Two environment variables steer it: + +| Variable | Purpose | +|---|---| +| `LOCALAI_E2E_BINARY` | path to the `local-ai` binary (default: `local-ai` in the repository root) | +| `LOCALAI_E2E_LOG_DIR` | directory for the per-process logs (default: a Ginkgo temp dir) | + +Set `LOCALAI_E2E_LOG_DIR` when debugging. A cluster failure is unreadable without the individual frontend and worker logs, and Ginkgo only tells you which assertion failed. + +A missing binary skips the cluster specs locally but fails them whenever `CI` is set, so a build problem cannot turn the CI job green without ever starting a cluster. `LOCALAI_E2E_REQUIRE_BINARIES=1` forces that failing behaviour anywhere; `LOCALAI_E2E_REQUIRE_BINARIES=0` forces the skip back on even under CI. + +Both suites run on pull requests via `.github/workflows/tests-e2e-distributed.yml`. + ### React UI tests and coverage The React UI (`core/http/react-ui/`) is covered by Playwright e2e specs, gated by a **monotonic line-coverage ratchet** (`make test-ui-coverage-check`, run in CI). The metric is non-deterministic — a fast local box reads higher than a slow CI runner for the same code — so a small tolerance is unavoidable.