Skip to content

continuous integration

Kadyapam edited this page Oct 3, 2026 · 2 revisions

Continuous Integration

.github/workflows/test.yml is the pre-merge gate, on pull_request and pushes to main.

step command
member versions ./ci/check-member-versions.sh
build cargo build --workspace --all-targets --locked
test cargo test --workspace --all-targets --locked
clippy cargo clippy --workspace --all-targets --locked -- -D warnings -A clippy::large_enum_variant -A clippy::result_large_err

⚠ member versions runs first, and a failure there hides everything else

ci/check-member-versions.sh (Release Member Versions, noetl/ai-meta#331) runs before the toolchain is installed. When it fails, build, test and clippy are all reported skipped — not green, unavailable.

This is worth knowing because a PR in that state looks like whatever you were expecting to fail. One clippy-gate PR read as a clippy failure and was not: it had changed executor/ while leaving executor/Cargo.toml unmoved, so the release would have cut a tag and published a noetl-executor that did not contain the change. Check which step failed before diagnosing.

If you touch executor/, bump executor/Cargo.toml in the same PR. The script will not guess patch-vs-minor for you, by design.

clippy gates, with two documented exceptions

-D warnings is live as of noetl/ai-meta#378. It previously ran under || true, so the step cost its full runtime on every PR and could not fail.

Clearing the way took 39 lints — and note the measurement trap: a -D warnings run reports only 15 of them, because warnings-as-errors aborts compilation at the first failing crate, so noetl-executor fails and the main noetl crate (holding 23 of the 39) is never linted at all. Use a non-gating cargo clippy to inventory; the gating run is not an inventory.

The two -A entries, and why they are not just lints to fix

lint why
large_enum_variant EhdbQueryCommand (src/lib.rs), Tool and ToolSpecDoc (executor/src/playbook.rs) are clap Subcommand and serde tagged/untagged derive enums. Boxing the large variants is clippy's fix, but Tool is also derived under the json-schema feature that generates the published DSL schema, so boxing changes that schema's output — a downstream contract change.
result_large_err the large Err is tonic::Status; clippy's advice is to shrink tonic::Status, which is upstream. Single test-helper site in arrow-flight-client/src/tests.rs.

Any new -A must carry its reason on the same line. An unexplained -A reads as configuration rather than debt and never gets revisited.

clippy.toml, and why a threshold beats an -A

too-many-arguments-threshold = 8

Two functions in src/lib.rs take 8 arguments against clippy's default ceiling of 7. Raising the threshold keeps the lint live: a function growing to 9 arguments still fails the gate (verified with a 9-arg probe, which fired (9/8)). -A clippy::too_many_arguments would have disabled it everywhere. Prefer a threshold or a site-scoped #[allow] over a workflow-level -A.

⚠ clippy --fix is a suggestion, not a correctness proof

Reviewing its output is not optional. On this crate its manual_flatten fix rewrote

for line in reader.lines() { if let Ok(line) = line { … } }

into reader.lines().flatten() — which is lines_filter_map_ok: flatten() loops forever if the reader repeatedly returns Err. It traded a style lint for a potential hang. Both sites are now map_while(Result::ok), which stops at the first error exactly as the original did.

The toolchain is pinned

rust-toolchain.toml pins channel = "1.91.1"; the workflow installs @stable but the pin wins, so the lint set only moves when that file changes. Keep it — noetl/tools, noetl/server and noetl/worker have no pin and their gates can go red from a Rust release alone.

Corollary: a local clippy green is not evidence about CI unless your toolchain matches the pin.

Related

Clone this wiki locally